Closed
Bug 907906
Opened 11 years ago
Closed 11 years ago
Finishing up the gfx::Margin template code
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: billm, Assigned: kats)
References
Details
Attachments
(1 file)
8.64 KB,
patch
|
cwiiis
:
review+
|
Details | Diff | Splinter Review |
There's a problem in bug 907495 where we want to call Deflate on a ScreenIntRect with a margin parameter. ScreenIntRect is an IntRectTyped, where the Margin template parameter is a MarginTyped<UnknownUnits>. This margin type actually uses floats for storage. When Deflate is instantiated, it calls std::max on int(0) (because ScreenIntRect uses ints) and a float from the margin. This is rejected by the compiler.
Assignee | ||
Comment 1•11 years ago
|
||
This compiles everywhere: https://tbpl.mozilla.org/?tree=Try&rev=00f6a13931e4
Attachment #793983 -
Flags: review?(chrislord.net)
Comment 2•11 years ago
|
||
Comment on attachment 793983 [details] [diff] [review] Patch Review of attachment 793983 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. ::: gfx/2d/Rect.h @@ +20,5 @@ > + public units { > + typedef BaseMargin<int32_t, IntMarginTyped<units> > Super; > + > + IntMarginTyped() : Super() {} > + IntMarginTyped(int32_t _top, int32_t _right, int32_t _bottom, int32_t _left) : Is this _ notation what we use now? (as opposed to aTop, aRight, etc.)
Attachment #793983 -
Flags: review?(chrislord.net) → review+
Comment 3•11 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #2) > Comment on attachment 793983 [details] [diff] [review] > Patch > > Review of attachment 793983 [details] [diff] [review]: > ----------------------------------------------------------------- > > LGTM. > > ::: gfx/2d/Rect.h > @@ +20,5 @@ > > + public units { > > + typedef BaseMargin<int32_t, IntMarginTyped<units> > Super; > > + > > + IntMarginTyped() : Super() {} > > + IntMarginTyped(int32_t _top, int32_t _right, int32_t _bottom, int32_t _left) : > > Is this _ notation what we use now? (as opposed to aTop, aRight, etc.) No, it isn't. Please fix that?
Assignee | ||
Comment 4•11 years ago
|
||
Isn't file consistency >>> style guidelines?
Comment 5•11 years ago
|
||
The file is inconsistent, so the style guide takes precedence.
Assignee | ||
Comment 6•11 years ago
|
||
Ok, I changed my patch to use the aFoo convention. https://hg.mozilla.org/integration/mozilla-inbound/rev/b5fff90c343d
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b5fff90c343d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in
before you can comment on or make changes to this bug.
Description
•