Bug 122369

Summary: Undo texture unit code refactoring - it is ok to bind a texture to multiple locations
Product: WebKit Reporter: Dean Jackson <dino>
Component: WebGLAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, gyuyoung.kim, kondapallykalyan, p.szymanski3, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Dean Jackson
Reported 2013-10-04 19:15:35 PDT
The fix for 117868 is an optimisation that causes us to fail the WebGL conformance test. In particular, it is valid to bind a single texture to multiple locations, as long as each location is in a different program. Revert the change http://trac.webkit.org/changeset/152351
Attachments
Patch (11.04 KB, patch)
2013-10-04 19:39 PDT, Dean Jackson
darin: review+
Radar WebKit Bug Importer
Comment 1 2013-10-04 19:15:45 PDT
Dean Jackson
Comment 2 2013-10-04 19:39:51 PDT
Darin Adler
Comment 3 2013-10-05 00:37:18 PDT
Comment on attachment 213432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213432&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:607 > + m_textureUnits[i].m_texture2DBinding = 0; > + m_textureUnits[i].m_textureCubeMapBinding = 0; nullptr is what we now use instead of 0 in new code like this > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:1634 > + m_textureUnits[i].m_texture2DBinding = 0; nullptr > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:1636 > + m_textureUnits[i].m_textureCubeMapBinding = 0; nullptr > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4877 > + WebGLTexture* tex = 0; nullptr Also, can we call this texture instead of tex? > Source/WebCore/html/canvas/WebGLRenderingContext.h:453 > + class TextureUnitState { > + public: struct seems better than class/public, although normally we do not use the m_ prefix on public data members in structs like this
Dean Jackson
Comment 4 2013-10-05 12:14:22 PDT
Comment on attachment 213432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213432&action=review I was right to be implicitly chastised for not taking the opportunity to clean up code :) >> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4877 >> + WebGLTexture* tex = 0; > > nullptr > > Also, can we call this texture instead of tex? Done. >> Source/WebCore/html/canvas/WebGLRenderingContext.h:453 >> + public: > > struct seems better than class/public, although normally we do not use the m_ prefix on public data members in structs like this I changed this to a struct, and removed the m_ prefixes.
Dean Jackson
Comment 5 2013-10-05 12:21:09 PDT
Dean Jackson
Comment 6 2013-10-07 12:18:33 PDT
*** Bug 120285 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.