Bug 73033 - [Chromium] WebGL active texture state corrupted upon WebGL contents present
: [Chromium] WebGL active texture state corrupted upon WebGL contents present
Status: RESOLVED FIXED
: WebKit
WebGL
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-11-23 11:20 PST by
Modified: 2011-12-01 12:02 PST (History)


Attachments
Patch (7.66 KB, patch)
2011-11-23 11:33 PST, Jeff Timanus
no flags Review Patch | Details | Formatted Diff | Diff
Patch (6.72 KB, patch)
2011-11-23 12:37 PST, Jeff Timanus
no flags Review Patch | Details | Formatted Diff | Diff
Patch (7.41 KB, patch)
2011-11-23 14:02 PST, Jeff Timanus
no flags Review Patch | Details | Formatted Diff | Diff
Patch (11.74 KB, patch)
2011-11-23 19:11 PST, Jeff Timanus
no flags Review Patch | Details | Formatted Diff | Diff
Patch (2.30 KB, patch)
2011-11-25 13:53 PST, Jeff Timanus
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 2011-11-23 11:20:44 PST
DrawingBuffer::publishToPlatformLayer is corrupting the WebGL active texture state.  The bound texture must be restored after the texture copy.

Webkit issue tracking crbug.com/105045.
------- Comment #1 From 2011-11-23 11:33:50 PST -------
Created an attachment (id=116387) [details]
Patch
------- Comment #2 From 2011-11-23 11:53:03 PST -------
(From update of attachment 116387 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=116387&action=review

> Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:66
> +    , m_activeTextureUnit(0)

The default value for m_activeTextureUnit should probably be GraphicsContext3D::GL_TEXTURE0, not 0, just in case the WebGL app doesn't call glActiveTexture() before DrawingBuffer::publishToPlatformLayer() gets called.

> Source/WebCore/platform/graphics/gpu/mac/DrawingBufferMac.mm:47
> +    , m_activeTextureUnit(0)

Same as above.

> Source/WebCore/platform/graphics/gpu/qt/DrawingBufferQt.cpp:42
> +    , m_activeTextureUnit(0)

Same as above.

> Source/WebCore/platform/graphics/gtk/DrawingBufferGtk.cpp:44
> +    , m_activeTextureUnit(0)

Same as above.
------- Comment #3 From 2011-11-23 11:54:40 PST -------
(From update of attachment 116387 [details])
Nice work tracking this down. Looks like it needs to be merged to top of tree, but r=me with that addressed.
------- Comment #4 From 2011-11-23 11:55:14 PST -------
BTW, possible to write a layout test for this?
------- Comment #5 From 2011-11-23 12:37:25 PST -------
Created an attachment (id=116400) [details]
Patch
------- Comment #6 From 2011-11-23 12:51:21 PST -------
(From update of attachment 116400 [details])
Attachment 116400 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10635041
------- Comment #7 From 2011-11-23 12:52:40 PST -------
(From update of attachment 116400 [details])
Attachment 116400 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10636045
------- Comment #8 From 2011-11-23 12:53:31 PST -------
(From update of attachment 116400 [details])
Attachment 116400 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10632048
------- Comment #9 From 2011-11-23 14:02:24 PST -------
Created an attachment (id=116422) [details]
Patch
------- Comment #10 From 2011-11-23 19:11:38 PST -------
Created an attachment (id=116467) [details]
Patch
------- Comment #11 From 2011-11-23 20:46:25 PST -------
(From update of attachment 116467 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=116467&action=review

Looks good; nice job on the test case. A few minor nits in the test that can be cleaned up later. r=me

> LayoutTests/fast/canvas/webgl/webgl-texture-binding-preserved.html:22
> +var requestAnimationFrame;

nit: this variable is no longer needed.

> LayoutTests/fast/canvas/webgl/webgl-texture-binding-preserved.html:26
> +  // before reaching this point.  If the texture state was corruted, then 

nit: corruted -> corrupted

> LayoutTests/fast/canvas/webgl/webgl-texture-binding-preserved.html:42
> +  canvas.width = 50; canvas.height = 50;

nit: not needed

> LayoutTests/fast/canvas/webgl/webgl-texture-binding-preserved.html:63
> +  setTimeout(draw, 100);

nit: add comment that requestAnimationFrame was tried for this purpose and it didn't work. Also that the 100 ms timeout seemed to reproduce the problem 100% of the time even during fast page reloads.
------- Comment #12 From 2011-11-23 22:06:46 PST -------
(From update of attachment 116467 [details])
Clearing flags on attachment: 116467

Committed r101120: <http://trac.webkit.org/changeset/101120>
------- Comment #13 From 2011-11-23 22:06:52 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #15 From 2011-11-25 13:53:30 PST -------
Created an attachment (id=116638) [details]
Patch
------- Comment #16 From 2011-11-25 13:58:13 PST -------
(In reply to comment #14)
> The test added by this patch is failing on Mac:
> http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r101177%20(34952)/fast/canvas/webgl/webgl-texture-binding-preserved-pretty-diff.html

Investigating the failure.  This test should pass on all browsers, and does not rely on Chrome-specific behaviour.
------- Comment #17 From 2011-11-28 06:48:55 PST -------
(From update of attachment 116638 [details])
OK.  r=me
------- Comment #18 From 2011-11-28 08:05:28 PST -------
(In reply to comment #14)
> The test added by this patch is failing on Mac:
> http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r101177%20(34952)/fast/canvas/webgl/webgl-texture-binding-preserved-pretty-diff.html

The test is failing because safari is failing to construct the shaders used by the test.  Added a suppression, and a new bug here:  https://bugs.webkit.org/show_bug.cgi?id=73148
------- Comment #19 From 2011-12-01 10:49:30 PST -------
Re-opening the bug, to allow the CQ to process pending patch #5.
------- Comment #20 From 2011-12-01 12:02:46 PST -------
(From update of attachment 116638 [details])
Clearing flags on attachment: 116638

Committed r101696: <http://trac.webkit.org/changeset/101696>
------- Comment #21 From 2011-12-01 12:02:53 PST -------
All reviewed patches have been landed.  Closing bug.