Bug 77246 – Use LayoutSize instead of IntSize for RenderLayer::scrollOffset

• Component: New Bugs
• Status: RESOLVED INVALID

[Tony Chang]

Use LayoutSize instead of IntSize for RenderLayer::scrollOffset

[Tony Chang]

Created attachment 124370 [details]
Patch

[Tony Chang]

In some sense, all of these should be Int{Size,Point} since you can't scroll a fractional unit, but it looks like RenderLayer already uses mostly Layout{Size,Point} everywhere, so this is just for consistency.

[Levi Weintraub]

Comment on attachment 124370 [details]
Patch

This LGTM (there are annoying reasons why we want LayoutUnits for scroll), but I'm not a reviewer.

[Darin Adler]

Comment on attachment 124370 [details]
Patch

I am not sure this is correct. We intentionally don’t use rendering coordinates for scrolling; this is the place that the fractional layout machinery meets the non-fractional scrolling so some conversion is necessary. We should get the folks working on fractional pixel layout to remind us about how this works.

[Tony Chang]

(In reply to comment #4)
> (From update of attachment 124370 [details])
> I am not sure this is correct. We intentionally don’t use rendering coordinates for scrolling; this is the place that the fractional layout machinery meets the non-fractional scrolling so some conversion is necessary. We should get the folks working on fractional pixel layout to remind us about how this works.

Levi is working on fractional pixel layout.  Maybe he can explain in more detail why we use LayoutUnits for scroll offsets.

[Levi Weintraub]

(In reply to comment #5)
> Levi is working on fractional pixel layout.  Maybe he can explain in more detail why we use LayoutUnits for scroll offsets.

My initial response was incorrect. In the correct version of our patch, which we're working to get committed now, all scroll values are stored as integers representing pixels. We're working to correct this fallacy in trunk. Sorry for the confusion.