Bug 80895 – After webgl canvas resize, bindings might be lost

• Component: WebGL
• Status: RESOLVED FIXED

[Zhenyao Mo]

This includes texture/framebuffer/renderbuffer bindings.

[Zhenyao Mo]

Created attachment 131436 [details]
Patch

[Zhenyao Mo]

Stephen, Ken is probably out, can you review this patch?

[Stephen White]

This looks ok, although I'm not super-familiar with this code.  r=me, in case this needs to go in in a hurry.

If it's DrawingBuffer that's messing up the bindings, should it be DrawingBuffer that restores them?  Or does this save us doing a bunch of get()'s?

[Stephen White]

Comment on attachment 131436 [details]
Patch

[Zhenyao Mo]

(In reply to comment #3)
> This looks ok, although I'm not super-familiar with this code.  r=me, in case this needs to go in in a hurry.
> 
> If it's DrawingBuffer that's messing up the bindings, should it be DrawingBuffer that restores them?  Or does this save us doing a bunch of get()'s?

If DrawingBuffer needs to restore the binding states, it needs to track the current bindings, which is not trivia.  For example, if a current bound framebuffer is deleted, we need to reset the binding, etc.  This is basically duplicate all the logic in WebGLRenderingContext in DrawingBuffer and GraphicsContext3D.  That's why I restore the bindings in WebGLRenderingContext.

[Stephen White]

(In reply to comment #5)
> (In reply to comment #3)
> > This looks ok, although I'm not super-familiar with this code.  r=me, in case this needs to go in in a hurry.
> > 
> > If it's DrawingBuffer that's messing up the bindings, should it be DrawingBuffer that restores them?  Or does this save us doing a bunch of get()'s?
> 
> If DrawingBuffer needs to restore the binding states, it needs to track the current bindings, which is not trivia.  For example, if a current bound framebuffer is deleted, we need to reset the binding, etc.  This is basically duplicate all the logic in WebGLRenderingContext in DrawingBuffer and GraphicsContext3D.  That's why I restore the bindings in WebGLRenderingContext.

Sounds reasonable.

I forgot to mention:  that test is great.  Is that something that's worth upstreaming to the WebGL conformance suite?

[Zhenyao Mo]

Committed r110526: <http://trac.webkit.org/changeset/110526>

[Jeff Timanus]

This change looks good.  DrawingBuffer::reset plays with the bound textures and framebuffers, which need to be re-assigned after the resize.

[Zhenyao Mo]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > This looks ok, although I'm not super-familiar with this code.  r=me, in case this needs to go in in a hurry.
> > > 
> > > If it's DrawingBuffer that's messing up the bindings, should it be DrawingBuffer that restores them?  Or does this save us doing a bunch of get()'s?
> > 
> > If DrawingBuffer needs to restore the binding states, it needs to track the current bindings, which is not trivia.  For example, if a current bound framebuffer is deleted, we need to reset the binding, etc.  This is basically duplicate all the logic in WebGLRenderingContext in DrawingBuffer and GraphicsContext3D.  That's why I restore the bindings in WebGLRenderingContext.
> 
> Sounds reasonable.
> 
> I forgot to mention:  that test is great.  Is that something that's worth upstreaming to the WebGL conformance suite?

The test was checked in to khronos by Gregg, and I copied from there, so it's already in the suite.