793201 - Fix compiler warnings in Azure with MSVC

Status: RESOLVED FIXED

Description

[Bas Schouten (:bas.schouten)]

Right now Azure causes a lot of compiler warnings. That should be addressed.

Comment 1

[Bas Schouten (:bas.schouten)]

So this bug is meant to address the compiler warnings when compiling code under gfx/2d using the MSVC compiler.

The majority of compiler warnings here will be conversions with precision losses. Those in most cases should just be changed to use explicit type conversions.

Comment 2

[Andrew Quartey [:drexler]]

Attachment: patch

Explicit conversions between types and the use of #pragma to silence compiler warning C4251 for private members of mozilla::gfx::AlphaBoxBlur in Blur.h.

Comment 3

[Bas Schouten (:bas.schouten)]

Comment on attachment 663648 [details] [diff] [review]
patch

Review of attachment 663648 [details] [diff] [review]:
-----------------------------------------------------------------

All in all this looks very good! I'll be very happy to see these compiler warnings go away. Some small comments.

::: gfx/2d/Blur.h
@@ +12,5 @@
>  namespace gfx {
>  
> +#ifdef _MSC_VER
> +#pragma warning( disable : 4251 )
> +#endif

I'm a little concerned about this being a header. It might at some point obscure something useful, maybe that's ok though.

::: gfx/2d/DrawTargetCairo.cpp
@@ +408,1 @@
>   

whitespace

::: gfx/2d/PathCairo.cpp
@@ +270,5 @@
>  
>    double x1, y1, x2, y2;
>  
>    cairo_path_extents(*mPathContext, &x1, &y1, &x2, &y2);
> +  Rect bounds((Float)x1, (Float)y1, (Float)(x2 - x1), (Float)(y2 - y1));

Float(x1), etc.

::: gfx/2d/PathHelpers.h
@@ +23,5 @@
>  
>    // Clockwise we always sweep from the smaller to the larger angle, ccw 
>    // it's vice versa.
>    if (!aAntiClockwise && (aEndAngle < aStartAngle)) {
> +    Float correction = (Float)ceil((aStartAngle - aEndAngle) / (2.0f * M_PI));

Float() etc.

Comment 4

[Andrew Quartey [:drexler]]

Attachment: patch

Fixed nits.

> > +#ifdef _MSC_VER
> > +#pragma warning( disable : 4251 )
> > +#endif
> 
> I'm a little concerned about this being a header. It might at some point
> obscure something useful, maybe that's ok though.
> 
I share similar concerns too. From my readings, the 'proper solution' seems be use of explicit template specializations for |IntRect| and |Rect| to satisfy the compiler but my attempts at such a solution generated errors elsewhere. This requires some serious template tricks thus my leaving the #pragma for suppression for now.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 664129 [details] [diff] [review]
patch

Review of attachment 664129 [details] [diff] [review]:
-----------------------------------------------------------------

Bugzilla suggests a lot of weeeeird whitespace cruft in PathHelpers.h, I'm not so sure why, we seem to get those a lot lately. Please upload a fixed patch, looks good otherwise!

Comment 6

[Andrew Quartey [:drexler]]

Fixed whitespace nits.http://hg.mozilla.org/integration/mozilla-inbound/rev/96ef3b8bd9ed

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/96ef3b8bd9ed