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.72k stars 343 forks source link

Fix warnings and MSVC errors #991

Closed acolwell closed 1 month ago

acolwell commented 4 months ago

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:

PR Description

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

What does this pull request do?

This change fixes various issues that prevent the code from compiling with the MSVC compiler.

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

Yes. The installer still builds and all the tests still pass. This change doesn't really change any behavior. It just makes slight adjustments to make the MSVC compiler happy.

Futher details of this pull request

This fixes MSVC compiling issues, but I do not have a full MSVC build working yet.

The HostOverlay fixes resolve the compiler errors I was encountering and preserves existing behavior (i.e. turns off "autokeying" when setting the value). It is not clear to me that this is correct since all the other setValue() calls near the ones that where fixed appear to pass in a KeyFrame object. I'm not familiar enough with this code to determine if this is correct or not so I just opted for preserving existing behavior for now. If we do decide to provide a KeyFrame object in the future, this should be easy to do in the new code.

rodlie commented 3 months ago

Looks ok, but note that I don't have anything setup to test Natron with MSVC. I do maintain a couple of applications that uses MSVC and Qt5, but I'm still on MSVC 2017 (15).

The HostOverlay fixes resolve the compiler errors I was encountering and preserves existing behavior (i.e. turns off "autokeying" when setting the value). It is not clear to me that this is correct since all the other setValue() calls near the ones that where fixed appear to pass in a KeyFrame object. I'm not familiar enough with this code to determine if this is correct or not so I just opted for preserving existing behavior for now. If we do decide to provide a KeyFrame object in the future, this should be easy to do in the new code.

I'm also not familiar with this code, so not much help here.