1647222 - Picture caching doesn't create slice for scrolled content at certain zoom levels

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jamie Nicol [:jnicol]]

I noticed this when investigating bug 1645665 on android. However, it reproduces on desktop too. (And not just with desktop-zooming, but with ctrl+/- zooming too)

Enable picture cache debugging and open https://webkit-search.igalia.com/webkit/search?q=borderwidth&path=

You can see that the scrollable search results get their own slice, and there is therefore no invalidation while scrolling.

Zoom in slightly with "ctrl +" and scroll. You can now only see a single slice. Its tiles remain in a fixed position meaning that the scrolling content is constantly invalidating.

Certain zooms work correctly, eg 100%, 150%, 200%. And some are broken, eg 110%, 120%, 133%

Comment 1

[Glenn Watson [:gw]]

I can reproduce this and can see what is happening.

The spatial tree when zoomed in is shown below.

For spatial node 4, we have:

viewport: Rect(1892.0×2059.0 at (0.0,0.0))
scrollable_size: 0.00012207031×0.0

Because this scroll frame technically has a non-zero scrollable size, WR treats it as a "real" scroll root, and attaches the slice to that. In reality, it appears to be some kind of rounding error in the Gecko/WR display list generation that is caused by the zoom factor?

The WR code that detects if it's a valid scroll root is [1]. If I change this zero check to be > 0.01 then the page correctly scrolls at all zoom levels.

It's probably reasonable to change WR to make this an epsilon style check. Although, perhaps Gecko should be providing an exact zero size scroll frame in this case, since it may suggest a deeper snapping / rounding issue elsewhere?

[1] https://searchfox.org/mozilla-central/rev/46e3b1ce2cc120a188f6940b5c6eab6b24530e4f/gfx/wr/webrender/src/spatial_tree.rs#661

┌ spatial tree
│  ├─ ReferenceFrame
│  │  ├─ kind: Transform
│  │  ├─ transform_style: Flat
│  │  ├─ source_transform: Value([I])
│  │  ├─ origin_in_parent_reference_frame: (0.0,0.0)
│  │  ├─ index: SpatialNodeIndex(0)
│  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) })
│  │  ├─ coordinate_system_id: CoordinateSystemId(0)
│  │  ├─ ScrollFrame
│  │  │  ├─ viewport: Rect(1892.0×2133.0 at (0.0,0.0))
│  │  │  ├─ scrollable_size: 0.0×0.0
│  │  │  ├─ scroll offset: (-0.0,-0.0)
│  │  │  ├─ external_scroll_offset: (0.0,0.0)
│  │  │  ├─ kind: PipelineRoot
│  │  │  ├─ index: SpatialNodeIndex(1)
│  │  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) })
│  │  │  ├─ coordinate_system_id: CoordinateSystemId(0)
│  │  │  ├─ ReferenceFrame
│  │  │  │  ├─ kind: Transform
│  │  │  │  ├─ transform_style: Flat
│  │  │  │  ├─ source_transform: Value([I])
│  │  │  │  ├─ origin_in_parent_reference_frame: (0.0,74.0)
│  │  │  │  ├─ index: SpatialNodeIndex(2)
│  │  │  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,74.0) })
│  │  │  │  ├─ coordinate_system_id: CoordinateSystemId(0)
│  │  │  │  ├─ ScrollFrame
│  │  │  │  │  ├─ viewport: Rect(1892.0×2059.0 at (0.0,0.0))
│  │  │  │  │  ├─ scrollable_size: 0.0×0.0
│  │  │  │  │  ├─ scroll offset: (-0.0,-0.0)
│  │  │  │  │  ├─ external_scroll_offset: (0.0,0.0)
│  │  │  │  │  ├─ kind: PipelineRoot
│  │  │  │  │  ├─ index: SpatialNodeIndex(3)
│  │  │  │  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,74.0) })
│  │  │  │  │  ├─ coordinate_system_id: CoordinateSystemId(0)
│  │  │  │  │  ├─ ScrollFrame
│  │  │  │  │  │  ├─ viewport: Rect(1892.0×2059.0 at (0.0,0.0))
│  │  │  │  │  │  ├─ scrollable_size: 0.00012207031×0.0
│  │  │  │  │  │  ├─ scroll offset: (-0.0,-0.0)
│  │  │  │  │  │  ├─ external_scroll_offset: (0.0,0.0)
│  │  │  │  │  │  ├─ kind: Explicit
│  │  │  │  │  │  ├─ index: SpatialNodeIndex(4)
│  │  │  │  │  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,74.0) })
│  │  │  │  │  │  ├─ coordinate_system_id: CoordinateSystemId(0)
│  │  │  │  │  │  ├─ ReferenceFrame
│  │  │  │  │  │  │  ├─ kind: Transform
│  │  │  │  │  │  │  ├─ transform_style: Flat
│  │  │  │  │  │  │  ├─ source_transform: Binding(PropertyBindingKey { id: PropertyBindingId { namespace: IdNamespace(106247), uid: 1 }, _phantom: PhantomData }, [I])
│  │  │  │  │  │  │  ├─ origin_in_parent_reference_frame: (0.0,0.0)
│  │  │  │  │  │  │  ├─ index: SpatialNodeIndex(5)
│  │  │  │  │  │  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,74.0) })
│  │  │  │  │  │  │  └─ coordinate_system_id: CoordinateSystemId(0)
│  │  │  │  │  │  ├─ ReferenceFrame
│  │  │  │  │  │  │  ├─ kind: Transform
│  │  │  │  │  │  │  ├─ transform_style: Flat
│  │  │  │  │  │  │  ├─ source_transform: Binding(PropertyBindingKey { id: PropertyBindingId { namespace: IdNamespace(106247), uid: 2 }, _phantom: PhantomData }, [I])
│  │  │  │  │  │  │  ├─ origin_in_parent_reference_frame: (0.0,0.0)
│  │  │  │  │  │  │  ├─ index: SpatialNodeIndex(6)
│  │  │  │  │  │  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,74.0) })
│  │  │  │  │  │  │  └─ coordinate_system_id: CoordinateSystemId(0)
│  │  │  │  │  │  ├─ ScrollFrame
│  │  │  │  │  │  │  ├─ viewport: Rect(1880.0×1974.8545 at (0.0,72.145454))
│  │  │  │  │  │  │  ├─ scrollable_size: 28880.0×23128.0
│  │  │  │  │  │  │  ├─ scroll offset: (-0.0,-549.8182)
│  │  │  │  │  │  │  ├─ external_scroll_offset: (0.0,549.8182)
│  │  │  │  │  │  │  ├─ kind: Explicit
│  │  │  │  │  │  │  ├─ index: SpatialNodeIndex(7)
│  │  │  │  │  │  │  ├─ content_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  │  ├─ viewport_transform: ScaleOffset { scale: (1.0,1.0), offset: (0.0,0.0) }
│  │  │  │  │  │  │  ├─ snapping_transform: Some(ScaleOffset { scale: (1.0,1.0), offset: (0.0,74.0) })
│  │  │  │  │  │  │  └─ coordinate_system_id: CoordinateSystemId(0)

Comment 2

[Jamie Nicol [:jnicol]]

Thanks for the investigation, Glenn!

So the scrollable_size is calculated from the content_size - frame_rect.size. Gecko doesn't directly provide the scrollable size, but rather provides the scollable content rect and the scroll frame's clip.

Those values come from here, as content_rect and clipBounds respectively. The clipbounds does not need a coordinate space conversion so is a nice round number, whereas the content rect needs converted from CSS pixels to device pixels. When the devPixel ratio isn't a nice fraction we hit this problem.

This code is webrender specific so AFAIK this shouldn't cause any issues in Gecko itself. Whether or not it should be fixed here in ClipManager or in webrender I am not so sure. Can/should the content rect ever be a fractional number of device pixels? If not, I guess we can round the content rect here and that will solve the problem. If a fractional content rect is allowed then I'm not sure if we can do anything here, so the epsillon check in webrender makes sense. Botond, do you have an opinion on this?

Comment 3

[Botond Ballo [:botond]]

APZ tracks the content rect (called "scrollable rect" in that part of the code) as a CSSRect, so it can be fractional in device pixels, yes.

Comparable "can this scroll?" checks in APZ, like this one, do use an epsilon, of size 0.01 pixels. The choice of this epsilon is explained here.

Comment 4

[Glenn Watson [:gw]]

Attachment: Bug 1647222 - Improve detection of real scroll frames.

We detect empty scroll roots by checking the valid scrollable size
of a frame, in order to avoid attaching picture cache slices to
these redundant scroll frames.

However, under some fractional zoom scenarios, rounding CSS pixels
to device pixels can result in small rounding errors.

Apply the same epsilon check that Gecko uses in APZ code in order
to detect if a scroll frame is actually scrollable.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b425233628af
Improve detection of real scroll frames. r=jnicol

Comment 6

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b425233628af