Some fixes for 2.6 #1039

cedricp · 2025-04-22T15:13:27Z

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:

I've followed the contributing guidelines to the best of my understanding
I've read and understood the contributing guidelines
I've formatted my code according to Natron's code style
I've searched the pull requests tracker to ensure that this PR is not a duplicate

PR Description

What type of PR is this? (Check one of the boxes below)

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (non-breaking change which does not add functionality nor fixes a bug but improves Natron in some way)
Breaking change (fix or feature that would cause existing functionality to change)
My change requires a change to the documentation
- I have updated the documentation accordingly

What does this pull request do?

Fixes viewer when it's undocked (floating) on Windows, the GL context seemed to be lost
Add CMake script to install MINGW libraries on the binary dir
Fix application icon that was not set
Fix python script to build with VScode
CMake call windeployqt.exe to install plugins and libs

Show a few screenshots (if this is a visual change)

[Your answer, delete this section if it is not a visual change.]

Have you tested your changes (if applicable)? If so, how?

[Your answer]

Futher details of this pull request

[Your answer]

acolwell

Thank you for trying to help improve the Windows build. I just have minor questions and comments.

acolwell · 2025-04-25T01:29:46Z

App/CMakeLists.txt

 # ***** END LICENSE BLOCK *****

 set(Natron_SOURCES NatronApp_main.cpp)
-if(WINDOWS)


I believe you should be able to just change this to WIN32. I don't think you need to change or move anything else. Also could you please do a similar WINDOWS -> WIN32 substitution in Renderer/CMakeLists.txt and libs/gflags/CMakeLists.txt . Both of these are a similar situation and are just using the wrong cmake variable for the if statement.

If you just put these WINDOWS -> WIN32 changes in a PR I'm happy to approve that right away or I can just do that myself if you don't mind.

acolwell · 2025-04-25T01:33:29Z

App/CMakeLists.txt

+    SET(MINGWPATH "${MINGW_CC_PATH}")
+    INSTALL(CODE "set(CMAKELIBPATH \"${CMAKE_SYSTEM_LIBRARY_PATH};${CMAKE_MINGW_SYSTEM_LIBRARY_PATH};${MINGWPATH}\")")
+    INSTALL(CODE "SET(MINGWPATH ${MINGWPATH})")
+    INSTALL(CODE "SET(INSTALL_BINPATH ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR})")


I believe you need to add " before and after the ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR} to avoid a cmake error related to spaces in the install path (e.g. C:/Program Files (x86)/Natron/bin)

acolwell · 2025-04-25T01:36:54Z

Gui/ViewerGLPrivate.cpp

    // always running in the main thread
    assert( qApp && qApp->thread() == QThread::currentThread() );
-    _this->makeCurrent();
+    //_this->makeCurrent();


This feels like it is treating a symptom instead of actually fixing the underlying issue. I'm happy to look into this if you file an issue with repro steps. For now how about removing it from this PR so we only have build changes here and no functional changes?

This has not indeed worked for long (don't know why this worked, though). I found a better fix for that : setting the AA_ShareOpenGLContexts to QCoreApplication. But this work when undocking only, when redocking, the widget is screwed up.

I suspect this is related to calls not happening on the expected threads. I've run into similar situations in the past in the Natron code. I suspect it worked for you sometimes because the "right" GL context was still current but then in other situations it wasn't set anymore. Once I'm able to reproduce this issue on my end I'll probably be able to give you more details. Could you please send me a description or a screen capture of how you are triggering the issues you are seeing.

Yes, when the viewer (or any other GL widget) is undocked (becomes a floating panel), the content is missing textures or is totally black. Is this working for you ? I tested on several machines with different GPU and the problem is the same.

acolwell · 2025-04-25T04:46:30Z

App/CMakeLists.txt

+        )
+        MESSAGE(STATUS "Resolving runtime dependencies for ${TARGET_APP}")
+        FOREACH(dep ${deps_resolved})
+        FILE(INSTALL ${dep} DESTINATION ${CMAKE_INSTALL_PREFIX}/bin)


Indent FOREACH bodies here and below to make them visually stand out?

acolwell · 2025-04-25T04:55:15Z

App/CMakeLists.txt

+    file(GENERATE OUTPUT
+    "${CONFIG_FILE}" CONTENT 
+    [[
+        EXECUTE_PROCESS(COMMAND ${MINGWPATH}/windeployqt.exe ${INSTALL_BINPATH}/Natron.exe)


I think you might need to put this line after the FILE(GET_RUNTIME_DEPENDENCIES) command. When I try building the code with it in this order, I get and error saying "Multiple conflicting paths found for Qt5Core.dll:" because it looks like the GET_RUNTIME_DEPENDENCIES code doesn't like that windeployqt has copied Qt5Core.dll into the bin directory already.

I saw that and it's fixed now on my side

acolwell · 2025-04-25T05:00:40Z

App/CMakeLists.txt

+        FILE(GET_RUNTIME_DEPENDENCIES
+            RESOLVED_DEPENDENCIES_VAR deps_resolved
+            UNRESOLVED_DEPENDENCIES_VAR deps_unresolved
+            LIBRARIES ${TARGET_APP}


Shouldn't this be EXECUTABLES instead of LIBRARIES since we are getting dependencies for the Natron executable?

Perhaps, it's working as it is, but you may be right

I don't actually know how are the MINGW libs deployed to make a release, the problem I have now is that the libraries of the OFX plugins need to be installed in the application directory to be found. If they are installed beside the plugin binaries, the linker doesn't find them. A search path issue I guess...

Tested with EXECUTABLES and it works

FYI the cmake build isn't the way full blown installer builds are done right now. Eventually I'd like to get there, but there are a number of things that need to be overcome as you have encountered. I suspect trying to get everything to work in cmake is not your primary goal. What specifically are you trying to accomplish here?

FWIW my current workflow is to build using the install scripts used by the continuous integration github actions (i.e. the scripts in tools/jenkins). That gives me a full installer build. Then if I'm just working on Natron, I do a cmake build and copy the resulting binaries into that full installer build. It's not great, but it works well enough for now. I am working on ways to improve this, but I just don't have a ton of time to rework all this at the moment.

OFX plugins with dependencies don't have to always put them in the application directory. You can have them live next to the plugin, but you have to do some manifest stuff on Windows to make it work. Take a look at this code in the openfx-io Windows CI build action for an example of how a dependency manifest is created and applied.

acolwell · 2025-04-25T05:04:37Z

tools/utils/sourceList.py

    sources.extend([f"{typename.lower()}_wrapper.cpp" for typename in types])

-    return [os.path.normpath(os.path.join(out, package, f)) for f in sources]
+    return [os.path.normpath(os.path.join(out, package, f)).replace("\\", "/") for f in sources]


Why do you need this? I was able to build with VS code w/o it. I'm assuming you are trying to deal with Windows native paths, but it's not clear to me why you are having to deal with those here. Where are the backslashes coming from?

This is the error I get when building with VScode :

[cmake] CMake Error at Engine/CMakeLists.txt:69 (add_library): [cmake] Syntax error in cmake code when parsing string [cmake] [cmake] D:\Development\sources\Natron\BUILD\Engine\Qt5\NatronEngine\natronengine_module_wrapper.cpp [cmake] [cmake] Invalid character escape '\D'.

Note that there's no build issue with a MINGW64 bash shell

Hmm... I wonder if there is a mismatch between which python and/or cmake is being invoked. Maybe adding "COMMAND_ECHO STDERR" to the execute_process call that runs this might provide some clues. It also might be good to look up earlier in the build output and make sure that proper python and cmake binaries are being used.

cedricp · 2025-04-25T07:11:20Z

You're right, there's some issues with this PR, just close it, I'm working on a cleaner one.

Cedric PAILLE added 3 commits April 22, 2025 16:56

[WIN32] Add installer dependency libs

0d09b7c

Fix build for VSCode

98e5813

Fix GL context when widget become floating

c4d3aa3

acolwell reviewed Apr 25, 2025

View reviewed changes

cedricp closed this Apr 25, 2025

acolwell mentioned this pull request May 20, 2025

2.6 debug #1040

Open

10 tasks

Some fixes for 2.6 #1039

Some fixes for 2.6 #1039

Uh oh!

Conversation

cedricp commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

Uh oh!

acolwell left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cedricp Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cedricp Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cedricp commented Apr 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cedricp commented Apr 22, 2025 •

edited

Loading

acolwell left a comment •

edited

Loading

cedricp Apr 25, 2025 •

edited

Loading

cedricp Apr 25, 2025 •

edited

Loading