Right now we allocate shmem surfaces and copy them back to xlib. We should just send xlib surfaces.

I'd like to funnel new texture creation into one function, to avoid potential duplication. With this you would just specify what properties you want your TextureClient to support. It looks like CanvasClient doesn't actually require a BufferTextureClient so I also changed that.

Comment on attachment 8344139 [details] [diff] [review] part 1: consolidate texture creation Review of attachment 8344139 [details] [diff] [review]: ----------------------------------------------------------------- This is probably what we want long term (so f+ for that), but for now I think it is a bad idea - things are going to get more complex due to using gralloc texture clients - these will only be used by content, not image/canvas (for now). Furthermore, until bug 908196 is fixed we really need to avoid using texture clients for non-content because allocation will fail further down the line. I'm not sure what Windows will look like, but I think we want a distinction there for content vs image/canvas. I think we have a separate method for buffer texture clients also because we really need a texture client that we can read/write not just write to. So I guess having one method and using a flag like you do here will still work, but unless there is a really good reason to, I'd prefer to avoid it - all the flags in the layers code is getting confusing already and I think having distinct methods is clearer. If it does really clean things up to do this, then I won't object, but you should f? nical too in that case. Also the 'Surface' path (as opposed to DrawTarget) should be being phased out soon, not sure how that will affect this though.

Comment on attachment 8344139 [details] [diff] [review] part 1: consolidate texture creation Okay, I don't think it actually helped me at all anyway :)

I updated this patch to trunk. I also added a call FinishX in TextureClientX11::Unlock, which stops it from crashing all the time. This might be bias, but some stuff definitely feels faster.

This patch makes scrolling much smoother for me. In my light testing, it seems to bring graphics performance in e10s up to the same level as basic layers.

Here's a slightly updated version of the patch. With the file descriptor fix that I'm about to attach, I haven't noticed any problems with it.

The reason we were getting crashes at shutdown is that the child process was exiting, then the pixmap was automatically getting released by the X server, and then the parent was trying to free the pixmap. This patch uses the same technique we use for the plugin container: dup() the client's X server connection into the parent so that the child's resources aren't released when the child exits or crashes.

Oh, and the reason I added the SendBackUpXResources call where I did is that gtk_init happens just a little bit earlier in the same function. I couldn't put it right next to gtk_init because the IPC connection hasn't opened at that time.

https://hg.mozilla.org/integration/mozilla-inbound/rev/66a7934ca853 https://hg.mozilla.org/integration/mozilla-inbound/rev/ab19291191fe

Oh this is so awesome, good job on figuring this out. For some reason I wasn't cc'ed on this?! There are two followups. In TextureHostX11::Unlock it would be cleaner to use FinishX(msSurface->XDisplay()). The comment over BackUpXResources in PContent.ipdl talks about plugins.

I had to back out because it doesn't build on Windows/Mac. Oops. https://hg.mozilla.org/integration/mozilla-inbound/rev/e1fb8eeca62e Tom, can you post a patch on top of the existing one?

Relanded with a few #ifdef changes. https://hg.mozilla.org/integration/mozilla-inbound/rev/db5a4ed50de4

And one more push, since bug 950235 landed right before me: https://hg.mozilla.org/integration/mozilla-inbound/rev/fce6b2c04bab

Backed out for IPC test crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/a150cbbc959b https://tbpl.mozilla.org/php/getParsedLog.php?id=34155855&tree=Mozilla-Inbound

Relanded again. The oranges Ryan mentioned were fixed by the commit in comment 14. https://hg.mozilla.org/integration/mozilla-inbound/rev/07cddaac17c2

This busted my local warnings-as-errors build (using clang++ on linux), with this warning (treated as an error in this directory): { gfx/layers/basic/TextureClientX11.cpp:5: ../../dist/include/mozilla/layers/TextureClientX11.h:59:16: error: private field 'mTextureFlags' is not used [-Werror,-Wunused-private-field] } I'm guessing this didn't bust TBPL because this code is linux-only, and the warning is clang-only, and our official linux builders don't use clang. I filed bug 968635 for the build warning; just posting here as a heads-up.

I'm getting a build error with Clang 3.3 with warnings-as-errors: 0:54.05 In file included from /hack/mozilla-central/gfx/layers/basic/TextureClientX11.cpp:5: 0:54.05 ../../dist/include/mozilla/layers/TextureClientX11.h:59:16: error: private field 'mTextureFlags' is not used [-Werror,-Wunused-private-field] 0:54.05 TextureFlags mTextureFlags; 0:54.05 ^ 0:54.05 1 error generated.

Oh, that is bug 968635 already. Thanks.