framebuffer binding should not be changed after canvas resize or compositing

• Component: WebGL
• Status: RESOLVED FIXED

[Zhenyao Mo]

At the moment WebGL conformance tests framebuffer-bindings-unaffected-on-resize.html are failing, which indicates that the binding is affected.

[Zhenyao Mo]

Created attachment 138379 [details]
Patch

[Zhenyao Mo]

Test is synced from khronos.

Please have a look.

[James Robinson]

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.

[Kenneth Russell]

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.

[Jeff Timanus]

(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

[Zhenyao Mo]

(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.

[Jer Noble]

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

[Jer Noble]

(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

[Jer Noble]

Rolled out in r114935.

Committed r114935: <http://trac.webkit.org/changeset/114935>

[Zhenyao Mo]

Created attachment 138442 [details]
Patch

[Jer Noble]

What is different about this patch?  Have you run the testcases which failed on the previous patch?

[Zhenyao Mo]

(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)

[Zhenyao Mo]

Committed r114961: <http://trac.webkit.org/changeset/114961>