Based on feedback from Daniel Koch from TransGaming, the section in the WebGL spec on restrictions on the stencil mask and ref values has been changed to do the validation at draw time rather than when stencilMaskSeparate and stencilFuncSeparate are called, since generating an INVALID_OPERATION error from those entry points may make it hard for applications to transition between states. The WebKit implementation needs to be updated to do the validation then as well. Ideally stencilMaskSeparate and stencilFuncSeparate would set a boolean flag which can be easily checked in drawArrays and drawElements.
Created attachment 77631 [details] Patch
Comment on attachment 77631 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77631&action=review Looks good; one cleanup upon landing please. > WebCore/html/canvas/WebGLRenderingContext.cpp:1119 > + if (m_stencilMask != m_stencilMaskBack || m_stencilFuncRef != m_stencilFuncRefBack || m_stencilFuncMask != m_stencilFuncMaskBack) { > + m_context->synthesizeGLError(GraphicsContext3D::INVALID_OPERATION); > + return; > + } > + Could you please refactor these tests into a helper function similar to validateDrawMode?
(In reply to comment #2) > (From update of attachment 77631 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=77631&action=review > > Looks good; one cleanup upon landing please. > > > WebCore/html/canvas/WebGLRenderingContext.cpp:1119 > > + if (m_stencilMask != m_stencilMaskBack || m_stencilFuncRef != m_stencilFuncRefBack || m_stencilFuncMask != m_stencilFuncMaskBack) { > > + m_context->synthesizeGLError(GraphicsContext3D::INVALID_OPERATION); > > + return; > > + } > > + > > Could you please refactor these tests into a helper function similar to validateDrawMode? Will do.
Committed r74818: <http://trac.webkit.org/changeset/74818>
Again, webkit-patch land does not close the bug automatically and I had to manually add the committed message and close the bug. This is the second time this happened. Both time I called webkit-patch land remotely through PUTTY.
http://trac.webkit.org/changeset/74818 might have broken SnowLeopard Intel Release (Tests)
Committed r74820: <http://trac.webkit.org/changeset/74820>
(In reply to comment #7) > Committed r74820: <http://trac.webkit.org/changeset/74820> I had to commit test changes a second time. I really don't understand what's happening here. Seems like r74818 only picked up changes under WebCore but not under LayoutTests; however, it picked up both change logs. Really strange. Again, I did the two commits through PUTTY.
(In reply to comment #8) > (In reply to comment #7) > > Committed r74820: <http://trac.webkit.org/changeset/74820> > > I had to commit test changes a second time. I really don't understand what's happening here. Seems like r74818 only picked up changes under WebCore but not under LayoutTests; however, it picked up both change logs. Really strange. > > Again, I did the two commits through PUTTY. I'm not sure this could be the issue, but I always run webkit-patch land while cd'd into the top-level WebKit directory, just like svn-create-patch.