david-cattermole / mayaMatchMoveSolver

A Bundle Adjustment solver for MatchMove related tasks.
https://david-cattermole.github.io/mayaMatchMoveSolver/
Other
101 stars 29 forks source link

Python 2.7 and 3.x and Maya 2022+ Support #217

Closed david-cattermole closed 2 years ago

david-cattermole commented 3 years ago

Problem

The VFX world is finally moving to Python 3.x, and Maya 2022, however mmSolver has so far only been tested and was written with Python 2.7 in mind.

We need to support Maya 2022+ while maintaining support for Maya 2020 and below in the same code-base.

Expected behavior: All tools in mmSolver work as expected in Maya 2022+.

Actual behavior: mmSolver is untested and will likely fail in Maya 2022+.

Software Versions

david-cattermole commented 3 years ago

To get us started this is the official "how-to" guide to porting:

https://docs.python.org/3/howto/pyporting.html

david-cattermole commented 3 years ago

Hello @ktonegawa @bpatchasaheb,

I know both of you are developing tools for mmSolver so I wanted to let you know about the aim of supporting Python 2.7 and 3.x.

If you're not already familiar with the concepts and details of Python 3, please have a read of the official "how-to": https://docs.python.org/3/howto/pyporting.html

Some of the details in the official "how-to" will be difficult or impossible for us because we use Python inside Maya and we do not use setup.py or pip with Maya (although some research is needed for Maya 2022 because we might be able to use pip).

The good news is that mmSolver is not using many Python dependencies (Qt.py and the FFT and Convolution module) and should all have Python 2.x and Python 3.x support already.

As a plan to go forward with Python 3.x support, while maintaining both 2.7 and 3.x support, I'm going to suggest these steps:

1) The use of the __future__ module is mandatory from now on.

For example adding the following lines to new modules:

from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

I will also look to add these lines to all existing modules, as time goes on.

2) Test with Maya 2022 Python 3 and Python 2 for all new code.

One of the best ways to find (obvious) errors is to actually run the code in both environments and see if there are errors.

3) Add more tests for code that does not use the GUI (and can be run with mayapy).

At the very least, all modules in mmSolver.utils.*, mmSolver.ui.* and mmSolver._api.* should have (unit or integration) tests.

Here are some documents on how to run the test suite: https://github.com/david-cattermole/mayaMatchMoveSolver/blob/master/DEVELOPER.md#testing https://github.com/david-cattermole/mayaMatchMoveSolver/blob/master/tests/README.md

The documentation might need to be updated a bit for testing with Maya 2022 and Python 3.x/2.7.

4) Make special note of the migration pain points, and keep them in mind when writing new code.

Especially, make note of the new bytes type: https://docs.python.org/3/howto/pyporting.html#text-versus-binary-data

There isn't much raw maths being done in mmSolver Python code (most is in C++), however we should beware of the changes to 'division': https://docs.python.org/3/howto/pyporting.html#division

Thanks, David

ktonegawa commented 3 years ago

Hi @david-cattermole , thank you for the notice. Read through some of the links lightly, got the jist of it I think. Will keep these in mind.

One thing I wonder though I personally only have access to Maya 2016.5 and 2019, what would be the most effective way to test for Maya 2022?

david-cattermole commented 3 years ago

Hi @ktonegawa,

Yes, that will make things tricky, but we can still reduce the risk of bugs coming up ahead of time by using a few flags and tests.

Some ideas after a quick Google search:

I can set -3 by default for the tests suite, but that will only help for code that has a test.

Also, I can add a script with a preset to auto-lint the Python code. This is something I should have done from day 1 on the project. If I'm feeling fancy I could even create an automated test every time code is pushed to the git server (DevOp stuff ;) )

David

david-cattermole commented 3 years ago

I have documented my suggestions above in #218 and #219, so we can track the progress.

david-cattermole commented 2 years ago

I have committed and pushed changes to allow building on Windows, for Maya 2022.

The changes can be checked out and tested in the build_issue217 branch.

Right now, the main problem is that the start-up mechanism that is currently provided is not working in Maya 2022. This seems to be caused by the changes and addition of "security preferences": https://help.autodesk.com/view/MAYAUL/2022/ENU/?guid=GUID-2176117C-6998-4802-9F9F-1417DF41C661

The C++ plug-in seems to compile perfectly fine without changes, however even when running the start-up code explicitly we get multiple Python 3.x problems.

Therefore, the next 2 big challenges will be:

1) Debugging and possibly re-writing the Startup mechanism to work in both Maya 2022 and previous versions, without users needing to do (any or many) extra steps.

2) Porting over the Python 2.x code to 3.x code.

We may also experience differences in the newer Qt libraries used in Maya 2022, or the differences between PySide and PyQt.

david-cattermole commented 2 years ago

In the build_issue217 branch I have added a few scripts (in Windows only for now) that will help us organise the transition to Python 3 and backwards compatibility with Python 2.7.

First, the standard build scripts will automatically activate and setup the needed Python virtual environment and install all third-party packages that are required for development. This will NOT affect the dependencies of mmSolver at runtime, only during development.

It is important to be in the main project root directory when running these scripts.

To help developers of mmSolver we can activate/deactivate the virtual environment like this (on Windows Command Prompt):

> scripts/python_venv_activate_maya2018.bat
> python
> scripts/python_venv_deactivate_maya2018.bat

This will automatically setup the required Python virtual environments and install all needed developer packages (the list is in requirements-dev.txt). There are scripts for each individual Maya version ("2018" shown above), currently all environments are the same, but in the future this will change, so it's important to activate the environment for the specific Maya version you are testing against.

Additionally, after the virtual environment is active and set up we can now use "2to3", "black" and "pylint" with these wrapper scripts:

> scripts/python_convert_run_2to3.bat
> scripts/python_formatter_run_black.bat
> scripts/python_linter_run_pylint.bat

This should help the transition, but more work is still needed.

david-cattermole commented 2 years ago

I have now added support for Flake8, which seems to return a LOT of errors. I have also merged these changes into the develop branch, to integrate all the different branches together.

For example on Windows:

> scripts/python_venv_activate_maya2018.bat
> scripts/python_linter_run_flake8.bat
> scripts/python_venv_deactivate_maya2018.bat

I have also added some Linux .bash files, which are run during TravisCI. The builds currently error on the develop branch because of all the Python 3 problems. https://app.travis-ci.com/github/david-cattermole/mayaMatchMoveSolver/builds/244882215

I may reduce some of the warnings that are printed (like line length), but the goal now is to fix as many of the Python 3 issues (and general bugs/problems) as possible.

Once the TravisCI builds return green I will be happy, and all future code will then be expected to return green, otherwise it will be rejected.

david-cattermole commented 2 years ago

The develop branch is now mostly working in Maya 2022 with Python 3, on Windows. Python 2.7 is also supported, but needs to be tested more to ensure nothing was broken.

The main outstanding issue is that mmSolver's start-up scripts will not be called by Maya 2022 due to the new security preferences. Get mmSolver to load, you must load the userSetup.py file into the Maya Script Editor and execute it - once done, this will build the mmSolver menus and shelves, as usual.

maxnbk commented 2 years ago

Hello, checking on the status of this. Was about to work on contributing some py3 compatibility stuff, but can see that there is active progress being made and don't want to duplicate work.

david-cattermole commented 2 years ago

Hi @maxnbk,

Yes, this is actively being worked on and has mostly been completed.

The changes will be available for the upcoming v0.4.0 release, along with a number of other features/improvements.

You can check out the latest changes in the develop branch.

I have also released the latest build a few days ago and this is ready for testing (but not for production use): https://github.com/david-cattermole/mayaMatchMoveSolver/releases/tag/v0.4.0.alpha3

v0.4.0.alpha3 has fixes for all known Python 2/3 compatibility issues, so if you're able to test and confirm that or not, it would be great.

As you may know, Python 2/3 compatibility for the 3DEqualizer R7 scripts has already been added in the current stable version v0.3.15.

Thanks, David

maxnbk commented 2 years ago

Thanks for the hard work David, The new structure and namings in the build files is making my current build fail, but after I address those issues I'll have our team give it a test in maya2022 and 3de r7 as best I can.

Maybe unrelated to the py3 issue, but: The way the build scripts have been re-designed is quite painful to use if trying to do an "out of source build", they all assume you are running scripts from the project-root-level, it seems.

Also, if you make the things like "CMAKE_EXE" into "?=", it will take upstream values, but supply a default for unprovided overrides. I can think of many situations where the linux build will not find "cmake3".

maxnbk commented 2 years ago

in 0.4.0.alpha4, my build is breaking when it gets to needing cxxbridge. What's the expected dependency here? I have rust and cargo available, but not sure what I'm meant to pass along. The set(rust_linktime_file "NOT-FOUND") seems to indicate I'm traveling over unpaved road?

david-cattermole commented 2 years ago

Thanks for the feedback, I'll see what I can do to make the build process better/easier.

Just to confirm, what operating system are you running on? I assume Linux / CentOS 7?

I assume you cannot or do not want to use the provided binaries or use the .bash scripts for building? If I can know your build limitations I can hopefully make things easier.

The build system complexity has increased a lot with the addition of Rust code, but I was hoping the default provided Docker files and refactored bash/batch scripts would make life easier.

If you are building on Linux, I highly recommend using a Docker container, as it will set up every in Linux exactly as needed for the build environment without needing to change your host system, and the provided binaries are ABI compatible with Maya. Docker is by far the best thing to happen to the build process in mmSolver in v0.4.0. https://github.com/david-cattermole/mayaMatchMoveSolver/blob/100abb35aec9a6f614048efb2e7c0e5c3c263280/Dockerfile_maya2022#L3

Before building the C++ code, the Rust (internal) crates must be compiled, in order, and generate C++ bindings before the main C++ Maya plug-in can be built. I have some code in the build scripts to run this automatically: https://github.com/david-cattermole/mayaMatchMoveSolver/blob/100abb35aec9a6f614048efb2e7c0e5c3c263280/scripts/internal/build_mmSolver_linux.bash#L120

cxxbridge is a Rust executable and must be "installed" to generate C++ bindings for the internal Rust code. You can manually install it with this command: cargo install cxxbridge-cmd --version 1.0.62

Thanks for the tips. I will make the changes for environment variables as you suggested, and I'll see what I can do to build the Rust code "out of source".

Again thanks for the feedback, it's very valuable so I know these issues. David

david-cattermole commented 2 years ago

set(rust_linktime_file "NOT-FOUND") is just saying that the compiled Rust library with C++ bindings cannot be found. If you install cxxbridge and build the crates then rust_linktime_file should be found at the default location, which for me with Maya 2022 is here:

src/mmSolver/mmscenegraph/cppbind/target_maya2022_linux/release/libmmscenegraph.a  # on Linux
src/mmSolver/mmscenegraph/cppbind/target_maya2022_windows64/release/mmscenegraph.lib  # on Windows
david-cattermole commented 2 years ago

Perhaps I should just make mmscenegraph optional for now, since it's currently in beta. You can pass BUILD_MMSCENEGRAPH=0 to CMake, but I suspect the build will still require it right now because I haven't tested that much.

maxnbk commented 2 years ago

Yeah, I got around the scenegraph issue as you described above.

The build environment we're in is it's own docker container because we have our developer toolchain in docker with tooling around it to mount various filesystems and other details correctly to be able to write the finished install back to our central package store. We're mostly behind "corporate firewall don't touch the internet" land, and already had builds of Ceres and other libraries to refer directly to, so I find it easier to just run the cmake directly with the args for locations I expect, but with this version there seem to be some new libraries so I'm starting from the bash scripts to understand what everything is and what I need to add locally.

Sorry for the off-topic chatter to the issue at hand.

(Primary OS, CentOS-7.6)

david-cattermole commented 2 years ago

@maxnbk That all makes sense, I probably should have assumed that. I will write a separate GitHub issue for changes needed to the build system for your use-case, but I want to confirm a few more things if that's ok?

Do you have strong opinions on shared vs static libraries for things like Ceres? I'm currently preferring to have dependencies static if possible because it's making my build scripts easier, and there are less files that can go wrong.

Am I correct to assume your preferred build script would be...

Would those points cover your use-case? What is your ideal situation? Do you want the ability to use the buildthirdpary*.bash scripts, or do you want to have complete control and always build each dependency from scratch?

I'll see what I can do about those steps. I cannot promise I'll have it done very soon.

maxnbk commented 2 years ago

Yes, most of that is solid.

Static-vs-shared makes no difference to me. Whatever is best for the runtime execution of the software.

In general, I don't care too much if there is say, a bash step, then a cmake step, but what gets irritating is telling both steps where my maya is, for example, so I find that it's a better experience when cmake can do it all.

Dependency fetching: Yeah, I think how OpenColorIO does this is pretty solid. have one variable for "allow download dependencies" off/on, flips them all off or on, then you can switch one dep at a time if you want to off or on after that. If it has to be external source fetched and placed inside the build tree, it's very helpful if it's expressed where I need to copy/unpack it to.

Rust code: no preference, frankly we haven't found any packages that yet needed rust code built for them in our pipe, everything we used so far was just standalone tools, so you're a fun test case. I think the simplest way to look at it is, I expect to provide a rust compiler to cmake at the beginning and that the cmake will do what it needs to, to build the rust stuff. Where I get lost is, if I need other rust package dependencies, we haven't really figured out yet how to have those interact in a packaged ecosystem, if that makes sense. So uh, do what makes sense to you, and we'll see how that works, heh!

third-party-dep-optional-install-for-maya: Yeah, I think whatever modules you needed at runtime ought to be installed with the library. If it was shared and linked externally, you should be able to rely on those. Imagine if maya vendored Ceres, and you would maybe have-to or want-to link to their Ceres to ensure compatibility with some internal maya API call, of course you would just link to that and move on with life, but since that isn't the case, you need them. In this studio, it doesn't matter much if I have to build 5 extra deps to satisfy one, but if your library is the only thing I need a dep for, it gets annoying. In this case, I think it would be best if you supply, in the cmake, an explanation of what each library is or does, why it is either optional or required, and your switches for controlling it's static/shared/download/find-package behavior. I don't really understand, as a builder, if the performance or accuracy of the code is suffering if I tell it that a specific solver is "the preferred solver", so a quick "Hey, you probably want X as preferred as it's fastest, but if you're a matchmoving galaxies, maybe use Y because Z" is really helpful.

And speaking of, that's the last thing, you have several FindX.cmake files, but missing them for Ceres and OpenMVG and LDPK I believe. Ideally, I want all packages to be findable by specifying -DPKG_ROOT / -DPKG_DIR , because these are the two shortest / mostly-canonical way to express to cmake "you can find this over here in a nonstandard place", and it cleanly expresses that you'll find the includes and libraries relative, and cmake is happy to do the "lib vs lib64" searching for you, generally, etc.

All that said, as a builder, I deal with more problems for less benefit every day, so don't feel like you need to move heaven and earth to make this easily buildable. My job is to find the workaround I need and often to patch the build to tell it where to find whatever I need to find, etc etc, so do what is best for the project and people will follow suit.

maxnbk commented 2 years ago

An aside, cmakes ExternalProject_Add is really useful for these kinds of situations that you're covering for with the bash scripts.

maxnbk commented 2 years ago

To the python part: Would it be reasonable if the py3 work was spun to a 0.3.16 or an actual 0.4.0 while all the dependency and toolchain tinkering makes it in to a later release? Or are the two chunks of work intertwined?

david-cattermole commented 2 years ago

Thanks for all the great feedback @maxnbk! I will try to address as much as possible soon, and especially for the final v0.4.0 release.

To the python part: Would it be reasonable if the py3 work was spun to a 0.3.16 or an actual 0.4.0 while all the dependency and toolchain tinkering makes it in to a later release? Or are the two chunks of work intertwined?

Right now the two are intertwined, but I could probably split them if I was pressed to do so, the Python 3 changes to the released code is actually quite minimal.

Do you need these back-ported "urgently"? At my current pace I think v0.4.0 could be released in a few months.

The larger issue is that Python 2 build scripts, and documentation generation were hard to keep on Python 2.7 without sacrificing functionality and it was hard to test as well on Windows and Linux. Rather than spending all my time back-porting changes to v0.3.x I wanted to make a larger change that was more maintainable for the future.

david-cattermole commented 2 years ago

Hello @maxnbk,

I have started making the mmscenegraph build changes you describe (currently pushed to the develop branch), but I will need a lot more time to fully implement an OpenColorIO-style build process.

I have decided it will be fairly quick and easy to back-port the Python 3 and Maya 2022 support to the v0.3.x releases (without all the complicated build script updates I have made).

Therefore you should expect a v0.3.16 release with the support you need.

David

maxnbk commented 2 years ago

Understood, thank you David! This will help us out quite a bit. Let me know if there is anything that you could use any contribution or testing.

On Sat, Feb 26, 2022 at 5:38 AM David Cattermole @.***> wrote:

Hello @maxnbk https://github.com/maxnbk,

I have started making the mmscenegraph build changes you describe (currently pushed to the develop branch), but I will need a lot more time to fully implement an OpenColorIO-style build process.

I have decided it will be fairly quick and easy to back-port the Python 3 and Maya 2022 support to the v0.3.x releases (without all the complicated build script updates I have made).

Therefore you should expect a v0.3.16 release with the support you need.

David

— Reply to this email directly, view it on GitHub https://github.com/david-cattermole/mayaMatchMoveSolver/issues/217#issuecomment-1051982322, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPOE3TWC2NEOSHMRF4XFXDU5CUSLANCNFSM42WPS73Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

david-cattermole commented 2 years ago

Hi @maxnbk,

I have released v0.3.16 with support for Maya 2022 and Python 3 (and Python 2). This support has been backported from the "develop" branch and represents mostly what v0.4.0 will contain, when it is released.

https://github.com/david-cattermole/mayaMatchMoveSolver/releases/tag/v0.3.16

Your CMake build system recommendations are still planned, but I'll track that in a different ticket, and I will implement those features in the latest v0.4.0 alpha versions.

Thanks to everyone for the input, I'll close this ticket for now - if there are issues, please feel free to comment.

david-cattermole commented 1 year ago

@maxnbk The new v0.4.0.beta1 release has an auto-download and compile setup. It also avoids any "in-source building" (although documentation generation still does that a bit).

Could you try it out and let me know if it's closer to what you need? https://github.com/david-cattermole/mayaMatchMoveSolver/releases/tag/v0.4.0.beta1

Please comment on #243 with any feedback.