[Webkit-unassigned] [Bug 75466] Remove style warning in GraphicsContext3DOpenGL.cpp

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=75466





--- Comment #5 from ChangSeok Oh <kevin.cs.oh at gmail.com>  2012-01-04 00:03:32 PST ---
(From update of attachment 120923)
View in context: https://bugs.webkit.org/attachment.cgi?id=120923&action=review

Thanks for review.

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:887
>>      // FIXME: This is not implemented on desktop OpenGL. We need to have ifdefs for the different GL variants
> 
> Nit: This comment is missing a period at the end of the line. Comments should be full sentences as per the WebKit style guide.

Done.

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:889
>> +    // ::glReleaseShaderCompiler();
> 
> We shouldn't commit commented out code. Instead of fixing the style issue on this line, I suggest we remove this line entirely because it's commented out code and it's clear from the name of this method (whose name coincides with a similarly named OpenGL call) that the implementation of this method will need to inform OpenGL to deallocate its internal resources associated with the shader compiler.

Removed glReleaseShaderCompiler() and added notImplemented() newly.

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1276
>> +        // Let OpenGL handle these.
> 
> This comment is inane as it doesn't explain "why" we should use OpenGL handle these cases. Either we should improve this comment or remove it.

Removed.

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1291
>> +        (*value) = getShaderInfoLog(shader).length();
> 
> Why the () around *value?

Removed ().

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1310
>> +        return "";
> 
> It is sufficient to call the default String() constructor here instead of the C-string variant:
> 
> return String();

Done.

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1315
>> +        GLint length;
> 
> If you're going to re-indent this, you might as well make it use early-return instead.

Done.

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1318
>> +            return "";
> 
> Ditto.

Done.

>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1321
>> +        GLchar* info = (GLchar*) fastMalloc(length);
> 
> Can we use OwnArrayPtr here? Then we can get rid of the fastFree() call on line 1326.

It looks no problem. So I did.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.