Summary: | [Chromium] WebGL active texture state corrupted upon WebGL contents present | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jeff Timanus <twiz> | ||||||||||||
Component: | WebGL | Assignee: | Jeff Timanus <twiz> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cc-bugs, dglazkov, eric, gustavo.noronha, gustavo, jamesr, kbr, rniwa, senorblanco, twiz, webkit.review.bot, xan.lopez | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Jeff Timanus
2011-11-23 11:20:44 PST
Created attachment 116387 [details]
Patch
Comment on attachment 116387 [details] Patch 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 on attachment 116387 [details]
Patch
Nice work tracking this down. Looks like it needs to be merged to top of tree, but r=me with that addressed.
BTW, possible to write a layout test for this? Created attachment 116400 [details]
Patch
Comment on attachment 116400 [details] Patch Attachment 116400 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10635041 Comment on attachment 116400 [details] Patch Attachment 116400 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10636045 Comment on attachment 116400 [details] Patch Attachment 116400 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10632048 Created attachment 116422 [details]
Patch
Created attachment 116467 [details]
Patch
Comment on attachment 116467 [details] Patch 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 on attachment 116467 [details] Patch Clearing flags on attachment: 116467 Committed r101120: <http://trac.webkit.org/changeset/101120> All reviewed patches have been landed. Closing bug. 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 Created attachment 116638 [details]
Patch
(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 on attachment 116638 [details]
Patch
OK. r=me
(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 Re-opening the bug, to allow the CQ to process pending patch #5. Comment on attachment 116638 [details] Patch Clearing flags on attachment: 116638 Committed r101696: <http://trac.webkit.org/changeset/101696> All reviewed patches have been landed. Closing bug. |