Closed Bug 1645665 Opened 4 years ago Closed 4 years ago

Scrolling searchfox on android is intermittently a lot less smooth

Categories

(Core :: Graphics: WebRender, defect)

defect

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: emilio, Assigned: jnicol)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

On this page:

https://webkit-search.igalia.com/webkit/search?q=borderwidth&path=

Scrolling is pretty bad most of the time on my new Android device (I enabled WR explicitly). It's a Moto g8 plus with an Adreno GPU.

The renderer is Qualcomm -- Adreno (TM) 610

Ah! It has to do with the zoom level. At some zoom levels it gets really bad. It happens even in about:support.

I was at the zoom level that you get when you tap the search box in searchfox.

Flags: needinfo?(jnicol)

Profile: https://share.firefox.dev/3d8gIqo

Lots of time spent in VBO uploads. The draw call count is extremely high: easily over a thousand on some frames. I think this is due to the texture cache being fragmented.

The texture cache's rgba8 textures have 16 layers each. Even after a slight zoom, we can still fit all the glyhps we need in a single texture, and we render quickly with 20 or so draw calls. Zooming in a wee bit more, so that our glyphs are spread out over 2 different textures, can bump the draw call count up to ~200 on some frames. Zooming in further, so our glyphs are spread over 6 textures results in some frames with 2000+ draw calls, and it is extremely jumpy.

This is especially pronounced on this page, but I have noticed the same sort of thing ever since we started using multiple fixed-size textures rather than growing the single textures.

There are some things we could potentially do avoid so much fragmentation: tweaking the number of layers of each texture, attempting to consolidate layers back in to fewer textures, or more aggressively evicting items in order to avoid creating new textures.

However, it still seems to have a much worse affect on the number of batches than you'd hope. 6 textures isn't all that many! I'll look a bit deeper in to why specifically we are breaking so many batches.

This is all exacerbated by the fact that picture caching doesn't seem to detect the scroll root properly on that page. So rather than the tiles scrolling with the page, the tiles remain fixed and become invalidated as the text moves.

Flags: needinfo?(jnicol)
Assignee: nobody → jnicol
Blocks: wr-android
Severity: -- → S2

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

I was at the zoom level that you get when you tap the search box in searchfox.

Sounds like it might be the same issue as bug 1642848.

See Also: → 1642848

Does tapping on a form cause us to zoom using mState == ANIMATING_ZOOM? If not, IsAsyncZooming would return false, which will cause us to rasterize glyphs for every frame, which would explain why there's so much stress on the texture cache.

However, even just a normal pinch zoom then a scroll is enough to fill a couple of 16x512x512 textures and make scrolling really slow, so I could believe that there's nothing unique about zooming with by tapping a form.

I believe tapping on a form should end up using the ZoomToRect API in APZC, which does set the state, here. But it might be that the state gets cleared somehow before the zoom animation is done, I didn't run it to see.

Filed bug 1647222 for the picture caching issue.

See Also: → 1647222

So the issue here is that in add_prim_to_batch() for text runs we call ResourceCache::fetch_glyphs(), which calls a provided callback for each series of consecutive glyphs in the text run which are in the same texture. When all glyphs are in a single texture array, this just calls the callback once. However, if the texture cache is fragmented the callback will be called multiple times for a single text run.

Each time the callback is called, we attempt to find a batch for the subrun of glyphs which share the same texture. However, note that we pass bounding_rect for the bounds of the glyphs. This is the bounds of the entire text run. This means that when we attempt to find a batch for the second (or later) subrun of glyphs, it will always overlap with the previous batch, because they have an identical bounding rect. This forces us to create an entirely new batch, rather than reuse a previous one.

See Also: → 1651753

When adding a text run to a batch, we split the text run in to contiguous
runs of glyphs which can be batched together. For example, when a glyph is
encountered mid-run with a different glyph format or that is cached in a
different texture, then it must be batched separately than the preceeding
glyphs.

However, even though we are only batching sub-runs of glyphs together, we use
the entire text run's bounding rect as the bounds we pass to
set_params_and_get_batch(). This means that each sub-run of glyphs that are
batched together will be forced to create an entirely new batch rather than
reuse an existing one (because their bounds will definitely overlap.)

This can have catastrophic consequences on performance. When the texture cache
is full enough that we have allocated a second or greater texture array, then we
risk having text runs with glyphs fragmented over different textures. This can
easily lead to thousands of draw calls on simple but text-heavy pages.

The solution is to calculate the bounds of just the sub-run of glyphs that are
being batched together, and use that instead of the text run's bounds.

Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5cee93aa5199
Calculate tight bounds for each sub-run of glyphs when batching. r=lsalzman
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
Regressions: 1758127
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: