[webkit-reviews] review denied: [Bug 37118] WebGLUniformLocation objects must be invalidated during linkProgram : [Attachment 75838] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Dec 8 11:47:32 PST 2010
Kenneth Russell <kbr at google.com> has denied Zhenyao Mo <zmo at google.com>'s
request for review:
Bug 37118: WebGLUniformLocation objects must be invalidated during linkProgram
https://bugs.webkit.org/show_bug.cgi?id=37118
Attachment 75838: patch
https://bugs.webkit.org/attachment.cgi?id=75838&action=review
------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=75838&action=review
Aside from the comments above the logic looks okay, but I think the term "link
signature" is confusing. I'd suggest "link count" or
"numberOfLinkProgramCalls".
> WebCore/html/canvas/WebGLUniformLocation.cpp:52
> + if (!m_program || m_program->getLinkSignature() != m_linkSignature)
Since the constructor asserts that m_program is non-NULL, checking against
!m_program is unnecessary.
> WebCore/html/canvas/WebGLUniformLocation.cpp:62
> + return -1;
I don't think silently returning -1 here is a good idea. The location -1 is
treated specially by OpenGL -- any call like glUniform3f taking a location of
-1 is silently ignored. If the WebGLRenderingContext ends up calling this then
it will likely produce wrong results instead of synthesizing the error it
should. I think this entry point should probably ASSERT that the link
signatures match.
More information about the webkit-reviews
mailing list