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
Product: WebKit
Classification: Unclassified
Component: WebGL
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Zhenyao Mo
:
Depends on: 84623 84629
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-23 10:16 PDT by Zhenyao Mo
Modified: 2012-04-23 16:22 PDT (History)
6 users (show)

See Also:


Attachments
Patch (16.70 KB, patch)
2012-04-23 10:34 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (16.62 KB, patch)
2012-04-23 15:43 PDT, Zhenyao Mo
no flags 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-04-23 10:16:14 PDT
At the moment WebGL conformance tests framebuffer-bindings-unaffected-on-resize.html are failing, which indicates that the binding is affected.
Comment 1 Zhenyao Mo 2012-04-23 10:34:25 PDT
Created attachment 138379 [details]
Patch
Comment 2 Zhenyao Mo 2012-04-23 10:35:22 PDT
Test is synced from khronos.

Please have a look.
Comment 3 James Robinson 2012-04-23 10:51:34 PDT
Comment on attachment 138379 [details]
Patch

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 Kenneth Russell 2012-04-23 11:32:56 PDT
Comment on attachment 138379 [details]
Patch

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 Jeff Timanus 2012-04-23 11:58:37 PDT
(In reply to comment #1)
> Created an attachment (id=138379) [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 Zhenyao Mo 2012-04-23 12:01:31 PDT
(In reply to comment #3)
> (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.

Gregg just fixed these issues on the khronos side.  I will re-sync before landing.
Comment 7 Jer Noble 2012-04-23 13:04:10 PDT
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 Jer Noble 2012-04-23 13:07:42 PDT
(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 Jer Noble 2012-04-23 13:13:02 PDT
Rolled out in r114935.

Committed r114935: <http://trac.webkit.org/changeset/114935>
Comment 10 Zhenyao Mo 2012-04-23 15:43:51 PDT
Created attachment 138442 [details]
Patch
Comment 11 Jer Noble 2012-04-23 15:49:19 PDT
What is different about this patch?  Have you run the testcases which failed on the previous patch?
Comment 12 Zhenyao Mo 2012-04-23 15:55:12 PDT
(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 Zhenyao Mo 2012-04-23 16:22:29 PDT
Committed r114961: <http://trac.webkit.org/changeset/114961>