[webkit-reviews] review denied: [Bug 78672] [Texmap] Move TextureMapperGL to use GraphicsContext3D : [Attachment 157911] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 13 11:36:18 PDT 2012


Noam Rosenthal <noam.rosenthal at nokia.com> has denied Roland Takacs
<rtakacs at inf.u-szeged.hu>'s request for review:
Bug 78672: [Texmap] Move TextureMapperGL to use GraphicsContext3D
https://bugs.webkit.org/show_bug.cgi?id=78672

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

------- Additional Comments from Noam Rosenthal <noam.rosenthal at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=157911&action=review


I have no issues with the actual patch - it's beginning to look real good. See
Kenneth's comments though :)

>> Source/WebCore/ChangeLog:169
>> +	    Reviewed by NOBODY (OOPS!).
> 
> Why does this include other changelog entries?

I think it would be best to squash these to one changelog entry, and to say
that it's based on a previous patch by Helder Correia.

>> Source/WebCore/platform/graphics/GraphicsContext3D.h:482
>> +	static PassRefPtr<GraphicsContext3D>
createWrapperForCurrentGLContext();
> 
> I would remove Wrapper from the name. createForCurrentGLContext() seems
descriptive enough for me given the class name

Either is OK with me, let's go with what Kenneth is suggesting :)


More information about the webkit-reviews mailing list