-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ogre2: explicitly request OpenGL 3.3 core profile context. #244
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @iche033 to take a second pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me. Just some very minor coding style comments so that they are consistent with ignition coding style
ogre2/src/Ogre2RenderEngine.cc
Outdated
(PFNGLXCREATECONTEXTATTRIBSARBPROC) | ||
glXGetProcAddress((const GLubyte*)"glXCreateContextAttribsARB"); | ||
|
||
if (glXCreateContextAttribsARB) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{
bracket on a new line
ogre2/src/Ogre2RenderEngine.cc
Outdated
glXCreateContextAttribsARB(x11Display, | ||
this->dataPtr->dummyFBConfigs[0], nullptr, | ||
1, context_attribs); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else
on new line and {
also on it's own line, ie.e
}
else
{
...
}
ogre2/src/Ogre2RenderEngine.cc
Outdated
glXGetProcAddress((const GLubyte*)"glXCreateContextAttribsARB"); | ||
|
||
if (glXCreateContextAttribsARB) { | ||
int context_attribs[] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use camelCase, i.e. contextAttribs[]
This change raises system requirements as it uses GLX 1.3 API and a GLX extension. Modern systems like Ubuntu bionic and focal satisfies these without issue. Signed-off-by: Hill Ma <hillma@google.com>
3d6f254
to
fdca12d
Compare
done |
This change raises system requirements as it uses GLX 1.3 API and a GLX extension. Modern systems like Ubuntu bionic and focal satisfies these without issue. Signed-off-by: Hill Ma <hillma@google.com>
This change raises system requirements as it uses GLX 1.3 API and a GLX extension. Modern systems like Ubuntu bionic and focal satisfies these without issue.
Tested on bionic without GPU (QEMU) and focal with GPU.
Bug Report
Fixes issue #128 and similar issues.
Summary
Newer GLX APIs are used to explicitly request core profile GL context that Ogre2 uses.
Reference:
https://www.khronos.org/opengl/wiki/Tutorial:_OpenGL_3.0_Context_Creation_(GLX)
Reproduce:
Run
ign gazebo shapes.sdf
with the default ogre2 renderer on systems with non-AMD (e.g. Intel) mesa GL drivers or in situations where the hardware GPU is not accessible (e.g. QEMU VM).The original code has a decent chance to fail due to it attempting to create OpenGL context in compatibility profile and the GL version of context created would be below the required 3.3.
Checklist
codecheck
passed (Seecontributing)
test coverage)
another open pull request
to support the maintainers
Note to maintainers: Remember to use Squash-Merge