Closed Bug 1663355 Opened 4 years ago Closed 4 years ago

Tab gets visually corrupted on Mali-G76

Categories

(Core :: Graphics: WebRender, defect)

ARM64
Android
defect

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox81 --- wontfix
firefox82 --- fixed

People

(Reporter: mobd0, Assigned: jnicol)

References

(Blocks 2 open bugs, Regression)

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Android 10; Mobile; rv:82.0) Gecko/82.0 Firefox/82.0

Steps to reproduce:

  • On Firefox Nightly, have the google search fixer addon enabled
  • Look something up on google images and filter for gif results
  • Scroll around on the page, click on some images

Actual results:

The page is displayed incorrectly, with random blocks and images all over the page. This state is persisted even on reloads, clicking on links, or going to a previous tab. I've also managed to reproduce (but not consistently) on reddit and some other sites.

Expected results:

The page behaves normally.

OS: Unspecified → Android
Hardware: Unspecified → ARM64
Version: unspecified → Trunk

Device is a Huawei Nova 6, Kirin 990, Mali-G76

Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: wr-mali
Summary: Tab gets visually corrupted → Tab gets visually corrupted on Mali-G76

Thanks for the report! I can also reproduce on my Samsung S9 (Mali-G72), so presumably it affects all Mali-Gxx devices.

Is it only in GIF mode you can reproduce, but not regular image search? I think the issue might only occur because the images are changing at the same time as the scroll changes. It would be useful to know which other sites specifically you've seen this on, and whether you have a consistent steps to reproduce on reddit or any others.

Could you also please verify for me that when you have reproduced, minimising the app and then opening it again fixes it? And if you go to about:config and set gfx.webrender.picture-caching to false, then kill the app and restart it, does it reproduce at all? Thanks!

Flags: needinfo?(mobd0)

Hi,

I can't reproduce on regular image search, no. The problem only occurs with playing videos for me (gif previews on google also seem to be videos). I think on reddit the issue happened when I clicked on a video, played it and went back again, after that scrolling resulted in those weird artifacts. However I wasn't able to make it happen again when I tried. The other website I noticed it on was an adult site, I'm not sure if I should post it here.

Minimising the app doesn't help at all when I try it now, but I do remember it fixing it once before. If I change gfx.webrender.picture-caching to false I can't reproduce anymore.

Flags: needinfo?(mobd0)
Severity: -- → S2

I managed to run mozregression (setting prefs to enable webrender and to set the user agent correctly to reproduce), and found that https://phabricator.services.mozilla.com/D61424 introduced the issue. The relevant part of that change is that we changed from intersecting local_dirty_rect with local_tile_rect to intersecting it with local_valid_rect. This affects the scissor rect that we set, eg the 2nd column of tiles will now have a 56x512 scissor rect rather than 1024x512 (on a 1080px wide screen assuming the full tile is invalidated).

I suspect this is probably the same underlying issue as bug 1558374, and that workaround we applied there (using a shader to clear instead of using a scissored glClear) only happened to worked on fairly static pages, which this page is not.

What I suspect is going on is that there is a bug in the Mali driver, where scissor rects confuse its logic that determines whether a tile has been modified and gets written back to memory. In bug 1558374, it was as if the scissored-clear didn't work on some tiles that were otherwise not rendered to. Using a shader instead of glclear appeared to fix it. But in this bug, it's as if rendering to the tile doesn't work at all since it's not been cleared first.

I think we can workaround this properly by reverting bug 1558374 (I don't think it does any harm, but isn't necessary), and ensuring that we always fully clear and re-render picture cache tiles. We can do this by setting supports_dirty_rects = false in Tile::post_update(), so that the dirty region is never a subportion of tile, and by glClear()ing the entire tile (without a scissor rect set) in Renderer::draw_picture_cache_target().

It might not even be so bad in terms of performance: yes we'll have to render entire picture cache tiles rather than smaller regions, but Mali will be able to avoid reading the textures into tile memory due to the full clear. While that is true, without this bug Mali could also presumably omit writing most of the tiles back to memory, if only a few were modified. So this probably won't save much memory bandwidth, and might well hurt performance. But it's the only way I can see to workaround it, so that's that.

Has Regression Range: --- → yes

On Mali-Gxx there is a driver bug which causes partial updates to offscreen
render targets to fail. This was originally encountered in bug 1558374, where we
thought that the problem was just to do with scissored glClear()s, so we used a
shader to clear the target instead of glClear(). On some sites, however, even
this is not enough, and sometimes renderering to the target fails leaving some
of the previous content in place.

We appear to be able to work around this by ensuring that the entire render
target is cleared, by calling glClear() with the scissor test disabled. This
means that for picture cache tiles we must ensure the entire valid region is
rendered. This patch also reverts the first attempt at a fix from bug 1558374,
as it is no longer necessary since the entire target is being cleared.

Assignee: nobody → jnicol
Status: NEW → ASSIGNED
Pushed by jnicol@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a0de15f51cff Always clear and render entire picture cache tiles on Mali-Gxx. r=gw,geckoview-reviewers,agi
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: