496204 - Stop using cairo xlib surfaces (at least optionally)

Status: NEW

(Core :: Graphics, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We have lots of problems with X fallbacks, crappy X servers, pixmap usage, weird performance problems in certain setups, etc. In particular we seem to be more sensitive to Xrender implementation quality that say Opera or Webkit/GTK+. I would like to have at least have an option to use cairo image surfaces for everything internally, and just copy image surface backbuffers to widgets, preferably using Xshm. Then we can see how good it is.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yep, absolutely -- I think this is a dup of an existing bug, but I can't find it.  Note that we already have 90% of the code for this in place, as used by fennec; the XShm piece still needs work, though that's not crucial (I think we could just use XPutImage an friends, perhaps through the gdk wrapper so that it works on directfb and other gdk systems).

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This will cause some issues for windowless plugins and GTK theme drawing, both of which want to draw into an X surface. Windowless plugins aren't a huge concern, for opaque ones we draw into a pixmap and fetch it back from the X server, for transparent ones we need to draw into two pixmaps and fetch them back from the X server --- basically cairo-xlib-utils does this for us. For theme drawing we will probably need the theme cache we've talked about. But I guess that's worth doing because we'll need it for our putative GL backend.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Vlad is bug 463738 the one you meant?

Comment 4

[Oleg Romashin (:romaxa)]

For Gtk themes we can use XShmGetImage... it seems to works fine, I even have patch somewhere about it (for microb)

The same can be done for plugins... also it would be nice to stabilize NPImageExpose (or some other name) NPAPI which allow us to render plugins data into image surface.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't see how XShmGetImage would help us all that much with the theme drawing. The problem is we need to get two images and do alpha recovery.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

XShmCreatePixmap might help us. We might be able to use it to create X pixmaps but then manipulate them using cairo-image-surface. Then, if we need to draw a GTK widget into the pixmap, we can give GTK the X pixmap. Before we start drawing with cairo-image-surface again, we'd probably have to XSync.

Comment 7

[Frederic Plourde (:fred23)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> XShmCreatePixmap might help us. We might be able to use it to create X
> pixmaps but then manipulate them using cairo-image-surface. Then, if we need
> to draw a GTK widget into the pixmap, we can give GTK the X pixmap. Before
> we start drawing with cairo-image-surface again, we'd probably have to XSync.

Hi Robert, we are kicking off a new project for getting rid of Xlib surfaces for rendering and basically make it so that all of the drawing goes through cairo-image backend. This rather "old" bug seems like the perfect place to track this.


(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> This will cause some issues for windowless plugins and GTK theme drawing,
> both of which want to draw into an X surface. Windowless plugins aren't a
> huge concern, for opaque ones we draw into a pixmap and fetch it back from
> the X server, for transparent ones we need to draw into two pixmaps and
> fetch them back from the X server --- basically cairo-xlib-utils does this
> for us. For theme drawing we will probably need the theme cache we've talked
> about. But I guess that's worth doing because we'll need it for our putative
> GL backend.

for starters, since the bug's latest activity is more than 2 years old, what's current status on this whole "we need to draw into 2 pixmaps to emulate alpha" thing ? Is it still the case ?  And what about the "them cache" you're talking about ?.. any development on that front ?

Comment 8

[Frederic Plourde (:fred23)]

Guys, nowadays, we're having

Comment 9

[Frederic Plourde (:fred23)]

Guys, nowadays, we're having gfxPlatformGTK::CreateOffscreenSurface() create gfxImageSurfaces when perf 'gfx.xrender.enabled' is enabled.

This is the **only** use of that perf.

So as a first step, to get rid of xlib surfaces for rendering on the GTK platform, I would suggest we just get rid of that perf and unconditionaly make gfxPlatformGTK::CreateOffscreenSurface()  go the gfxImageSurface road.

Comment 10

[Frederic Plourde (:fred23)]

Sorry, 'UseXRender' is also used to set mUseTextureFromPixmap on compositor side.
So I guess as a first step, we'd just :

1) keep the perf
2) unconditionnaly make gfxPlatformGTK::CreateOffscreenSurface() got the cairo-image backend

Comment 11

[Karl Tomlinson (:karlt)]

> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> > This will cause some issues for windowless plugins and GTK theme drawing,
> > both of which want to draw into an X surface. Windowless plugins aren't a
> > huge concern, for opaque ones we draw into a pixmap and fetch it back from
> > the X server, for transparent ones we need to draw into two pixmaps and
> > fetch them back from the X server --- basically cairo-xlib-utils does this
> > for us. For theme drawing we will probably need the theme cache we've talked
> > about. But I guess that's worth doing because we'll need it for our putative
> > GL backend.

(In reply to Frederic Plourde (:fred23) from comment #7)
> what's current status on this whole "we need to draw into 2 pixmaps to
> emulate alpha" thing ? Is it still the case ?

For transparent windowless plugins, this is already/still happening.
For opaque windowless plugins, we don't need to pay this penalty.
The plugin's drawing is in an X surface in an ImageLayer. 

> And what about the "theme cache" you're talking about ?

This will no longer be necessary with GTK3 theme drawing, because GTK3 themes
draw into a cairo_t instead of a Pixmap.

Comment 12

[Frederic Plourde (:fred23)]

(In reply to Oleg Romashin (:romaxa) from comment #4)
> For Gtk themes we can use XShmGetImage... it seems to works fine, I even
> have patch somewhere about it (for microb)
> 
> The same can be done for plugins... also it would be nice to stabilize
> NPImageExpose (or some other name) NPAPI which allow us to render plugins
> data into image surface.

Hey Oleg, please, could you provide URL to this microb patch you were talking about ?
I'd like to take a look, thanks

Comment 13

[Frederic Plourde (:fred23)]

Attachment: 0001-Unconditionaly_use_image-backend_rather_than_Xlib_surfaces_in_GTK_plaform.patch

Hey guys, see the attached patch.

Just by unconditionnaly creating gfxImageSurface-typed surfaces in gfxPlatformGtk::CreateOffscreenSurface, the compositor seems to naturally pick up the UseShm() path and wrap the content's gfxImageSurfaces into ShmXImages using XShmCreateImage and use that to composite using XShmPutImage in widget/shared/nsShmImage.cpp:nsShmImage::Put(...). So seems Gecko already has provisions for that. I tested the patch with many contents (both GTK2 and GTK3 platforms, with xrender.enabled true and false) and it seemed to render everything just right.

Jeff : So that's kinda like the first step we were talking about on last meeting. Seems with this patch we're rendering using Image-surface backend, but still compositing using XRENDER. 

Now, the gtk theme native rendering *still* happens through gfxXlibSurfaces, of course, and go the good old path : 
gfxXlibSurface::CreateCairoSurface
CreateTempXlibSurface
gfxXlibNativeRenderer::Draw ** we're going through Xlib surfaces
gfxGdkNativeRenderer::Draw
nsNativeThemeGTK::DrawWidgetBackground  *** we're rendering the gtk window's background
sDisplayThemedBackground::PaintInternal
FrameLayerBuilder::PaintItems
FrameLayerBuilder::DrawThebesLayer
BasicThebesLayer::PaintBuffer
BasicThebesLayer::Validate
BasicContainerLayer::Validate
BasicLayerManager::EndTransactionInternal
nsDisplayList::PaintForFrame
nsDisplayList::PaintRoot
nsLayoutUtils::PaintFrame
PresShell::Paint
[...]

For native rendering to go through image backend, we'd indeed need the XShmCreatePixmap twist roc suggested earlier.

But Jeff : tell me, concerning our Collabora project, which end goal is to turn on Skia opengl layers for content rendering instead of cairo... I guess gtk native rendering does not matter and could still be using xlib, right ?

Comment 15

[Karl Tomlinson (:karlt)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> XShmCreatePixmap might help us.

Most systems don't support XShmCreatePixmap. e.g.
% xdpyinfo -ext MIT-SHM | tail -n 2
MIT-SHM version 1.1 opcode: 130, base event: 65, base error: 128
  shared pixmaps: no

If Thebes layers draw to image surfaces, then glTexImage2D (if using OpenGL layers) or XShmPutImage would usually be the fastest ways to upload to the GPU for compositing.  MIT-SHM is not always available, so an XPutImage (or cairo) fallback would also be required.

Comment 16

[Frederic Plourde (:fred23)]

(In reply to Karl Tomlinson (:karlt) from comment #15)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> > XShmCreatePixmap might help us.
> 
> Most systems don't support XShmCreatePixmap. e.g.
> % xdpyinfo -ext MIT-SHM | tail -n 2
> MIT-SHM version 1.1 opcode: 130, base event: 65, base error: 128
>   shared pixmaps: no

I'd like to get a sense of how much is "most systems" here. In fact, if this "shared pixmap" feature isn't widely supported, I don't think it's worth putting much efforts into it at this point.
> 
> If Thebes layers draw to image surfaces, then glTexImage2D (if using OpenGL
> layers) or XShmPutImage would usually be the fastest ways to upload to the
> GPU for compositing. 

I think Roc meant using XShmCreatePixmap only as a means to accomodate GTK2 native theme drawing which requires drawing into Pixmaps. That will not be the case for GTK3 native rendering.
Roc: any comment on this ?

> MIT-SHM is not always available, so an XPutImage (or
> cairo) fallback would also be required.

yeah, but isn't that already implemented ?

Comment 17

[Karl Tomlinson (:karlt)]

(In reply to Frederic Plourde (:fred23) from comment #16)
> > MIT-SHM is not always available, so an XPutImage (or
> > cairo) fallback would also be required.
> 
> yeah, but isn't that already implemented ?

Essentially through gShmAvailable, it seems.  Note that existing nsShmImage code is only used *after* compositing.

Note also that newer versions of cairo use MIT-SHM when available and do delayed syncing, only when necessary.  GTK3 builds require system-cairo, and so there would be no need to reimplement a special MIT-SHM path.

Comment 18

[Karl Tomlinson (:karlt)]

(In reply to Frederic Plourde (:fred23) from comment #16)
> (In reply to Karl Tomlinson (:karlt) from comment #15)
> > Most systems don't support XShmCreatePixmap. e.g.
> > % xdpyinfo -ext MIT-SHM | tail -n 2
> > MIT-SHM version 1.1 opcode: 130, base event: 65, base error: 128
> >   shared pixmaps: no
> 
> I'd like to get a sense of how much is "most systems" here. In fact, if this
> "shared pixmap" feature isn't widely supported, I don't think it's worth
> putting much efforts into it at this point.

Exa doesn't support shared pixmaps.  I haven't checked further.
http://cgit.freedesktop.org/xorg/xserver/tree/exa/exa.c?id=60014a4a98ff924ae7f6840781f768c1cc93bbab#n960

Comment 19

[Frederic Plourde (:fred23)]

(In reply to Karl Tomlinson (:karlt) from comment #17)
> Essentially through gShmAvailable, it seems.

Correct. But speaking of which...
Currently in nsShmImage::Create, we *try* to create and attach an XShmImage and if it doesn't work, only then will we set gShmAvailable = false.  Isn't there any way we could query the MIT-SHM extension instead ?

> Note that existing nsShmImage
> code is only used *after* compositing.

Right, atm, IIUC, we're only using nsShmImage->Put to bang composited result (main offscreen surface) onto the display.

nsWindow::OnExposeEvent
[...]
#  ifdef MOZ_HAVE_SHMIMAGE
    if (nsShmImage::UseShm() && MOZ_LIKELY(!mIsDestroyed)) {
#if (MOZ_WIDGET_GTK == 2)
        mShmImage->Put(mGdkWindow, exposeRegion.mRects, exposeRegion.mRectsEnd);
#else
        mShmImage->Put(mGdkWindow, exposeRegion.mRects);
#endif

> Note also that newer versions of cairo use MIT-SHM when available and do
> delayed syncing, only when necessary.  GTK3 builds require system-cairo, and
> so there would be no need to reimplement a special MIT-SHM path.

For comprehension's sake, could you tell me what's basically the difference between moz-cairo and system-cairo ?

Comment 20

[Karl Tomlinson (:karlt)]

(In reply to Frederic Plourde (:fred23) from comment #19)
> (In reply to Karl Tomlinson (:karlt) from comment #17)
> > Essentially through gShmAvailable, it seems.
> 
> Correct. But speaking of which...
> Currently in nsShmImage::Create, we *try* to create and attach an XShmImage
> and if it doesn't work, only then will we set gShmAvailable = false.  Isn't
> there any way we could query the MIT-SHM extension instead ?

I'm sure the presence of the extension can be queried, but I suspect the presence of the extension does not guarantee that XShmAttach will succeed.
I wouldn't bother with nsShmImage now that cairo has support, anyway.

> > Note also that newer versions of cairo use MIT-SHM when available and do
> > delayed syncing, only when necessary.  GTK3 builds require system-cairo, and
> > so there would be no need to reimplement a special MIT-SHM path.
> 
> For comprehension's sake, could you tell me what's basically the difference
> between moz-cairo and system-cairo ?

moz-cairo is an old version of cairo with some patches to make things work as desired with Gecko; it is built into Gecko.  system-cairo is the cairo installed in the operating system.  I expect that systems with GTK3 will have a cairo newer than moz-cairo.  Gecko can be built to use system-cairo instead of moz-cairo.
The patches in gfx/cairo/ provide some hints as to what might not work so well with system cairo.

Comment 21

[Frederic Plourde (:fred23)]

(In reply to Karl Tomlinson (:karlt) from comment #20)
> I'm sure the presence of the extension can be queried, but I suspect the
> presence of the extension does not guarantee that XShmAttach will succeed.

You're right, even recent cairo now tests success of XShmAttach() before returning true in "can_use_shm()"

> I wouldn't bother with nsShmImage now that cairo has support, anyway.
Agreed.

Comment 22

[Frederic Plourde (:fred23)]

Attachment: WIP: Xlib content surface removal

Here's a WIP patch about disabling Xlib content surfaces and using XShm XImage based surfaces for the OGL and Basic compositors.

Now, as this is a huge piece of work, I'd like this initial patch to be meant as a discussion base for upcoming meetings I'd like to conduct about this. In fact, while discussing with many #gfx guys independently, I realized we'll have to get more people interested about this one if we want to make this land someday.

Ok, first, this patch does not touch (yet) at the typical, single-threaded case where composition happens directly to widget drawable via BasicLayerManager::PaintLayer() and DrawTargetCairo::DrawSurface(). It's modifying X11 TextureClient and Host and defining another SurfaceDescriptor : XShmSurfaceDescriptor that carries info about the XImage.

Notice in TextureClientX11::AllocateForSurface, that we're creating a gfxImageSurface rather than gfxXlibSurface. I'm currently considering adding another class 'gfxXImageSurface' that would simply create an XImage and basically wrap the underlying gfxImageSurface.

You'll notice also all the MIT-SHM calls (shmget/at) in SurfaceDescriptorXShm ctor. That probably shouldn't needed, as we could only keep the XShmCreateImage parts here and use ShmemTextureClient to carry image data accross thread/process border, but I wouldn't dare at the moment, because ShmemTextureClient::Allocate hard-code shmem type to TYPE_BASIC for now (and we'd need TYPE_SYSV for XShmImageCreate).

There are still a lot of questions of stuff to discuss (in upcoming comments).
thanks !

Comment 23

[Frederic Plourde (:fred23)]

Attachment: WIP: on-MTC Xlib surface removal

Here's the second patch for Xlib surfaces removal. It builds ok, but there are still buggy corner cases and is presented here as a WIP patch to get your thoughts and comments about it so we can make it better.

Note that it still creates an intermediate Xlib surface in DrawTargetCairoXShmImage::Snapshot. That's only temporary to see if everything holds together atm and, as final step, as explained in Bug 738937, we will directly upload Image content to destination DrawTarget using XShmPutImage (without using Xrender).

Also, not present in this patch are all the #ifdef for platform/OS coherence. This will be included shortly.

Comment 24

[Frederic Plourde (:fred23)]

I have created a better bug structure to track everything related to Xlib surfaces removal.
This bug will become our Meta bug for the whole effort.

I'll split actual work into two different bugs (already present) :

1) Bug 720523 - Use cairo image surfaces with OpenGL layers on linux
   For the OMTC patch (since OGL layers are automatically forcing OMTC)


2) Bug 738937 - Use Cairo image surfaces and XShmPutImage instead of XRender
   For the on-MTC patch

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:bhood, could you have a look please?
For more information, please visit auto_nag documentation.