Bug 80895 - After webgl canvas resize, bindings might be lost
Summary: After webgl canvas resize, bindings might be lost
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-12 14:53 PDT by Zhenyao Mo
Modified: 2012-03-13 09:15 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.97 KB, patch)
2012-03-12 16:09 PDT, Zhenyao Mo
senorblanco: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zhenyao Mo 2012-03-12 14:53:23 PDT
This includes texture/framebuffer/renderbuffer bindings.
Comment 1 Zhenyao Mo 2012-03-12 16:09:40 PDT
Created attachment 131436 [details]
Patch
Comment 2 Zhenyao Mo 2012-03-12 16:10:31 PDT
Stephen, Ken is probably out, can you review this patch?
Comment 3 Stephen White 2012-03-12 18:39:18 PDT
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?
Comment 4 Stephen White 2012-03-12 18:39:44 PDT
Comment on attachment 131436 [details]
Patch
Comment 5 Zhenyao Mo 2012-03-12 18:43:10 PDT
(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.
Comment 6 Stephen White 2012-03-12 18:51:36 PDT
(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?
Comment 7 Zhenyao Mo 2012-03-12 18:53:54 PDT
Committed r110526: <http://trac.webkit.org/changeset/110526>
Comment 8 Jeff Timanus 2012-03-13 08:32:33 PDT
This change looks good.  DrawingBuffer::reset plays with the bound textures and framebuffers, which need to be re-assigned after the resize.
Comment 9 Zhenyao Mo 2012-03-13 09:15:22 PDT
(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.