1632249 - WebGL compositing refactor (for out-of-process)

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, enhancement, P1)

Description

[Kelsey Gilbert [:jgilbert]]

One of the deep assumptions of in-process-webgl was about how backbuffers were presented to the compositor. Out-of-process breaks many assumptions, so we need to rework our approach here.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Support compositing out-of-process WebGL.

• Majorly simplity CanvasRenderer
• Replace GLScreenBuffer with trivial GLSwapChain
• Use descriptor structs so that future SharedSurface changes aren't so painful
  to propagate
• Mortgage/strip out more OffscreenCanvas code for now

Comment 2

[Kelsey Gilbert [:jgilbert]]

Not bad for a first try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfa2ccabd9d5c4a5a18ab97dba32e44878feb5ed

Comment 3

[Kelsey Gilbert [:jgilbert]]

Mochitests green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e85cfc88c3886da2672d5550aec8c2f224e438c3

Comment 4

[Kelsey Gilbert [:jgilbert]]

Talos motionmark-gl runs look good too, though the reftests should show some breakage still.

Comment 5

[Kelsey Gilbert [:jgilbert]]

Win10 WR R2 is thei only new failure I see:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=976eadf185158bd80d2d8175d54478354e40c5cd

vs merge-base:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=02e94d1d140b542c41ccc49d640f74ab56f4bb5c

I haven't been able to repro it locally on my machines so far. Possible speculative fix by reverting supposedly-unrelated change:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c13242b6f855e55f7c6a2c0beaca53266960f098

Comment 6

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - WebXR compositing fixes.

Comment 7

[Imanol Fernandez]

Attachment: Bug 1632249 - Create a separate WebGLContext::PresentInto function for WebXR.

Comment 8

[Imanol Fernandez]

Attachment: Bug 1632249 - Enable WebGL SwapChain pooling on Android. Use a separate SwapChain for WebVR.

Some hack to add pooling and measure performance on Android. Not refactored for production yet.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - NON_PREMULT TextureClient iff NON_PREMULT CanvasClient.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bbfe23c61add
Support compositing out-of-process WebGL. r=handyman,lsalzman,nical,geckoview-reviewers,agi,imanol
https://hg.mozilla.org/integration/autoland/rev/16d84439756f
WebXR compositing fixes. r=imanol
https://hg.mozilla.org/integration/autoland/rev/9ff26bcc580c
Create a separate WebGLContext::PresentInto function for WebXR. r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/cdaa8a4e9e36
NON_PREMULT TextureClient iff NON_PREMULT CanvasClient. r=handyman

Comment 11

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Fix Bb failures.

/builds/worker/checkouts/gecko/gfx/gl/SharedSurface.cpp:141:1: error: control reaches end of non-void function [-Werror=return-type]

Old compilers don't realize the switch being exhaustive means the
function always returns, which we workaround here.

Comment 12

[Dorel Luca [:dluca]]

Backed out 4 changesets (bug 1632249) for Build bustages and mda failures. CLOSED TREE

Logs:

Build bustage:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305552636&repo=autoland&lineNumber=21981

mda:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305555857&repo=autoland&lineNumber=2255

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=eTPqevlfS9e-aV7lYo6v3g-0&revision=cdaa8a4e9e366ad041b429a8f5bd0b9b259d4722

Backout:
https://hg.mozilla.org/integration/autoland/rev/4b54c176764ecb8e0c9576687087d4c75bd1da4f

Comment 13

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Mark mCapturedFrameInvalidated even if mIsCanvasDirty.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd00e2da3a0f
Support compositing out-of-process WebGL. r=handyman,lsalzman,nical,geckoview-reviewers,agi,imanol
https://hg.mozilla.org/integration/autoland/rev/6e9a89ead3a5
WebXR compositing fixes. r=imanol
https://hg.mozilla.org/integration/autoland/rev/71ad2ed8e9ba
Create a separate WebGLContext::PresentInto function for WebXR. r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/b15d91e64a25
NON_PREMULT TextureClient iff NON_PREMULT CanvasClient. r=handyman
https://hg.mozilla.org/integration/autoland/rev/04f5127c85d5
Fix Bb failures. r=gw
https://hg.mozilla.org/integration/autoland/rev/c93972b05d4f
Mark mCapturedFrameInvalidated even if mIsCanvasDirty. r=handyman

Comment 15

[Alexandru Michis [:malexandru]]

Backed out 6 changesets for causing bustages in CanvasRenderingContext2D.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/c7dbcbcbb07745ad124640bc639c86a251c66300

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=hazard&tochange=c7dbcbcbb07745ad124640bc639c86a251c66300&fromchange=c93972b05d4f105953213185bd4e0da2403dedd1&selectedTaskRun=KgnTCag0TbKBqMwBK4cSDA-0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305867365&repo=autoland&lineNumber=34974

Comment 16

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Remove unused var g2DContextLayerUserData.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/204b899f436d
Support compositing out-of-process WebGL. r=handyman,lsalzman,nical,geckoview-reviewers,agi,imanol
https://hg.mozilla.org/integration/autoland/rev/7c94d37c446e
WebXR compositing fixes. r=imanol
https://hg.mozilla.org/integration/autoland/rev/073302d26c5e
Create a separate WebGLContext::PresentInto function for WebXR. r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/6f19f43e0a0b
NON_PREMULT TextureClient iff NON_PREMULT CanvasClient. r=handyman
https://hg.mozilla.org/integration/autoland/rev/64fbb616a0f3
Fix Bb failures. r=gw
https://hg.mozilla.org/integration/autoland/rev/d5b7fe789001
Mark mCapturedFrameInvalidated even if mIsCanvasDirty. r=handyman
https://hg.mozilla.org/integration/autoland/rev/4ff99aab3ee8
Remove unused var g2DContextLayerUserData.

Comment 18

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/204b899f436d
https://hg.mozilla.org/mozilla-central/rev/7c94d37c446e
https://hg.mozilla.org/mozilla-central/rev/073302d26c5e
https://hg.mozilla.org/mozilla-central/rev/6f19f43e0a0b
https://hg.mozilla.org/mozilla-central/rev/64fbb616a0f3
https://hg.mozilla.org/mozilla-central/rev/d5b7fe789001
https://hg.mozilla.org/mozilla-central/rev/4ff99aab3ee8

Comment 19

[Cristina Coroiu [:ccoroiu]]

Backed out 7 changesets (Bug 1632249) for Gtest perma chrash in [@ mozilla::BlockingResourceBase::CheckAcquir

Backout: https://hg.mozilla.org/integration/autoland/rev/3a722303e8853eb8b88422f151d3d12dfd6234bc

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4ff99aab3ee8bc2310e49d8db7032b5ede0490e2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305961758&repo=autoland&lineNumber=26464

Comment 20

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Replace GLContextProvider::CreateOffscreen with GLContext::CreateOffscreenDefaultFb.

Comment 21

[Markus Stange [:mstange]]

It seems that this patch set severely regresses macOS WebGL performance - it causes WebGL to use readback in the content process.
The patch set made it into the current Nightly. I discovered this because my virtu-allhands experience was not great.

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged: https://hg.mozilla.org/mozilla-central/rev/3a722303e885

Comment 23

[Jamie Nicol [:jnicol]]

(In reply to Markus Stange [:mstange] from comment #21)

It seems that this patch set severely regresses macOS WebGL performance - it causes WebGL to use readback in the content process.
The patch set made it into the current Nightly. I discovered this because my virtu-allhands experience was not great.

It appears to have the same effect on Android, too, by making us use a SharedSurface_Basic instead of a SharedSurface_SurfaceTexture. It appears we now select the SurfaceFactory type based on the "preferred texture type", which is basic on android. Setting gfx.use-surfacetexture-textures = true makes us use SurfaceTextures for canvas again, and it appears to work fine. However, we don't want to do that as it makes us use SurfaceTextures for painted content. We need to distinguish between the preferred texture type for CPU-painted content and for GPU canvas.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

FYI, we also saw a Nightly [@ mozilla::layers::CanvasClient::UseTexture ] crash spike that looks related to this bug:
https://crash-stats.mozilla.org/report/index/28b7aa44-07a7-4bb2-ab95-7e2130200612

Comment 25

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Determine webgl's layers::TextureType in Present.

Comment 26

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Check weakptr value before deref in gl::Texture/Renderbuffer.

Comment 27

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1632249 - Disable now-failing DXGL test for now.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16c5dd2cfd60
Support compositing out-of-process WebGL. r=handyman,lsalzman,nical,geckoview-reviewers,agi,imanol
https://hg.mozilla.org/integration/autoland/rev/0b191b707750
WebXR compositing fixes. r=imanol
https://hg.mozilla.org/integration/autoland/rev/4cda148ef28f
Create a separate WebGLContext::PresentInto function for WebXR. r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/fd4a74ec97a5
NON_PREMULT TextureClient iff NON_PREMULT CanvasClient. r=handyman
https://hg.mozilla.org/integration/autoland/rev/8f0fb988dc5a
Fix Bb failures. r=gw
https://hg.mozilla.org/integration/autoland/rev/2739940a4fb7
Mark mCapturedFrameInvalidated even if mIsCanvasDirty. r=handyman
https://hg.mozilla.org/integration/autoland/rev/e326157c5af9
Remove unused var g2DContextLayerUserData.
https://hg.mozilla.org/integration/autoland/rev/8439f89e170b
Replace GLContextProvider::CreateOffscreen with GLContext::CreateOffscreenDefaultFb. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/05896dbeb434
Determine webgl's layers::TextureType in Present. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/832f39c3b02c
Check weakptr value before deref in gl::Texture/Renderbuffer. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/567a8768593e
Disable now-failing DXGL test for now.

Comment 29

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/16c5dd2cfd60
https://hg.mozilla.org/mozilla-central/rev/0b191b707750
https://hg.mozilla.org/mozilla-central/rev/4cda148ef28f
https://hg.mozilla.org/mozilla-central/rev/fd4a74ec97a5
https://hg.mozilla.org/mozilla-central/rev/8f0fb988dc5a
https://hg.mozilla.org/mozilla-central/rev/2739940a4fb7
https://hg.mozilla.org/mozilla-central/rev/e326157c5af9
https://hg.mozilla.org/mozilla-central/rev/8439f89e170b
https://hg.mozilla.org/mozilla-central/rev/05896dbeb434
https://hg.mozilla.org/mozilla-central/rev/832f39c3b02c
https://hg.mozilla.org/mozilla-central/rev/567a8768593e

Comment 30

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Pulsebot from comment #17)

https://hg.mozilla.org/integration/autoland/rev/c93972b05d4f
Mark mCapturedFrameInvalidated even if mIsCanvasDirty. r=handyman

== Change summary for alert #26188 (as of Thu, 11 Jun 2020 21:41:47 GMT) ==

Regressions:

221% glterrain macosx1014-64-shippable opt e10s stylo 2.92 -> 9.39

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26188

Comment 31

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Alexandru Michis [:malexandru] from comment #15)

Backed out 6 changesets for causing bustages in CanvasRenderingContext2D.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/c7dbcbcbb07745ad124640bc639c86a251c66300

== Change summary for alert #26230 (as of Tue, 16 Jun 2020 03:43:13 GMT) ==

Improvements:

35% glterrain macosx1014-64-shippable opt e10s stylo 4.58 -> 2.96

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26230

Comment 32

[Alexandru Ionescu (needinfo me) [:alexandrui]]

deleted