Closed Bug 1586696 Opened 5 years ago Closed 5 years ago

[Wayland] Use wayland dmabuf as WebGL backend

Categories

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

Product:
Core
Component:
Graphics: CanvasWebGL
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla74
Tracking Flags:
Tracking Status
firefox74 --- fixed

People

(Reporter: stransky, Assigned: stransky)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

webgl-dmabuf patch
(deleted), patch
jgilbert
: feedback+
Details | Diff | Splinter Review
Bug 1586696 [Wayland] Use wayland dmabuf as WebGL backend, r?jgilbert
(deleted), text/x-phabricator-request
Details

We can use wayland dmabuf surface as target for WebGL rendering to avoid readbacks.

Priority: -- → P3

GLContextEGL::CreateEGLPBufferOffscreenContext() is a point where we should start.

Depends on: 1608380
Attached patch webgl-dmabuf patch (deleted) — — Splinter Review

Hi Sotaro,

there's a WIP dmabuf backend patch for WebGL, I see 100% performance boost with it for simple WebGL samples at GL compositor (it's even faster than chrome/chromium on my box). It implements shared surfaces backed by dmabuf surface.

But I have some questions about it:

  • Is is better to export export texture or framebuffer by shared surface? (dmabuf has both)
  • What does mean a surface can be recycled? (a param at SharedSurface::SharedSurface()). Is that important and what is needed for that?
  • I'm unable to decode what are the lock/unlock methods used for as there isn't any documentation for it:
    LockProdImpl()/UnlockProdImpl()
    ProducerAcquireImpl()/ProducerReleaseImpl()
    ProducerReadAcquireImpl()/ProducerReadReleaseImpl()
    So do I need to implement them and do we need to implement commit?
  • WebRender() - when I use this patch with webrender I get GL errors although it generally works somehow. I wonder if that's due to missing sync or something. I'll look at it later.

Thanks.

Assignee: nobody → stransky
Attachment #9120077 - Flags: feedback?(sotaro.ikeda.g)

(In reply to Martin Stránský [:stransky] from comment #2)

But I have some questions about it:

  • Is is better to export export texture or framebuffer by shared surface? (dmabuf has both)

Could you clarify more about the question?

  • What does mean a surface can be recycled? (a param at SharedSurface::SharedSurface()). Is that important and what is needed for that?

Resource of SharedSurface could be expensive to allocate. Then we could get better performance if we recycle SharedSurface. Itis stored in SharedSurfaceTextureClient and its content is forwarded to compositor. It is recycled at SurfaceFactory::RecycleCallback() and mCanRecycle is checked there.
https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/gfx/gl/SharedSurface.cpp#335

Recycle timing is determined by Host side. When CompositableTextureHostRef is released to 0 on host side,TextureHost::NotifyNotUsed() is called. and "notify not used" event is delivered to client side and SurfaceFactory::RecycleCallback() is called.

During the forwarding, SharedSurfaceTextureData::Serialize() calls SharedSurface::ToSurfaceDescriptor().
https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/gfx/layers/client/TextureClientSharedSurface.cpp#39

SharedSurfaceTextureClient is allocated or re-used at SurfaceFactory::NewTexClient()
https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/gfx/gl/SharedSurface.cpp#287

On wayland, the callback might be called too early, because gecko does not handle the wait gpu task end on linux. If it happens, SharedSurface is re-used even during GPU usage and causes a flickering.

With WebRender, AsyncImagePipelineManager defers releasing compositable ref of TextureHost at AsyncImagePipelineManager::CheckForTextureHostsNotUsedByGPU(). But RenderCompositorEGL does not have related implementation. RenderCompositorANGLE does it by RenderCompositorANGLE::WaitForPreviousGraphicsCommandsFinishedQuery() and RenderedFrameId RenderCompositorANGLE::GetLastCompletedFrameId().

On D3D compositor case, WaitForFrameGPUQuery() limit a number of ongoing gpu task. With it, the D3D compositor avoids the problem.

  • I'm unable to decode what are the lock/unlock methods used for as there isn't any documentation for it:
    LockProdImpl()/UnlockProdImpl()
    ProducerAcquireImpl()/ProducerReleaseImpl()
    ProducerReadAcquireImpl()/ProducerReadReleaseImpl()
    So do I need to implement them and do we need to implement commit?

It seems better to be answered by :jgilbert. :jgilbert, can you answer the questions?

Flags: needinfo?(jgilbert)
Comment on attachment 9120077 [details] [diff] [review]
webgl-dmabuf patch

About GLScreenBuffer and SharedSurface_DMABUF,it seems better to feedback by :jgilbert.
:jgilbert, can you feedback to the patch?
Attachment #9120077 - Flags: feedback?(sotaro.ikeda.g) → feedback?(jgilbert)

Thanks a lot for the feedback!

Bug 1608380 landed so this patch can be applied/tested on trunk, you need to enable it by widget.wayland_dmabuf_webgl.enabled pref.

Bug 1608800 is also related and when lands we can create dmabuf framebuffer/textures with modifiers. It should improve dmabuf performance for surfaces/textures used exclusively by GPU, like WebGL framebuffer.

Comment on attachment 9120077 [details] [diff] [review]
webgl-dmabuf patch

Review of attachment 9120077 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/SharedSurfaceEGL.h
@@ +15,5 @@
>  #  include "AndroidNativeWindow.h"
>  #endif
>  
> +#ifdef MOZ_WAYLAND
> +#  include "mozilla/widget/WaylandDMABufSurface.h"

Prefer forward declarations instead of ifdefs.

@@ +193,5 @@
>  };
>  
>  #endif  // MOZ_WIDGET_ANDROID
>  
> +#ifdef MOZ_WAYLAND

Avoid ifdefs.
Attachment #9120077 - Flags: feedback?(jgilbert) → feedback+

(In reply to Martin Stránský [:stransky] from comment #2)

Created attachment 9120077 [details] [diff] [review]
webgl-dmabuf patch

Hi Sotaro,

there's a WIP dmabuf backend patch for WebGL, I see 100% performance boost with it for simple WebGL samples at GL compositor (it's even faster than chrome/chromium on my box). It implements shared surfaces backed by dmabuf surface.

But I have some questions about it:

  • Is is better to export export texture or framebuffer by shared surface? (dmabuf has both)

Texture is better, if GL can naively wrap the texture in its own framebuffer for RTT.

  • What does mean a surface can be recycled? (a param at SharedSurface::SharedSurface()). Is that important and what is needed for that?

We avoid reallocating surfaces for two reasons:

  • glGetError is a pipeline flush, but we need to use it to check for OOM/failure to allocate.
  • Framebuffer completeness should be cached/reused instead of making temporary framebuffers and requiring revalidation.
  • I'm unable to decode what are the lock/unlock methods used for as there isn't any documentation for it:
    LockProdImpl()/UnlockProdImpl()
    ProducerAcquireImpl()/ProducerReleaseImpl()
    ProducerReadAcquireImpl()/ProducerReadReleaseImpl()
    So do I need to implement them and do we need to implement commit?

Yep, we need those for synchronization. Check out other SharedSurface implementations to see what these are for:
https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/gfx/gl/SharedSurfaceANGLE.cpp#111

LockProd(ucer) is exclusive:
https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/gfx/gl/SharedSurface.cpp#63

ProducerAcquire/Release is Acquired before WebGL renders into a surfaces, and Released at the end of a WebGL frame. Here's the example implementation for stock GL:
https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/gfx/gl/SharedSurfaceGL.cpp#127

ProducerReadAcquire/Release are when the producer (like WebGL, generally content) needs to read from the surface, but isn't writing to it.

  • WebRender() - when I use this patch with webrender I get GL errors although it generally works somehow. I wonder if that's due to missing sync or something. I'll look at it later.

Just don't enable it until it works without errors.

Flags: needinfo?(jgilbert)

I'm going to need to audit all GL code that landed leading up to this without GFX peer or WebGL peer review.

(In reply to Jeff Gilbert [:jgilbert] from comment #9)

I'm going to need to audit all GL code that landed leading up to this without GFX peer or WebGL peer review.

Great, the widget part (DMABuf surface implementation) is here:

https://searchfox.org/mozilla-central/source/widget/gtk/WaylandDMABufSurface.cpp
https://searchfox.org/mozilla-central/source/widget/gtk/WaylandDMABufSurface.h

and it's used by GL/WebRender:

gfx/layers/opengl/WaylandDMABUFTextureClientOGL.cpp
gfx/layers/opengl/WaylandDMABUFTextureClientOGL.h
gfx/layers/opengl/WaylandDMABUFTextureHostOGL.cpp
gfx/layers/opengl/WaylandDMABUFTextureHostOGL.h
gfx/webrender_bindings/RenderWaylandDMABUFTextureHostOGL.cpp
gfx/webrender_bindings/RenderWaylandDMABUFTextureHostOGL.h

I see that some shared surfaces are created as AttachmentType::GLTexture/GLRenderbuffer and someone as AttachmentType::Screen. Do I understand correctly that GLTexture/GLRenderbuffer are rendered into/shared and then used as source for subsequent compositing but 'Screen' surfaces can be directly attached as GL front buffer. There may not be any extra composition cost a 'Screen' shared surface but it needs rigorous synchronization. Am I correct? And is it better to implement the surface as GLTexture or Screen? (dmabuf surfaces can be used as GL front buffer).

Flags: needinfo?(jgilbert)

GLTexture is ideal, avoid Screen attachment. That's for surfaces which it's only possible to render to by overriding FB-zero, which is generally a huge pain, and is generally less efficient. It's not actually the monitor-screen, but rather the GL context's virtual "screen".

Flags: needinfo?(jgilbert)
Component: Graphics → Canvas: WebGL
  • Create new SharedSurfaceType called EGLSurfaceDMABUF
  • Implement EGLSurfaceDMABUF by SharedSurfaceDMABUF which is backed by WaylandDMABufSurface.
    It can be used by Wayland/EGL WebGL backend.
  • It's disabled by default, can be enabled by widget.wayland_dmabuf_webgl.enabled pref.

Thanks. The patch here works fine with both GL compositor and WebRender. I see similar or slightly better performance than Chrome on Wayland/EGL.

Jeff, thanks for looking at it. The webrender breakage was caused by double-free of textures by GL texture host which is covered by Bug 1613052.

Depends on: 1613052
Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f788224c8d2
[Wayland] Use wayland dmabuf as WebGL backend, r=jgilbert

https://hg.mozilla.org/mozilla-central/rev/4f788224c8d2

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox74: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

With Bug 1614568 checked in it makes sense to test this one. I see rapid WebGL improvement with GL compositor on Wayland when widget.wayland_dmabuf_webgl.enabled is set.

Depends on: 1614568
Depends on: 1618031

(In reply to Martin Stránský [:stransky] from comment #19)

With Bug 1614568 checked in it makes sense to test this one. I see rapid WebGL improvement with GL compositor on Wayland when widget.wayland_dmabuf_webgl.enabled is set.

Hello everyone,
i've followed the activation procedure for wayland dmabuf backend for webgl on Martin's blog(https://mastransky.wordpress.com/2020/03/03/webgl-and-fgx-acceleration-on-wayland/)
But as soon as I set widget.wayland-dmabuf-webgl.enabled and open a webgl demo . My tab crash
If I reset widget.wayland-dmabuf-webgl.enabled to false. It's working again.

This is my crash report : https://crash-stats.mozilla.org/report/index/239ff676-78c6-471d-abd0-2e1c10200310#tab-metadata
Do you something else about this issue ?

(In reply to GreyXor from comment #20)

(In reply to Martin Stránský [:stransky] from comment #19)
This is my crash report : https://crash-stats.mozilla.org/report/index/239ff676-78c6-471d-abd0-2e1c10200310#tab-metadata
Do you something else about this issue ?

Please file a new bug about it.
Thanks.

(In reply to Martin Stránský [:stransky] from comment #21)

(In reply to GreyXor from comment #20)

(In reply to Martin Stránský [:stransky] from comment #19)
This is my crash report : https://crash-stats.mozilla.org/report/index/239ff676-78c6-471d-abd0-2e1c10200310#tab-metadata
Do you something else about this issue ?

Please file a new bug about it.
Thanks.

This is the issue about this problem
https://bugzilla.mozilla.org/show_bug.cgi?id=1624290

Depends on: 1624441
No longer depends on: 1624441
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: