Closed Bug 907906 Opened 7 years ago Closed 7 years ago

Finishing up the gfx::Margin template code

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: billm, Assigned: kats)

References

Details

Attachments

(1 file)

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.
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+
(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?
Isn't file consistency >>> style guidelines?
The file is inconsistent, so the style guide takes precedence.
https://hg.mozilla.org/mozilla-central/rev/b5fff90c343d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.