Skip to content
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

[SofaKernel/SofaGuiQt] Qt viewer with intel drivers #1690

Merged

Conversation

jnbrunet
Copy link
Contributor

@jnbrunet jnbrunet commented Jan 2, 2021

We got this error many times in the past, and I never had the chance to dig into the issue. There was actually two issues:

  1. SOFA was unable to find some GL extensions on Intel drivers with the Qt viewer while they were supposed to be there (they were correctly found when using the qglviewer)
  2. When no extensions at all were found by Glew, SOFA tried to convert a null pointer to a std::string, which created a segmentation fault

This PR fixes both of these issues. It first make sure that the pointer returned by Glew isn't null before converting it to a std::string. It also asks Qt to enable deprecated functions, which are used by SOFA.

In addition, I've removed the part of the Qt viewer that was forcing the environment variable MESA_GL_VERSION_OVERRIDE on Linux. I've also remove the error when this environment variable wasn't set with Intel driver (which was never the case because we were forcing it...) and added a warning instead (Qt viewer works fine on Intel drivers, maybe this was an old bug?).

Will most probably fix #1567


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

Adds the DeprecatedFunctions option to the Qt widget to let
recent drivers support the old extensions used by SOFA

Remove the forced Mesa OpenGL version 3.0 and warn the user instead of triggering an error.
@jnbrunet jnbrunet added pr: fix Fix a bug pr: status to review To notify reviewers to review this pull-request issue: bug (minor) Bug affecting only some users or with no major impact on the framework labels Jan 2, 2021
@jnbrunet jnbrunet added this to the v20.12 milestone Jan 2, 2021
@jnbrunet jnbrunet requested a review from fredroy January 2, 2021 14:17
@jnbrunet jnbrunet linked an issue Jan 2, 2021 that may be closed by this pull request
@jnbrunet
Copy link
Contributor Author

jnbrunet commented Jan 3, 2021

[ci-build][with-all-tests]

@fredroy
Copy link
Contributor

fredroy commented Jan 4, 2021

👍
I remember it was at a time where Intel driver and Linux was crashing #526 with Core profile (I guess?)
Maybe a bug in mesa at that time or the intel driver.
Anyway we would need people using Intel & Linux to test this on their setup.

@jnbrunet
Copy link
Contributor Author

jnbrunet commented Jan 4, 2021

+1
I remember it was at a time where Intel driver and Linux was crashing #526 with Core profile (I guess?)
Maybe a bug in mesa at that time or the intel driver.
Anyway we would need people using Intel & Linux to test this on their setup.

Thanks for linking this issue, I wasn't aware.

I wouldn't be surprise that the crash that was initially happening was for the same reason as the one for which I made this PR (the 0 GL extension -> std::string(nullptr) -> crash). With this PR, such a crash won't happen anymore. In addition, the extensions will now be found due to the magic call to f.setOption(QSurfaceFormat::DeprecatedFunctions, true);

@hugtalbot hugtalbot changed the title Qt viewer with intel drivers [SofaKernel/SofaGuiQt] Qt viewer with intel drivers Jan 4, 2021
@fredroy fredroy added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Jan 6, 2021
@fredroy
Copy link
Contributor

fredroy commented Jan 6, 2021

If anybody still does have a problem with Intel/OpenGL/Mesa, please open an issue.

@fredroy fredroy merged commit 6b4ef54 into sofa-framework:master Jan 6, 2021
jnbrunet added a commit that referenced this pull request Jan 11, 2021
* [SofaHelper] Fix seg fault when no OpenGL extensions are found

* [QtViewer] Fix unsupported intel drivers

Adds the DeprecatedFunctions option to the Qt widget to let
recent drivers support the old extensions used by SOFA

Remove the forced Mesa OpenGL version 3.0 and warn the user instead of triggering an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug (minor) Bug affecting only some users or with no major impact on the framework pr: fix Fix a bug pr: status ready Approved a pull-request, ready to be squashed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runSofa fails with mesa graphics on AMD
2 participants