Closed Bug 1569724 Opened 5 months ago Closed 4 months ago

Canvas redraw doesn't update on screen after made hidden, then visible

Categories

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

Desktop
All
defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 --- disabled
firefox-esr68 --- disabled
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- verified

People

(Reporter: ori, Assigned: sotaro)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Tested on latest Nightly with WebRender enabled.

The attached testcase hides and shows a canvas. Clicking the canvas redraws it.
After the canvas is visible again, the next draw operation(s) is not displayed, as if nothing happens.
Subsequent draw operations after that do appear correctly.

This only happens with WebRender enabled, and with a canvas element (e.g. doing the same thing with a div works correctly).
Hiding and showing the canvas in the testcase is done by setting the "visibility" property, but the same happens by changing the "display" property, or using other tricks such as calling "replaceWith()".

Debian Testing, KDE, X11

mozregression --good 2018-01-15 --bad 2018-06-15 --pref gfx.webrender.all:true browser.tabs.drawInTitlebar:true browser.startup.homepage:'https://bug1569724.bmoattachments.org/attachment.cgi?id=9081359'

4:29.76 INFO: Last good revision: 4d5c3fa215602dd850bf4a994e19afc2065d74b7 (2018-02-12)
4:29.76 INFO: First bad revision: e43f2f6ea111c2d059d95fa9a71516b869a69698 (2018-02-13)
4:29.76 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4d5c3fa215602dd850bf4a994e19afc2065d74b7&tochange=e43f2f6ea111c2d059d95fa9a71516b869a69698

5:04.51 INFO: There are no build artifacts on inbound for these changesets (they are probably too old).

Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → Desktop

In the regression range, a likely culprit may be bug 1237454: "Optimize animations in visibility:hidden elements".

Hiro, can you take a look to see if Bug 1237454 might have an impact here?

Flags: needinfo?(hikezoe.birchill)

Yeah, I will take a look of course, but I don't think bug 1237454 caused this because it affects only animations. The issue looks like more hit-testing issue (on WebRender), so this commit maybe?

Jeff - I see you were the reviewer for the commit Hiro linked to above, not sure if that might be related somehow?

Blocks: wr-69
Flags: needinfo?(jmuizelaar)
Priority: -- → P3

Confirmed that the revision right before bug 1436058 doesn't have the issue and the last commit of bug 1436058 has the issue.

Flags: needinfo?(hikezoe.birchill)
Regressed by: 1436058
Flags: needinfo?(nical.bugzilla)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)

and the last commit of bug 1436058 has the issue.

The patch was originally reviewed at Bug 1432708

Regressed by: 1432708

I want to take a bug.

Assignee: nobody → sotaro.ikeda.g
Flags: needinfo?(nical.bugzilla)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)

Confirmed that the revision right before bug 1436058 doesn't have the issue and the last commit of bug 1436058 has the issue.

The above was related to a root cause. AsyncImagePipelineManager::UpdateImageKeys() requests to use different TextureHost for updating rendering. And PersistentBufferProviderShared does not work well with AsyncImagePipelineManager and WR in the user case.

When TextureClient is managed by PersistentBufferProviderShared, AsyncImagePipelineManager expects that read lock of TextureHost is locked during rendering. Read lock prevents that TextureClient is reused during rendering. But when the problem happened, the read lock was not held.

When the canvas became hidden, WebRenderCanvasRendererAsync was destroyed and then the read lock of TextureHost was released. When the canvas became visible again, WebRenderCanvasRendererAsync was re-created and the previous TextureClient/TextureHost was rendered as initial content. But its read lock was not held. At a next canvas update, the same TextureClient wa used for canvas rendering, since it was not read locked. It broke AsyncImagePipelineManager's assumption.

Pushed by sikeda.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5cd0b643ffd2
Do not use front TextureClient as next back buffer when PersistentBufferProviderShared::ClearCachedResources() is called r=nical
Flags: needinfo?(jmuizelaar)

The problem does not happen on Windows. PersistentBufferProviderShared is not used when compositor supports texture direct mapping. See PersistentBufferProviderShared::Create().
https://searchfox.org/mozilla-central/source/gfx/layers/PersistentBufferProvider.cpp#106

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Is this something we should consider uplifting to Beta for Fx69 or can this ride with Fx70 to release?

Flags: needinfo?(sotaro.ikeda.g)

The problem does not happen on Windows, then we could stay on Fx70.

Flags: needinfo?(sotaro.ikeda.g)
Flags: qe-verify+

I have managed to reproduce the reproduce the issue using Fx 70.0a1 (2019-07-22), I can confirm that the issue is fixed on Fx 70.0b12.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.