Bug 83497 – Replace numeric_limits<LayoutUnit>::min/max with constants

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Emil A Eklund]

As came up in the original review of the FractionalLayoutUnit code we should not register numeric_limits for the FractionalLayoutUnit in the std namespace and instead use constants for the min/max value.

[Emil A Eklund]

Created attachment 136298 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 136298 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136298&action=review

> Source/WebCore/ChangeLog:10
> +        Replace all uses of numeric_limits<LayoutUnit>::min/max with
> +        MIN_LAYOUT_UNIT and MAX_LAYOUT_UNIT respectively in preparation for the
> +        switch to subpixel layout.

Why does this make the switch easier?

[Emil A Eklund]

(In reply to comment #2)
> (From update of attachment 136298 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136298&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        Replace all uses of numeric_limits<LayoutUnit>::min/max with
> > +        MIN_LAYOUT_UNIT and MAX_LAYOUT_UNIT respectively in preparation for the
> > +        switch to subpixel layout.
> 
> Why does this make the switch easier?

It allows us to use an abstraction to represent the min/max values for ints (for now) and FractionalLayoutUnits (after the switch). We originally planed to just leave it as is and define numeric_limits for FractionalLayoutUnit but that was shoot down during the initial review of FractionalLayoutUnit.h as it would involve declaring it in the std namespace.

[Eric Seidel (no email)]

Comment on attachment 136298 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136298&action=review

>>> Source/WebCore/ChangeLog:10
>>> +        switch to subpixel layout.
>> 
>> Why does this make the switch easier?
> 
> It allows us to use an abstraction to represent the min/max values for ints (for now) and FractionalLayoutUnits (after the switch). We originally planed to just leave it as is and define numeric_limits for FractionalLayoutUnit but that was shoot down during the initial review of FractionalLayoutUnit.h as it would involve declaring it in the std namespace.

OK.

I think I might prefer LayoutUnit::maxValue and LayoutUnit::zeroValue, etc.  But I understand that's impractical at this juncture.

[Levi Weintraub]

(In reply to comment #4)
> (From update of attachment 136298 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136298&action=review
> 
> >>> Source/WebCore/ChangeLog:10
> >>> +        switch to subpixel layout.
> >> 
> >> Why does this make the switch easier?
> > 
> > It allows us to use an abstraction to represent the min/max values for ints (for now) and FractionalLayoutUnits (after the switch). We originally planed to just leave it as is and define numeric_limits for FractionalLayoutUnit but that was shoot down during the initial review of FractionalLayoutUnit.h as it would involve declaring it in the std namespace.
> 
> OK.
> 
> I think I might prefer LayoutUnit::maxValue and LayoutUnit::zeroValue, etc.  But I understand that's impractical at this juncture.

We have FractionalLayoutUnit::max and min, and would be happy to add a ::zero. That'll definitely be part of our cleanup pass once we've flipped the switch.

[Emil A Eklund]

Comment on attachment 136298 [details]
Patch

Will do a cleanup pass once we've made the switch to make it LayoutUnit::max/min/zero as suggested.

[WebKit Review Bot]

Comment on attachment 136298 [details]
Patch

Clearing flags on attachment: 136298

Committed r113665: <http://trac.webkit.org/changeset/113665>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.