Closed
Bug 793201
Opened 9 years ago
Closed 9 years ago
Fix compiler warnings in Azure with MSVC
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: bas.schouten, Assigned: drexler)
Details
(Whiteboard: [mentor=bas.schouten@live.nl][lang=c++])
Attachments
(1 file, 1 obsolete file)
24.27 KB,
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
Right now Azure causes a lot of compiler warnings. That should be addressed.
Reporter | ||
Comment 1•9 years ago
|
||
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.
Summary: Fix compiler warnings in Azure → Fix compiler warnings in Azure with MSVC
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → andrew.quartey
Assignee | ||
Comment 2•9 years ago
|
||
Explicit conversions between types and the use of #pragma to silence compiler warning C4251 for private members of mozilla::gfx::AlphaBoxBlur in Blur.h.
Attachment #663648 -
Flags: review?(bas.schouten)
Reporter | ||
Comment 3•9 years ago
|
||
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.
Assignee | ||
Comment 4•9 years ago
|
||
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.
Attachment #663648 -
Attachment is obsolete: true
Attachment #663648 -
Flags: review?(bas.schouten)
Attachment #664129 -
Flags: review?(bas.schouten)
Reporter | ||
Comment 5•9 years ago
|
||
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!
Attachment #664129 -
Flags: review?(bas.schouten) → review+
Assignee | ||
Comment 6•9 years ago
|
||
Fixed whitespace nits.http://hg.mozilla.org/integration/mozilla-inbound/rev/96ef3b8bd9ed
Status: NEW → ASSIGNED
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/96ef3b8bd9ed
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in
before you can comment on or make changes to this bug.
Description
•