899100 - FrameMetrics::mCompositionBounds needs to be consistent across top-level FrameMetrics and subdocument FrameMetrics

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

FrameMetrics::mCompositionBounds represents the user-visible area that a particular document is rendered into. It is currently of type ScreenIntRect, which makes sense for the top-level document. This is because if the user has a device with a screen of 500x1000 pixels (and assuming content is full-screened) then the composition bounds are always 500x1000 ScreenPixels, regardless of what the zoom factor and/or resolution is on the content.

However, for subdocuments, the situation is different. Consider an iframe of CSS pixel dimension 100x100, for example. As the user pinch-zooms on the container page, the ScreenPixel size of the iframe composition bounds will change. In effect, the iframe composition bounds makes most sense to be stored in LayoutDeviceIntPixels, so that it doesn't need to be changed as resolution and the async transform change.

So we have a conundrum: for the top-level document the mCompositionBounds should be a ScreenIntRect but for subdocuments it should be a LayoutDeviceIntRect.

I guess the best solution here depends on what mCompositionBounds is used for. Currently one of the main uses is in the CalculateIntrinsicScale() function which I've already decided should be killed (bug 885023). The other big use is in fixed-position calculations. It seems to me that both of these are only relevant for the top-level document, so we might be able to get away with ignoring it on subdocuments. (By "ignoring it" I mean that we should redefine it to always mean the top-level document's composition bounds, and so all FrameMetrics instances will pretty much have the same value at any given time).

If that doesn't work the next best alternative I think is to convert to LayoutDeviceIntPoint, and have a function that returns the value as a ScreenPoint. The function would basically need to walk up the layer tree, grabbing the frame metrics from each container layer and the APZC from each scrollable layer, and multiply in the mResolution and async transforms from all of those. This is not simple to do so I would prefer to go with the first approach if at all possible.

Comment 1

[Chris Lord [:cwiiis]]

iirc, mCompositionBounds were only ever used for determining what tiles to update in a batch, async fixed-pos layer calculations and reused tiles (now defunct) - I think your first idea will work fine for now.

ftr, I also don't think the second suggestion would be particularly hard to implement though, if we needed to.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

The other big use of the composition bounds which I previously overlooked is to determine scrolling behaviour in Axis.cpp. If there is a subframe with a content height of 1000 pixels then it should max out at a y-axis scroll offset of 1000 - something, where the something is the height of the composition bounds. At least that's how it works currently. We might be able to switch that code to use mViewport, maybe. But it definitely needs to be corrected somehow.

Comment 3

[Botond Ballo [:botond]]

This is fixed by bug 904533 and its follow-ups (bug 913205, bug 914825, bug 918682).