Bug 84609 - framebuffer binding should not be changed after canvas resize or compositing
: framebuffer binding should not be changed after canvas resize or compositing
Status: RESOLVED FIXED
: WebKit
WebGL
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
: 84623 84629
:
  Show dependency treegraph
 
Reported: 2012-04-23 10:16 PST by
Modified: 2012-04-23 16:22 PST (History)


Attachments
Patch (16.70 KB, patch)
2012-04-23 10:34 PST, Zhenyao Mo
no flags Review Patch | Details | Formatted Diff | Diff
Patch (16.62 KB, patch)
2012-04-23 15:43 PST, Zhenyao Mo
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-04-23 10:16:14 PST
At the moment WebGL conformance tests framebuffer-bindings-unaffected-on-resize.html are failing, which indicates that the binding is affected.
------- Comment #1 From 2012-04-23 10:34:25 PST -------
Created an attachment (id=138379) [details]
Patch
------- Comment #2 From 2012-04-23 10:35:22 PST -------
Test is synced from khronos.

Please have a look.
------- Comment #3 From 2012-04-23 10:51:34 PST -------
(From update of attachment 138379 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=138379&action=review

> LayoutTests/fast/canvas/webgl/resources/webgl-test-utils.js:1144
> +var cancelRequestAnimFrame = (function() {

the standard name of this function is "cancelAnimationFrame"

> LayoutTests/fast/canvas/webgl/resources/webgl-test-utils.js:1145
> +  return window.cancelCancelRequestAnimationFrame ||

window.cancelCancel...() ? what is this?

> LayoutTests/fast/canvas/webgl/resources/webgl-test-utils.js:1150
> +         window.clearTimeout;

nothing here checks for the real name of the function - window.cancelAnimationFrame() - so this won't work in a browser with only the unprefixed version.
------- Comment #4 From 2012-04-23 11:32:56 PST -------
(From update of attachment 138379 [details])
The code looks fine to me; please make the test changes James points out upstream. (See https://developer.mozilla.org/en/DOM/window.cancelAnimationFrame .) Feel free to upload another version of the patch fixing those issues. r=me with those fixes made.
------- Comment #5 From 2012-04-23 11:58:37 PST -------
(In reply to comment #1)
> Created an attachment (id=138379) [details] [details]
> Patch

Thanks for looking into this issue, Mo.  The changes in WebGLRenderingContext, and DrawingBuffer look good to me.  Caching the bound framebuffer object in the drawing buffer seems like the simplest fix for this problem.

Jeff
------- Comment #6 From 2012-04-23 12:01:31 PST -------
(In reply to comment #3)
> (From update of attachment 138379 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138379&action=review
> 
> > LayoutTests/fast/canvas/webgl/resources/webgl-test-utils.js:1144
> > +var cancelRequestAnimFrame = (function() {
> 
> the standard name of this function is "cancelAnimationFrame"
> 
> > LayoutTests/fast/canvas/webgl/resources/webgl-test-utils.js:1145
> > +  return window.cancelCancelRequestAnimationFrame ||
> 
> window.cancelCancel...() ? what is this?
> 
> > LayoutTests/fast/canvas/webgl/resources/webgl-test-utils.js:1150
> > +         window.clearTimeout;
> 
> nothing here checks for the real name of the function - window.cancelAnimationFrame() - so this won't work in a browser with only the unprefixed version.

Gregg just fixed these issues on the khronos side.  I will re-sync before landing.
------- Comment #7 From 2012-04-23 13:04:10 PST -------
Apparently this patch got landed in http://trac.webkit.org/changeset/114929.  However, it's broken a ton of canvas/webgl tests, including:


fast/canvas/webgl/texture-bindings-uneffected-on-resize.html
fast/canvas/webgl/webgl-composite-modes-repaint.html
fast/canvas/webgl/compressed-tex-image.html
fast/canvas/webgl/webgl-viewport-parameters-preserved.html
fast/canvas/webgl/webgl-composite-modes.html
fast/canvas/webgl/framebuffer-bindings-unaffected-on-resize.html
------- Comment #8 From 2012-04-23 13:07:42 PST -------
(In reply to comment #7)
> Apparently this patch got landed in http://trac.webkit.org/changeset/114929.  However, it's broken a ton of canvas/webgl tests, including:
> 
> 
> fast/canvas/webgl/texture-bindings-uneffected-on-resize.html
> fast/canvas/webgl/webgl-composite-modes-repaint.html
> fast/canvas/webgl/compressed-tex-image.html
> fast/canvas/webgl/webgl-viewport-parameters-preserved.html
> fast/canvas/webgl/webgl-composite-modes.html
> fast/canvas/webgl/framebuffer-bindings-unaffected-on-resize.html

Here's the failure report for Lion Release (Tests) with 35 webgl test errors:

http://build.webkit.org/results/Lion%20Release%20(Tests)/r114929%20(7859)/results.html
------- Comment #9 From 2012-04-23 13:13:02 PST -------
Rolled out in r114935.

Committed r114935: <http://trac.webkit.org/changeset/114935>
------- Comment #10 From 2012-04-23 15:43:51 PST -------
Created an attachment (id=138442) [details]
Patch
------- Comment #11 From 2012-04-23 15:49:19 PST -------
What is different about this patch?  Have you run the testcases which failed on the previous patch?
------- Comment #12 From 2012-04-23 15:55:12 PST -------
(In reply to comment #11)
> What is different about this patch?  Have you run the testcases which failed on the previous patch?

webkit-test-util.js is different from the previous version, and the issue caused the failures are fixed (tested locally)
------- Comment #13 From 2012-04-23 16:22:29 PST -------
Committed r114961: <http://trac.webkit.org/changeset/114961>