Jeff Muizelaar [:jrmuizel] Reporter
	Description • 13 years ago

Having UseClientSideRendering true will make this happen.

	Frederic Plourde (:fred23) Assignee
	Comment 1 • 10 years ago

Hey, just found out about this bug !
It's really related to Bug 496204 , if not a complete dup
Also, WIP patch already submitted does exactly that :
https://bug496204.bugzilla.mozilla.org/attachment.cgi?id=8402672

	Frederic Plourde (:fred23) Assignee
	Comment 2 • 10 years ago

Patch is still WIP, the XShm/XImage are not finished yet, still have to find the perfect place for it, but it gives an insight into the whole idea.

	George Wright (:gw280) (needinfo me!)
	Comment 3 • 10 years ago

Related: bug 738937

	Frederic Plourde (:fred23) Assignee
	Comment 4 • 10 years ago

Ok, I've started working on this bug... I have traced the code and checked how it behaved with pref layers.use-offscreen-image-surfaces (from Bug 1015218) turned on and it seems to be behaving just right.

With image surfaces (either drawn to with cairo or skia backend), those give rise to MemoryTextureHosts (when multi-threaded OMTC) or ShmemTextureHosts (when multi-process OMTC) on compositor side and those TextureHosts create TextureImageTextureSourceOGL TextureSources that will create gl textures, bind those textures to upload data content to gpu when appropriate (::Update) and render them to TextureImage target when compositing the different layers. All that's already done! Then, when frame is finished (CompositorOGL::EndFrame), rendered content is readBack from gpu and written on our destination drawTarget (our gtkWidget's xlib drawable *or* the intermediate nsShmImage returned by GetThebesSurface)

Now, as it was the case with OTMC 'basic', I'd be inclined to think that using the nsShmImage ThebesSurfaces as an intermediate step for final compositing *would be detrimental to performance*, because we're basically already using cairo's default XShmPutImage features when copying readBack image data to final widget's xlib drawable... using the nsShmImage here too would just duplicate this step. So I will gather performance numbers and post them here so we can take a decision on that front.

	Karl Tomlinson (:karlt)
	Comment 5 • 10 years ago

(In reply to Frederic Plourde (:fred23) from comment #4)
> I'd be inclined to think that
> using the nsShmImage ThebesSurfaces as an intermediate step for final
> compositing *would be detrimental to performance*,

Yes, indeed.

> when frame is finished
> (CompositorOGL::EndFrame), rendered content is readBack from gpu and written
> on our destination drawTarget (our gtkWidget's xlib drawable *or* the
> intermediate nsShmImage returned by GetThebesSurface)

This sounds like some kind of fill-in implementation because the ideal paths were not completed, or perhaps this is fallback to unintended code paths for some reason.

GL should be used to composite directly to the window and then swap front and back buffers when done.

	Frederic Plourde (:fred23) Assignee
	Comment 6 • 10 years ago

(In reply to Karl Tomlinson (needinfo?:karlt) from comment #5)
> This sounds like some kind of fill-in implementation because the ideal paths
> were not completed, or perhaps this is fallback to unintended code paths for
> some reason.
> 
> GL should be used to composite directly to the window and then swap front
> and back buffers when done.

Now that you say it, of course, it seems pretty wrong (at least perf-wise) to be reading the pixels back from the fbo... I dunno what configuration my build was in when I traced, but it should basically have picked up the GlContextProviderGLX :: CreateForWindow() stuff that makes opengl render directly to the window drawable.. but somehow it did not..

I will check again, thanks

	Frederic Plourde (:fred23) Assignee
	Comment 7 • 10 years ago

Ok, I rechecked with this set of prefs : 

pref("layers.offmainthreadcomposition.force-basic", false);
pref("layers.offmainthreadcomposition.force-enabled", true);
pref("layers.use-image-offscreen-surfaces", false);
pref("layers.acceleration.force-enabled", true

and environment variable MOZ_ACCELERATED=1


...and accelerated compositing works as expected when drawing with the 'cairo' backend on our image surfaces.

It's properly creating a CompositorOGL, correctly initializes the GLContextProviderGLX context (correctly creates the renderTarget FBO pointed towards our main widget's Window via GLContextProviderGLX::CreateForWindow).

Then when layers rendering is done, it's correctly swapping the glx buffers in CompositorOGL::EndFrame() and rendering appears on screen. No alpha-related kinds of visual artefacts noticeable.

Last week, when I tried, I had the "skia backend" enabled and it was taking a fallback path, so I'll investigate why today and report it back here asap.

	Frederic Plourde (:fred23) Assignee
	Comment 8 • 10 years ago

Hey Karl, here's the OMTC OGL program flow I'm currently getting on a GTK3 build patched with 8446913 (Remove_Xlib_Offscreen_Surface v.2) and with those prefs :

pref("layers.offmainthreadcomposition.enabled", true);
pref("layers.acceleration.force-enabled", true);
pref("layers.use-image-offscreen-surfaces", true);

==================================
==================================

Surface creation
++++++++++++++++++++++
gfxPlatform::Init()
 gfxPlatform::GetPlatform()
  gfxPlatformGtk::CreateOffscreenSurface()
   gfxImageSurface::gfxImageSurface()



Compositor creation:
++++++++++++++++++++++
Client:
nsBaseWidget::CreateCompositor()
 CompositorChild::AllocPLayerTransactionChild()
---------------------------------------------------- ipc/ipdl
Server:
CompositorParent::InitializeLayerManager()
 CompositorOGL::CompositorOGL()



Buffer update and layer rendering/compositing
+++++++++++++++++++++++++++++++++++++++++++++
LayerManagerComposite::Render()
 CompositorOGL::BeginFrame()
  mCurrentRenderTarget = CompositingRenderTargetOGL::RenderTargetForWindow()
  mCurrentRenderTarget->BindRenderTarget()
 ContainerLayerComposite::RenderLayer()
  ...
  ContentHostBase::Composite()
   BufferTextureHost::GetTextureSources()
    BufferTextureHost::MaybeUpload()
     BufferTextureHost::Upload()
      CompositorOGL::CreateDataTextureSource()
       TextureImageTextureSourceOGL::TextureImageTextureSourceOGL()
     TextureImageTextureSourceOGL::Update()
      CreateBasicTextureImage()
   ...
   CompositorOGL::DrawQuad()
    BindAndDrawQuadWithTextureRect()
     TextureImageTextureSourceOGL::BindTexture()
     ...
 CompositorOGL::BeginFrame()
  GLContextGLX::SwapBuffers()


==================================
==================================

Looks like to me everything is working as it should here.
Tell me what you think

	Frederic Plourde (:fred23) Assignee
	Comment 9 • 10 years ago

sorry, Comment 8 program flow was for azure.content = 'cairo'

	Frederic Plourde (:fred23) Assignee
	Comment 10 • 10 years ago

Just verified that we're hitting the same flow as above in Comment 8 with azure.content = 'skia'

	Karl Tomlinson (:karlt)
	Comment 11 • 10 years ago

Flow looks OK from what I know.  If things work as expected with brief testing, then check reftest results and performance measurements.

	Frederic Plourde (:fred23) Assignee
	Comment 12 • 10 years ago

argh, seems we're out of luck, 

  The GTK3 try builds with OMTC-ogl all got *failing* talos performance tests :(

The related test failure bugs in bugzilla all look related to non-linux platforms bugs : 
http://pastebin.com/dkrG11Cx

but even when I "limit tests to linux32 + 64" and even if I don't choose android/b2g-only tests from the TryChooser, talos perf tests still fail!

I'll try to get comparison numbers locally for OMTC ogl.

***

Concerning reftests though, it looks quite similar to what we've got so far for regular GTK3 builds on the 'elm' branch...See :

Here's a recent revision on the 'elm' try branch (GTK3 automated try runs) : 
https://tbpl.mozilla.org/?tree=Elm

Here's in turn, GTK3 try builds with :

1) accelerated layers + cairo content
https://tbpl.mozilla.org/?tree=Try&rev=dc2e0ef1af56

2) accelerated layers + skia content
https://tbpl.mozilla.org/?tree=Try&rev=f6179e06170b

3) accelerated layers + skia canvas
https://tbpl.mozilla.org/?tree=Try&rev=43a640658ee4

4) accelerated layers + skia content + canvas
https://tbpl.mozilla.org/?tree=Try&rev=e60747e80c55


It looks quite similar except for JP, bc3 and Me10-s(3+4)

	Frederic Plourde (:fred23) Assignee
	Comment 13 • 10 years ago

Ok, I successfully ran all talos tests on my local machine for OMTC OGL! I'll try to arrange to numbers nicely side by side for easy comparison with non-patched vs. patched with OMTC-ogl (cairo + skia content) and I'll post everything in here shortly.

My intention is too try to push at least the "Remove Xlib Surfaces" patch alone now (the one that already got r+) if it makes OMTC ogl perform better without breaking reftests.

cheers.

	Frederic Plourde (:fred23) Assignee
	Comment 14 • 10 years ago

Ok, would be worth it to get perf numbers for OMTC OGL because this bug might be already fixed by 

https://hg.mozilla.org/integration/mozilla-inbound/rev/6e532c9826e7 from Bug 1015218

Indeed, having that patch applied, I extensively tested the OMTC OGL case and it seemed to work just fine and by tracing the code, I could see that program flow was just going as expected (see Comment 8)
To test, please set those prefs in a try job patch :

pref("layers.offmainthreadcomposition.force-basic", false);
pref("layers.offmainthreadcomposition.force-enabled", true);
pref("layers.use-image-offscreen-surfaces", true);
pref("layers.acceleration.force-enabled", true)

and environment variable MOZ_ACCELERATED=1


At the time of my last comment, I couldn't test that on the try servers because all talos tests were crashing for the OMTC OGL config.

	AnAkkk
	Comment 15 • 10 years ago

Anything new here? :)

	Frederic Plourde (:fred23) Assignee
	Comment 16 • 10 years ago

(In reply to AnAkkk from comment #15)
> Anything new here? :)

nothing new, As I moved to another project, this current thread will be tackled by whoever will be assigned to chasing OMTC OGL bugs on GTK3. But feel free to dump in here anything that could be helpful, thanks.

	Christian Stadelmann
	Comment 17 • 8 years ago

Now that Gtk+ 3 support has landed in release builds on Linux, I've tested this for some days with preferences as provided in comment #14 and it works very nice and fast, especially with e10s. Any chance to get this landed in production?