NatronGitHub / Natron

Open-source video compositing software. Node-graph based. Similar in functionalities to Adobe After Effects and Nuke by The Foundry.
http://NatronGitHub.github.io
GNU General Public License v2.0
4.56k stars 333 forks source link

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

Closed devernay closed 2 years ago

devernay commented 2 years ago

@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 https://github.com/NatronGitHub/Natron/pull/765

(looking for a volunteer to work on this)

YakoYakoYokuYoku commented 2 years ago

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 commented 2 years ago

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

https://github.com/NatronGitHub/Natron/blob/e6fd45987558d3d18d8fdaf240b6dd603f5a56d7/Engine/Settings.cpp#L419-L427

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

https://github.com/NatronGitHub/Natron/blob/dbd7fb08016f18de7cac919ff8aa336e29c36fe9/Engine/GPUContextPool.cpp#L102-L109

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 commented 2 years ago

Thank you for taking a closer look!

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

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

YakoYakoYokuYoku commented 2 years ago

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 commented 2 years ago

I think we should replace this PR by:

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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