CanvasLayerOGL is a bit broken on Mac with retained layers: -- some work that's done on every paint should be moved to Updated and the destructor -- We were passing the wrong CGLContext to CGLTexImagePBuffer (we need to pass the destination context, not the pbuffer context) -- Bogus assertion in LayerManagerOGL; it's OK to not have a root at the start of a transaction

We can crash if the CGLContext is released before the widget. Also, GetLayerManager should not crash if we have no outer window. This can happen temporarily as we go fullscreen, apparently.

Hmm, so maybe I'm not undertanding something, when retained layers are in use -- the problem is calling BindTexImage makes rendering to the bound buffer undefined. So it's not something that can just be done once, it has to be done just when the pbuffer is to be used as a texture, and it has to be detached each time. This is why it was done during Render time, and not during Updated() time. Without retained layers, we create a new layer each time and just call Updated() right away, but that's being called while we're actually rendering and building up the layer tree. When does Updated() get called with retained layers?

Comment on attachment 451467 [details] [diff] [review] Part 2: Retain CGLContext for the lifetime of its widget. we may want to rename this to mCGLContext; mContext is fairly generic

WebGLContext::GetCanvasLayer and nsCanvasRenderingContext2D::GetCanvasLayer call Updated when they returns an old layer. I can modify the patch to Unbind after RenderLayer and Bind on every Updated call, if you like. I don't think we need to do that on Mac, though. See http://developer.apple.com/mac/library/documentation/GraphicsImaging/Reference/CGL_OpenGL/Reference/reference.html#//apple_ref/c/func/CGLTexImagePBuffer If you modify the content of a pixel buffer that uses mipmap levels, you must call this function again before drawing with the pixel buffer, to ensure that the content is synchronized with OpenGL. For pixel buffers without mipmaps, simply rebind to the texture object to synchronize content. Sound OK?

As long as we can ensure that no script will execute between GetCanvasLayer/RenderLayer, that sounds fine. On Win32, the mac bit isn't true -- from the WGL_render_texture extension: - The application must release the color buffer from the texture before it can render to the pbuffer again. When the color buffer is bound as a texture, draw and read operations on the pbuffer are undefined. I guess we can skip it on OSX. (Note that I believe these two statements are using the term 'bind' differently.) One additional interesting bit: 11. When the color buffer is released from the texture (back to the pbuffer) should the contents be preserved? No, this may prove difficult to implement on some architectures. That's interesting, because that's not how things work currently on any win32 box that I've tried. But it sounds like this is all the more reason to start using FBOs for this. I'll work on that at some point soon.

Fixed to rebind Windows pbuffer at every paint.

Trivial

Comment on attachment 451814 [details] [diff] [review] Part 1 v2 >+#if defined(XP_MACOSX) >+ // We only need to do this for the first time we set up the texture >+ if (mTexture == 0) { This needs to be "if (newTexture)", right?

Er, yep!

Addressed comment.

http://hg.mozilla.org/mozilla-central/rev/b5b2dd87d528 http://hg.mozilla.org/mozilla-central/rev/5f9932cbe799 http://hg.mozilla.org/mozilla-central/rev/22cffed4ebda