There are some misusage of GraphicsContext3D::activeTexture in WebGLRenderingContextBase::checkTextureCompleteness. GraphicsContext3D::activeTexture is expected to be used with an argument that is greater than or equal to GraphicsContext3D::TEXTURE0. However, WebGLRenderingContextBase::checkTextureCompleteness calls GraphicsContext3D::activeTexture with a zero-based index.
Created attachment 258989 [details] Patch
Comment on attachment 258989 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258989&action=review > Source/WebCore/ChangeLog:11 > + No new tests because there is no behavior change. If there is no behavior change, then what does this fix?
Comment on attachment 258989 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258989&action=review > Source/WebCore/ChangeLog:11 > + No new tests because there is no behavior change. There should be a behavior change since there is a bug in passing the wrong parameter value to GraphicsContext3D::activeTexture(). Can not we have a layout test or even a test case which shows the rendering difference between before and after the fix? > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4064 > + m_context->activeTexture(m_activeTextureUnit + GraphicsContext3D::TEXTURE0); Yes this looks a correct fix. This can be verified from https://www.khronos.org/opengles/sdk/docs/man/xhtml/glActiveTexture.xml and from WebGLRenderingContextBase::activeTexture() in which we set m_activeTextureUnit = texture - GraphicsContext3D::TEXTURE0.
(In reply to comment #2) > Comment on attachment 258989 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=258989&action=review > > > Source/WebCore/ChangeLog:11 > > + No new tests because there is no behavior change. > > If there is no behavior change, then what does this fix? You're right. I'll revise my patch.
(In reply to comment #3) > Comment on attachment 258989 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=258989&action=review > > > Source/WebCore/ChangeLog:11 > > + No new tests because there is no behavior change. > > There should be a behavior change since there is a bug in passing the wrong > parameter value to GraphicsContext3D::activeTexture(). Can not we have a > layout test or even a test case which shows the rendering difference between > before and after the fix? > > > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4064 > > + m_context->activeTexture(m_activeTextureUnit + GraphicsContext3D::TEXTURE0); > > Yes this looks a correct fix. This can be verified from > https://www.khronos.org/opengles/sdk/docs/man/xhtml/glActiveTexture.xml and > from WebGLRenderingContextBase::activeTexture() in which we set > m_activeTextureUnit = texture - GraphicsContext3D::TEXTURE0. Thank you for review. I'll try to include a test case that shows the behavior difference.
Created attachment 259377 [details] Patch
Created attachment 259379 [details] Patch
Comment on attachment 259379 [details] Patch Attachment 259379 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/76152 New failing tests: fast/canvas/webgl/texture-complete.html
Created attachment 259385 [details] Archive of layout-test-results from ews107 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 259379 [details] Patch Attachment 259379 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/76302 New failing tests: fast/canvas/webgl/texture-complete.html
Created attachment 259390 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 259416 [details] Patch
Comment on attachment 259416 [details] Patch I confirmed that the new test fails in tip-of-tree webkit, passes in Chrome and Firefox, and is the correct behavior. Thanks!
(In reply to comment #13) > Comment on attachment 259416 [details] > Patch > > I confirmed that the new test fails in tip-of-tree webkit, passes in Chrome > and Firefox, and is the correct behavior. Thanks! Nice!
<rdar://problem/22353988>
Comment on attachment 259416 [details] Patch Clearing flags on attachment: 259416 Committed r188666: <http://trac.webkit.org/changeset/188666>
All reviewed patches have been landed. Closing bug.
This fixed conformance/textures/texture-npot.html
(In reply to comment #18) > This fixed conformance/textures/texture-npot.html Hey! A PROGRESSION! Nice work Jinyoung!
(In reply to comment #19) > (In reply to comment #18) > > This fixed conformance/textures/texture-npot.html > > Hey! A PROGRESSION! > > Nice work Jinyoung! Wow! Thanks!
*** Bug 100402 has been marked as a duplicate of this bug. ***