[webkit-reviews] review denied: [Bug 70989] Attribute and Uniform variable names need translation in shader : [Attachment 166787] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 2 17:49:50 PDT 2012


James Robinson <jamesr at chromium.org> has denied Dean Jackson <dino at apple.com>'s
request for review:
Bug 70989: Attribute and Uniform variable names need translation in shader
https://bugs.webkit.org/show_bug.cgi?id=70989

Attachment 166787: Patch
https://bugs.webkit.org/attachment.cgi?id=166787&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=166787&action=review


> Source/Platform/chromium/public/WebGraphicsContext3D.h:408
> +    virtual WebString translateShaderANGLE(WebGLId shader) { return
WebString(); }

this change will break things since we have implementations of this code in the
chromium repo.	To change any public Chromium API (see the directory name - it
has chromium/public in it) you generally speaking need to do a 2-sided roll,
one to implement the new name of the function in implementations of
WebGraphicsContext3D and then one on the WebKit side to change the exposed
interface name.

For this I'd recommend simply calling getTranslatedShaderSourceANGLE() in
Extensions3DChromium.  I don't think there's an especially compelling reason to
rename this.  Honestly, as far as WebKit style goes I think the "get" is
warranted since this is definitely *not* a simple getter.


More information about the webkit-reviews mailing list