(Bug): check why OpenGL rendering is disabled in NatronRenderer

[devernay]

@YakoYakoYokuYoku reported here that OpenGL rendering seems to be disabled when using NatronRenderer, even when a display is available and detected.

I'd like to get an answer to this before testing and merging #765

(looking for a volunteer to work on this)

[YakoYakoYokuYoku]

This was spotted while using Shadertoy, the command output displays a kOfxStatFailed and the stack traces points to the source openfx-misc/Shadertoy/Shadertoy.cpp:1312, previously from there it checks on args.openGLEnabled, which corresponds to the kOfxImageEffectPropOpenGLEnabled property, to then perform the render. The documentation of kOfxImageEffectPropOpenGLEnabled in openfx/include/ofxOpenGLRender.h mentions that when a plugin and a host both establish that they're capable for OpenGL rendering then this property is enabled, otherwise it's disabled. Upon this it concludes that it's either one or both of those who are at fault for not testing OpenGL correctly.

[YakoYakoYokuYoku]

I might be a little bit late but I think I've found the root cause of this. The following snippet is the culprit

Natron/Engine/Settings.cpp

Lines 419 to 427 in e6fd459

	bool
	Settings::isOpenGLRenderingEnabled() const
	{
	if (_enableOpenGL->getIsSecret()) {
	return false;
	}
	EnableOpenGLEnum enableOpenGL = (EnableOpenGLEnum)_enableOpenGL->getValue();
	return enableOpenGL == eEnableOpenGLEnabled || (enableOpenGL == eEnableOpenGLDisabledIfBackground && !appPTR->isBackground());
	}

Currently we have three settings for enabling OpenGL rendering, namely eEnableOpenGLEnabled, eEnableOpenGLDisabled and
eEnabledOpenGLDisabledIfBackground. The engine checks the enableOpenGLRendering knob in
Settings::isOpenGLRenderingEnabled is equal to eEnableOpenGLEnabled and it also does this with
eEnableOpenGLDisabledIfBackground but this might lead to a wrong result, notably in GPUContextPool::attachGLContextToRender

Natron/Engine/GPUContextPool.cpp

Lines 102 to 109 in dbd7fb0

	OSGLContextPtr
	GPUContextPool::attachGLContextToRender(bool checkIfGLLoaded)
	{
	if (checkIfGLLoaded && (!appPTR->isOpenGLLoaded() || !appPTR->getCurrentSettings()->isOpenGLRenderingEnabled())) {
	return OSGLContextPtr();
	}
	QMutexLocker k(&_imp->contextPoolMutex);

In case we are doing a background rendering the function is going to return a null OpenGL context pointer (OSGLContextPtr)
given that we had OpenGL loaded (which will be true if running NatronRenderer under a desktop environment). This will, in
turn, make the EffectInstance::attachOpenGLContext method not to attach a context, thus disabling OpenGL in the effect
instance and in the OpenFX plugins by consequence.

With what was mentioned above I've tried with making Settings::isOpenGLRenderingEnabled to always return true and this led to a successful render with the Shadertoy plugin in NatronRenderer.

So to provide a fix for this I have two ideas in mind. One is to set enableOpenGLRendering to eEnableOpenGLEnabled by
default but this might be undesired in some cases. The other is to modify GPUContextPool::attachGLContextToRender to
ignore Settings::isOpenGLRenderingEnabled, though this might make it to attach an OpenGL context an any call. Otherwise if
anyone has a better fix then we should go ahead with it.

[devernay]

Thank you for taking a closer look!

What I understand is that Natron does what is expected, based on the enableOpenGLRendering setting:

• if eEnableOpenGLEnabled, use OpenGL rendering for GUI and background rendering
• if eEnableOpenGLDisabledIfBackground, use OpenGL rendering for GUI, but not for background - this is the default because we don't has as much control on what happens on the GPU, and rendering may happen on multi-core CPUs that may starve the GPU

If you set the setting to eEnableOpenGLEnabled in the GUI, is background rendering using OpenGL?

[YakoYakoYokuYoku]

If you set the setting to eEnableOpenGLEnabled in the GUI, is background rendering using OpenGL?

When I set enableOpenGLRendering to eEnableOpenGLEnabled in the Gui and save settings Natron's background rendering does indeed use OpenGL.

So the issue originates from my expectation to have OpenGL enabled when I run NatronRenderer, yet it's disabled in background by default. I'm gonna say in my opinion that enableOpenGLRendering is better set to be enabled by default to avoid this scenario. Though I think that we could warn users when an effect is trying to use OpenGL given it's disabled.

[devernay]

I think we should replace this PR by:

• do not search for OpenGL renderers when eEnableOpenGLDisabledIfBackground and running in background, or eEnableOpenGLDisabled
• output a qDebug message stating that OpenGL rendering is disabled (and give the NatronRenderer command-line flag to enable it)

Hardware-accelerated OpenGL rendering is disabled by default when running in background because different hardware may render differently, and the hardware may not implement floating-point rendering, thus the output quality may be degraded.

For these reasons, users should be extra careful when enabling OpenGL rendering for high-quality renders, and this shouldn't be the default.

[devernay]

The command-line option (for the message) is --setting enableOpenGLRendering=0 (0 is enabled, 1 is disabled, 2 is foreground) or --setting enableOpenGLRendering=\"enabled\"

[YakoYakoYokuYoku]

I think we should replace this PR by:

* do not search for OpenGL renderers when `eEnableOpenGLDisabledIfBackground` and running in background, or `eEnableOpenGLDisabled`

* output a qDebug message stating that OpenGL rendering is disabled (and give the NatronRenderer command-line flag to enable it)

Noticed, will open another PR.

The command-line option (for the message) is --setting enableOpenGLRendering=0 (0 is enabled, 1 is disabled, 2 is foreground) or --setting enableOpenGLRendering="enabled"

I think that a flag alias --opengl "<option>" to --setting enableOpenGLRendering="<option>" could be more suitable here and easier to remember or use.

[devernay]

I think that a flag alias --opengl "<option>" to --setting enableOpenGLRendering="<option>" could be more suitable here and easier to remember or use.

Feel free to make that change too