Bug 83491 – Prepare html classes for sub-pixel LayoutUnits

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Levi Weintraub]

This eliminates remaining diffs in html/ with the subpixellayout branch.

[Levi Weintraub]

Created attachment 136279 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 136279 [details]
Patch

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

> Source/WebCore/html/ValidationMessage.cpp:123
> +    bubble->setInlineStyleProperty(CSSPropertyTop, hostY + static_cast<double>(hostRect.height()), CSSPrimitiveValue::CSS_PX);

Why the double casts?

[Levi Weintraub]

Comment on attachment 136279 [details]
Patch

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

>> Source/WebCore/html/ValidationMessage.cpp:123
>> +    bubble->setInlineStyleProperty(CSSPropertyTop, hostY + static_cast<double>(hostRect.height()), CSSPrimitiveValue::CSS_PX);
> 
> Why the double casts?

You're right, this can be removed. We were just missing a double flavor of operator+ in FractionalLayoutUnit. Will upload again.

[Levi Weintraub]

Created attachment 136295 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 136295 [details]
Patch

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

> Source/WebCore/platform/Length.h:127
> +    // FIXME: When we switch to sub-pixel layout, value will return float by default, and this will
> +    // no longer simply return value().
> +    int intValue() const
> +    {
> +        return value();
> +    }
> +

What will it return?  static_cast<int>(value())?  round(value())?  Can we make it return whatever that is now?  Or add such in a comment or something?

[Levi Weintraub]

Comment on attachment 136295 [details]
Patch

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

>> Source/WebCore/platform/Length.h:127
>> +
> 
> What will it return?  static_cast<int>(value())?  round(value())?  Can we make it return whatever that is now?  Or add such in a comment or something?

Sorry, my comment was better in the changelog than the code. This will simply inherit the current logic of value().

[Levi Weintraub]

Created attachment 136299 [details]
Patch

[Levi Weintraub]

I hope I've cleared up the issues you called out. I'd love another look :)

[Eric Seidel (no email)]

Comment on attachment 136299 [details]
Patch

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

> Source/WebCore/platform/Length.h:122
> +    // FIXME: When we switch to sub-pixel layout, value will return float by default, and this will inherit
> +    // the current implementation of value().

Were you going to change this?

[Eric Seidel (no email)]

Comment on attachment 136299 [details]
Patch

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

>> Source/WebCore/platform/Length.h:122
>> +    // the current implementation of value().
> 
> Were you going to change this?

Oh, I guess you did.  Anyway, the switch is very soon, so this is fine regardless.

[Levi Weintraub]

Comment on attachment 136299 [details]
Patch

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

>>> Source/WebCore/platform/Length.h:122
>>> +    // the current implementation of value().
>> 
>> Were you going to change this?
> 
> Oh, I guess you did.  Anyway, the switch is very soon, so this is fine regardless.

Thanks for the review!

[WebKit Review Bot]

Comment on attachment 136299 [details]
Patch

Clearing flags on attachment: 136299

Committed r113645: <http://trac.webkit.org/changeset/113645>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.