1636616 - Scrolling https://en.wikipedia.org/wiki/Earth on Intel has jank

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

This is split off from the similar bug 1576637

Comment 1

[Jeff Muizelaar [:jrmuizel]]

I took a GPUView trace of this on my 4400 running at 2560x1440
An example skipped frame has 4 calls to CreateTexture2D, each for a size of 1056x1056. These are coming from DCLayerTree::CreateEGLSurfaceForCompositionSurface allocating through DirectComposition which doesn't have symbols for some reason.
After these calls which take in total about 7ms we end up submitting 10ms of work to the GPU and then waiting in RenderCompositorANGLE::WaitForPreviousGraphicsCOmmandFinishedQuery() for majority of that.

All in all quite a disaster.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

If I turn on picture cache debugging I see some whole screen flashes of red as I'm scrolling. I don't know for certain whether those correspond to the skipped frames (we really need to add a gpu marker that records the amount of invalidation) but they probably don't help.

Bert, do you want to take a look at the picture caching issue?

Comment 3

[Jeff Muizelaar [:jrmuizel]]

When I turn off DirectComposition I don't see the TextureCreation happening but I still get some long gpu times 9ms. These long gpu times don't start until 12ms into our frame budget so we have no hope of making it on time.

Comment 4

[Bert Peers [:bpeers]]

Attachment: wikipedia_tilecache.png

Taking a quick look, the suspicious frame is when the clip rect changes?
Most of the time scrolling only causes an invalidation on the bottom 2 tiles due to a PrimCount change, all good. But occasionally there's an invalidation of multiple tiles across the screen all due to:
prim_clip_rect changed from 1635x6025 at (264,119) to 1635x6144 at (264,512)
and then
prim_clip_rect changed from 1635x6144 at (264,512) to 1635x6144 at (264,1024)
and then
prim_clip_rect changed from 1635x6144 at (264,1024) to 1635x6144 at (264,1536)
etc. -- every so often the clip rect "rebases" by 512 units (its size stays constant after the 1 jump up).

Maybe Glenn's work to better separate spatial nodes from clip chains will help for this?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

I actually see jank on my HD 530 running at the same resolution as well.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

The 512 units is presumably coming from here: https://bugzilla.mozilla.org/show_bug.cgi?id=1635472 and happens every time we get a new scene. I think I saw the flashing happen more often on a build before that change.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

This issue reminds me a bit of bug 1625816

Comment 8

[Glenn Watson [:gw]]

The comments above are correct, thanks for the initial investigation!

Previously, it was only the background rectangle primitives that were getting clips set to different values when the display port changed.

Since there is some (small) overhead in WR to handle this without invalidation, it has an optimization (the clip_by_tile variable) where only rectangles clamp the primitive clip to tile boundaries.

However, something has changed (in Gecko?) where other primitives are now having their clips affected by the display port.

I verified that setting clip_by_tile on for all primitives fixes the issue - we no longer see invalidations of the whole page when a new display list arrives.

I'll do a bit more investigating to see why the behavior changed, but it's probably reasonable to change WR to do the clip_by_tile work for every primitive, so I might just make that change to fix it anyway.

Comment 9

[Glenn Watson [:gw]]

Attachment: Bug 1636616 - Fix invalidations occuring when display port changes.

On some pages, primitives other than rectangles may have their
clip rect affected by the display port, which was causing incorrect
invalidations and extra rasterization work.

Remove clip_by_tile check, and clamp clip rect to tile boundaries
for all primitive types. Also switch to a more efficient min/max
representation of the clip rect for primitive dependencies.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3dfc4d905e23
Fix invalidations occuring when display port changes. r=Bert

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3dfc4d905e23

Comment 12

[Glenn Watson [:gw]]

Jeff, Jessie, what do you think about trying to get this uplifted to beta? It probably fixes a number of scroll / stutter issues on various pages. I'm not sure if it applies cleanly to beta yet.

Comment 13

[Jessie [:jbonisteel] pls NI]

I'm not sure if it applies cleanly to beta yet.

If that means this might cause issues in Beta, we should likely wait.

If it is low risk, then we can request an uplift and just make sure to be detailed about why we need it, and why we think it is low risk.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

We missed this beta train.