1689746 - Slow scrolling on weather.com today page

Status: NEW

(Core :: Graphics: WebRender, defect, P3)

Description

[Asif Youssuff]

Attachment: New York, NY Weather Forecast and Conditions - The Weather Channel _ Weather.com (1_29_2021 5_09_20 PM).7z

Basic information

Steps to Reproduce:

1. Navigate to https://weather.com/weather/today/l/10010:4:US
2. Scroll down using touchpad

Expected Results:

Speedy scrolling.

Actual Results:

Slow scrolling.

---

More information

Profile URL: https://share.firefox.dev/2MDpLbz

Basic systems configuration:

OS version: Windows 10

GPU model: NVIDIA NVS 3100M

Number of cores: 2

Amount of memory (RAM): 8GB

Thanks so much for your help.

Comment 1

[Asif Youssuff]

Attachment: about:support

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:yoasif, if you think that's a regression, could you try to find a regression range using for example mozregression?

Comment 3

[Bas Schouten (:bas.schouten)]

This is pretty bad since it's compositor jank on a fairly major website. Marking qf:p1 and moving to WR.

Comment 4

[Glenn Watson [:gw]]

Picture caching doesn't seem to be effective here - content is constantly invalidating and redrawing during scrolling, which will be at least part of the explanation for the jank (there does also seem to be issues in the content process, maybe they're related).

It's not clear why, since it looks like a simple page - will need some further investigation.

Comment 5

[Glenn Watson [:gw]]

Wow, despite the mostly static page (there are occasional animations occurring, depending on the ads that get served), there's a lot going on here .. there's a few things occurring that are making WR think the content is changing and needs to be re-rasterized every frame.

The main one is that several images that are constantly getting update_image_template called every frame. I think it might be the weather icons in the forecast, but I'm not totally sure. Each time an image is updated, WR increases the image generation, and invalidates any tiles that intersect those image primitives.

Next week, I'll dig further and find exactly which images are constantly being updated and why. Andrew, any ideas what might cause these images to constantly update, or hints on where I should start to investigate?

Once we work out what's going on here and fix those, we can then consider the less frequently occurring invalidations and see if those are bugs as well.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

For raster images, we can update_image_template in two cases:

• During a decode, it will issue an update with a dirty rect as it decodes more rows of the image. This rarely is seen as most images decode pretty quickly.
• During an animation, it will issue an update on each frame change with a dirty rect. It also changes the external image ID which backs the image key (it is cycling through a set of buffers that it reuses on frame updates).

So probably an animated image in there. Fixing bug 1650378 would help with either case.

Comment 7

[Glenn Watson [:gw]]

Un-assigning for now, as Jeff is going to take a look at the image updates first (which appear to be inline svg blobs). Once we know more there, we can re-assign based on whatever the next steps are to fix this.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Attachment: A reduced test case

When running the reduced test case I don't get continual update_image_template calls but I still get invalidation on scroll. Tileview suggests that it's prim_clip_rect changes causing the invalidation:

Content: Descriptor changed for uid 292057776418

    prim_clip_rect changed from 2658,153.84264 -> 2688,251.84264 to 2658,153.68889 -> 2688,251.68889
    Item: PrimKey { common: PrimKeyCommonData { flags: IS_BACKFACE_VISIBLE, prim_rect: RectangleKey { x: 2658.0, y: 0.0, w: 30.0, h: 98.0 } }, kind: Image { key: ImageKey(IdNamespace(6), 7), stretch_size: SizeKey { w: 30.0, h: 98.0 }, tile_spacing: SizeKey { w: 0.0, h: 0.0 }, color: ColorU { r: 255, g: 255, b: 255, a: 255 }, image_rendering: Auto, alpha_type: PremultipliedAlpha } }

gw, is this still something that we'd expect to be caused on the Gecko side?

Comment 9

[Jeff Muizelaar [:jrmuizel]]

It looks like Gecko is mostly not sending new display lists when scrolling so it feels like this probably a problem on the WebRender side.

Comment 10

[Glenn Watson [:gw]]

That does seem likely to be WR related, unless there are some accuracy issues coming in the external scroll offsets being supplied, or APZ sampling is causing this. I'll take a look at this reduced test case to see what the root cause is, thanks!

Comment 11

[Glenn Watson [:gw]]

Attachment: Another reduced test case

A further reduced test case that still invalidates on scroll. The background-attachment: fixed looks mighty suspicious here, but still need to investigate further.

Comment 12

[Glenn Watson [:gw]]

Attachment: 0001-WIP-Remove-code-that-avoids-creating-slices-for-para.patch

The main issue is that there is a fixed position gradient background that covers the viewport. Since this is included in the main scrolling slice, we invalidate every time we scroll on most of the content.

The attached patch removes the change that skips creating a slice in this case, and with it applied there isn't much invalidation occurring when I scroll (mostly around areas where the image updates are occurring).

The code in the patch I removed was originally intended to keep subpixel AA enabled in some of these parallax scrolling cases. But, I think the general case makes sense to create slices for these. We have the gfx.webrender.quality.force-subpixel-aa-where-possible config setting for users who really want to pay the performance cost to ensure that subpixel AA is available on parallax / fixed scrolling pages.

I think what we should probably do is:

• Get Jeff to confirm that this patch fixes the main invalidation issue.
• Land this patch in nightly and let it bake for a few weeks - to see if we get reports of quality / perf issues on other pages (and if so, find a better way to fix them than the hack in this patch). This would probably mean waiting until the merge is done in a week or so before landing this though.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Asif, how well does this page perform in Chrome/Edge for you?

Comment 14

[Glenn Watson [:gw]]

One option suggested by Matt is to make this be a fixed position slice, so that we don't need to redraw the gradient each frame at least.

Comment 15

[Asif Youssuff]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #13)

Asif, how well does this page perform in Chrome/Edge for you?

Jeff, Edge is not great either - it janks while scrolling a decent amount. Firefox is smoother to scroll when at its best, but checkerboards more often, especially initially.

Comment 16

[Glenn Watson [:gw]]

This page seems to run really well for me now on nightly. The picture caching seems to work, without drawing the full-screen gradient. I see CPU times ~2.3ms and GPU times ~0.8ms on my local environment. Is this much better on your local machine now too?

Comment 17

[Asif Youssuff]

Glenn, the page seems better than before, and I would have no complaints with an ad blocker enabled.

Without an ad blocker, however things aren't as good - here's a profile: https://share.firefox.dev/3RRWUxj

I uploaded the page as it existed when I profiled the page: https://drive.google.com/file/d/1rXpeeh9U0m6NxsUknAYFEAPCaQG_frNj/view?usp=sharing

If there isn't much interesting in the profile, I suppose we can mark this solved, since things seem better.