[webkit-reviews] review denied: [Bug 64878] [Qt] Adopt GraphicsContext3DOpenGL.cpp and ANGLE (part 2) : [Attachment 101609] adopt GraphicsContext3DOpenGL.cpp and ANGLE

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has denied Andrew Wason
<rectalogic at rectalogic.com>'s request for review:
Bug 64878: [Qt] Adopt GraphicsContext3DOpenGL.cpp and ANGLE (part 2)
https://bugs.webkit.org/show_bug.cgi?id=64878

Attachment 101609: adopt GraphicsContext3DOpenGL.cpp and ANGLE
https://bugs.webkit.org/attachment.cgi?id=101609&action=review

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


Comments inline.

> Source/WebCore/WebCore.pro:3641
> +    *g++* {
> +	   QMAKE_CXXFLAGS += -Wno-unused-variable
> +	   QMAKE_CXXFLAGS += -Wno-missing-noreturn
> +	   QMAKE_CXXFLAGS += -Wno-unused-function
> +	   QMAKE_CXXFLAGS += -Wno-parentheses
> +	   QMAKE_CXXFLAGS += -Wno-reorder
> +	   # g++ < 4.3 doesn't support -Wno-type-limits
> +	   system( $$QMAKE_CXX --version | grep -e "\\<[4-9]\\.[3-9]\\.[0-9]" )
{
> +	       QMAKE_CXXFLAGS += -Wno-type-limits
> +	   }

Please explain :)

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:32
> +#if ENABLE(WEBGL) && !defined(QT_OPENGL_ES_2)

So, does this mean we use GraphicsContext3DQt for ES, and
GraphicsContext3DOpenGL for desktop?
Maybe we should make this decision in the pro file rather than pollute the
#ifdef space, especially in files shared by other ports.

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:336
> +    // Always set to 1 for OpenGL ES.

Shouldn't we allow more if we're not in ES?