[Webkit-unassigned] [Bug 57154] [Qt] Make OpenGL symbol resolver transparent

[bugzilla-daemon at webkit.org]

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





--- Comment #38 from Andrew Wason <rectalogic at rectalogic.com>  2011-07-19 13:27:55 PST ---
(In reply to comment #36)
> > Source/WebCore/WebCore.pro:3619
> > +    CONFIG += opengl-shims
> 
> Indentation seems a bit off.

Actually, the existing indentation of the INCLUDEPATH directly above my CONFIG
was wrong, so I fixed that when I added the new CONFIG. Both these should line
up with the SOURCES further above (that you can't see in the diff),
not with the filenames themselves that are the indented value of SOURCES.

> > Source/WebCore/platform/graphics/cairo/OpenGLShims.cpp:20
> > +#if ENABLE(WEBGL) || (PLATFORM(QT) && USE(TEXTURE_MAPPER_GL))
> 
> This is a bit awkward. Maybe add a define QT_OPENGL_SHIMS inside the contains(CONFIG...) clause?
> Otherwise we repeat the build-time logic that decides whether to use it twice.

I'll add QT_OPENGL_SHIMS.

> > Source/WebCore/platform/graphics/cairo/OpenGLShims.cpp:34
> > +#define ASSIGN_FUNCTION_TABLE_ENTRY(FunctionName, success) \
> 
> Strange to use uppercase for FunctionName and lowercase for success.

I agree, but the existing definition of ASSIGN_FUNCTION_TABLE_ENTRY already used those names
(see a few lines below in the diff).
Should I fix the existing definition and my new definition, or leave my new definition
consistent with the existing one?

> > Source/WebCore/platform/graphics/cairo/OpenGLShims.cpp:46
> > +    return QGLContext::currentContext()->getProcAddress(QLatin1String(procName));
> 
> This doesn't buy you anything. getProcAddress(procName) is essentially the
> same thing and is somewhat more readable.

I'll remove QLatin1String.

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