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

BUG: Use OpenGL compatibility profile on Windows by default #1144

Closed

Conversation

lassoan
Copy link
Contributor

@lassoan lassoan commented May 29, 2019

On Windows, OpenGL compatibility profile is used by default.
For troubleshooting purposes, compatibility/core profile can be explicitly selected by modifying General/OpenGLProfile application settings key (valid values: default, compatibility, core).

Implemented OpenGL profile mode setting in qMRMLWidget so that it can be accessed easily from all tests. It is in postInitializeApplication() method to allow configuring the surface format based on application settings (which are not available before the application object is created).

On Windows, OpenGL compatibility profile is used by default.
For troubleshooting purposes, compatibility/core profile can be explicitly selected by modifying General/OpenGLProfile application settings key (valid values: default, compatibility, core).

Implemented OpenGL profile mode setting in qMRMLWidget so that it can be accessed easily from all tests. It is in postInitializeApplication() method to allow configuring the surface format based on application settings (which are not available before the application object is created).
@lassoan lassoan requested review from jcfr and pieper May 29, 2019 02:00
@jcfr
Copy link
Member

jcfr commented May 29, 2019

Thanks for working on this.

@pieper
Copy link
Member

pieper commented May 29, 2019

LGTM. I didn't build but could try a mac build later today if it seems needed. This also cleans up the code a lot 👍

@lassoan
Copy link
Contributor Author

lassoan commented May 29, 2019

There should be no change in behavior on Mac or Linux, still the core configuration is used on those platforms by default. On Mac there is no full support for compatibility profiles, so probably core profile must be used. On Linux, using compatibility profile may help (it can be enabled by editing Slicer.ini).

@jcfr
Copy link
Member

jcfr commented May 29, 2019

There should be no change in behavior on Mac or Linux

Agreed. On my side, compilation was done as a sanity check.

On Mac there is no full support for compatibility profiles, so probably core profile must be used. On Linux, using compatibility profile may help (it can be enabled by editing Slicer.ini).

Should we display a warning on these platforms in case the user/developer change the profile ... this could save time troubleshooting in the future ?

@pieper
Copy link
Member

pieper commented May 29, 2019

On Mac there is no full support for compatibility profiles, so probably core profile must be used. On >> Linux, using compatibility profile may help (it can be enabled by editing Slicer.ini).

Should we display a warning on these platforms in case the user/developer change the profile ... this > could save time troubleshooting in the future ?

If we know it's not a valid setting we could just disallow it.

@lassoan
Copy link
Contributor Author

lassoan commented May 30, 2019

There is no GUI for changing this setting, so I think it is fine to allow forcing any profile on any platform. Maybe we could print the current choice to the application log (it's a bit tricky, as it happens very early during startup but maybe we can query it later).

@jcfr do you plan to merge this yourself or I should do it?

@jcfr
Copy link
Member

jcfr commented May 30, 2019

do you plan to merge this yourself or I should do it?

First, I will do an incremental build on macOS and will let you know so that you can integrate

maybe we can query it later

What about displaying the current mode in the application info here: https://github.com/Slicer/Slicer/blob/62ed8d6d406f8828abff3b8a857073efb2ae1aaa/Base/QTGUI/qSlicerApplication.cxx#L828-L847

@jcfr
Copy link
Member

jcfr commented May 30, 2019

an incremental build on macOS and will let you know so that you can integrate

All set. No problem, you can go ahead and integrated.

@lassoan
Copy link
Contributor Author

lassoan commented May 30, 2019

I did some more testing and Qt logs a warning if we change the default surface format after the QApplication object is created. However, QApplication needs to be set up to be able to access QSettings.

So, it seems that we cannot make OpenGL profile configurable from application settings after all (unless we tolerate the warning on each startup, which I don't think we should).

Should I add a command-line option for forcing core profile on Windows (for troubleshooting and easier experimentation) or we should not pollute the command-line interface with such experimental option? I would vote for the latter: less code to implement, maintain, and document; and we would probably not use it anyway.

@pieper
Copy link
Member

pieper commented May 30, 2019

A command line option to force the mode would be okay (although I agree the command line options are already unwieldy).

As an alternative, couldn't you create a QApplication (or a QCoreApplication actually) just to access the settings and then create the real one once you know the desired surface format?

@jcfr
Copy link
Member

jcfr commented May 30, 2019

QApplication needs to be set up to be able to access QSettings

Was thinking about writing the setting in the launcher settings. For consistency the file is also distributed on macOS. See qSlicerCoreApplication::launcherSettingsFilePath()
To write the value, you could pass a different setting object to registerProperty

Now to read the setting before the application, a simple regex would allow to extract the value.

This settings could be changed only if user is admin or has write access to where slicer is installer/.

As an alternative, couldn't you create a QApplication (or a QCoreApplication actually) just to access the settings and then create the real one once you know the desired surface format?

Unlikely i think, you can have only one instance of the app in the process.

@pieper
Copy link
Member

pieper commented May 30, 2019

As an alternative, couldn't you create a QApplication (or a QCoreApplication actually) just to access the settings and then create the real one once you know the desired surface format?

Unlikely i think, you can have only one instance of the app in the process.

I think you can create one and then delete it, but it's not common behavior and it may be a lot of overhead for this purpose.

Anyway since this is more of a developer debug thing anyway, an environment variable could be the easiest.

@jcfr
Copy link
Member

jcfr commented May 30, 2019

environment variable could be the easiest.

👍

@lassoan
Copy link
Contributor Author

lassoan commented May 31, 2019

Environment variable is a great idea! I've implemented that. I've also implemented printing of requested OpenGL version and profile during startup.

Committed r28277.

@lassoan lassoan closed this May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants