We are currently using glGetIntegerv() with GL_MAJOR_VERSION and GL_MINOR_VERSION to get the opengl version, but this is not compatible with OpenGL < 3.0. We need to use glGetString(GL_VERSION) instead, which is compatible with all versions (so far).
Created attachment 295849 [details] Patch
Comment on attachment 295849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295849&action=review > Source/WebCore/platform/graphics/GLContext.cpp:168 > + // The version number always has the form "mayor.minor.release". mayor -> major. Nit, since I'm leaving other comments on this comment: vendor-specific with a hyphen. Nit: If you want to leave a line break in the middle of the comment then you should leave an extra blank line as well. I'd prefer to write this comment on just two lines. Looks like it fits: // Version string always begins with the version number, then a space and the vendor- // specific info. The version number always has the form "major.minor.release". Alternatively (less-preferred): // Version string always begins with the version number, then a space and the vendor- // specific info. // // The version number always has the form "major.minor.release". > Source/WebCore/platform/graphics/GLContext.cpp:174 > + versionStringComponents.at(0).split('.', versionDigits); > + m_version = versionDigits.at(0).toUInt() * 100 + versionDigits.at(1).toUInt() * 10; Looks like it's going to crash if the version does not have a . character. I guess it will always exist, but is it guaranteed, or should we be robust to that?
Comment on attachment 295849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295849&action=review > Source/WebCore/platform/graphics/GLContext.cpp:167 > + // Version string always begins with the version number, then a space and then vendor > + // specific info. I think this is not always true, in case of OpenGL ES, the version string starts with "OpenGL ES", see https://www.khronos.org/opengles/sdk/1.1/docs/man/glGetString.xml. >> Source/WebCore/platform/graphics/GLContext.cpp:168 >> + // The version number always has the form "mayor.minor.release". > > mayor -> major. > > Nit, since I'm leaving other comments on this comment: vendor-specific with a hyphen. > > Nit: If you want to leave a line break in the middle of the comment then you should leave an extra blank line as well. I'd prefer to write this comment on just two lines. Looks like it fits: > > // Version string always begins with the version number, then a space and the vendor- > // specific info. The version number always has the form "major.minor.release". > > Alternatively (less-preferred): > > // Version string always begins with the version number, then a space and the vendor- > // specific info. > // > // The version number always has the form "major.minor.release". And this is not true either, it could be major.minor.release or just major.minor see https://www.opengl.org/wiki/GLAPI/glGetString
Created attachment 295934 [details] Patch
Created attachment 295937 [details] Patch
Comment on attachment 295937 [details] Patch Clearing flags on attachment: 295937 Committed r209234: <http://trac.webkit.org/changeset/209234>
All reviewed patches have been landed. Closing bug.
Is there seriously no forwards-compatible way to check OpenGL version? What we committed looks pretty fragile.