TheFoundryVisionmongers / nuke-ML-server

A Nuke client plug-in which connects to a Python server to allow Machine Learning inference in Nuke.
Apache License 2.0
134 stars 36 forks source link

Add an op for creating geometry #14

Open mishurov opened 5 years ago

mishurov commented 5 years ago

Hi!

I've created an additional op based on SourceGeo for models that create geometry from images. I moved the code related to the client-server interactions into a mixin. Unfortunately I hadn't found an elegant way avoiding using templates yet I'd separated template implementation into another file so it doesn't increase compilation time.

As an example of usage I've added a TensorFlow Lite model. The file sizes of the models I use are small, so they can easily fit into the repository without needing to download from external resources. The only caveat is that the model was extracted from Google's ARCore .apk file, so I don't know if it's permissible to use it in open source projects.

Here https://ai.googleblog.com/2019/03/real-time-ar-self-expression-with.html

In the article they also mention some "windowed smoothing", the term is quite broad. I didn't implement anything thus the animation can be noisy. A Kalman filter or so probably may be implemented in the server side code.

I also a bit confused about "get_geometry_hash()" it's quite possible that I'm using it wrongly in the code.

There're were also issues with using protobuf simultaneously in two shared libraries.

Some pictures to make the pull request more fancy.

Screenshot_2019-06-19_22-48-38

Screenshot_2019-06-19_22-41-12

ringdk commented 5 years ago

Hi @mishurov!

First off, this is absolutely amazing. Thank you for taking the time to put this together. We're going to review it over the week and pull it in.

Secondly, I think you're right about using data extracted from the .apk. We're aiming to be fully license compliant, for both ourselves and especially anyone who wants to use this project and so we won't include any extracted data where the usage license doesn't permit it or is a bit grey. However there are usually solutions to this and we can start investigating those.

Thanks again, I'm looking forward to playing with this!

mishurov commented 5 years ago

@ringdk thank you, that's inspiring. The code is within limits of my understanding of the API and so forth, so I wouldn't even pretend that it is an ideal solution, more like a draft. The neural network I've included is tremendously convenient for debugging due to its modest requirements yet it doesn't actually matter if there would be any other network which takes an image and returns geometry.

mateuszwojt commented 5 years ago

Hi @mishurov I have couple of issues related to your code.

MLClient/MLClientGeo.cpp:89:19: error: cannot convert ‘DD::Image::GeoOp*’ to ‘DD::Image::Iop*’ in return
     return input1()->iop();

I believe you should use dynamic_cast, since GeoOp doesn't have a iop() method:

Similar issue one line below that:

MLClient/MLClientGeo.cpp:90:51: warning: invalid conversion from ‘DD::Image::Op*’ to ‘DD::Image::Iop*’ [-fpermissive]
   return MLClientMixin<SourceGeo>::default_input(0);

Apart from that, I'm having an error while accessing elements from protobuf::RepeatedField

MLClient/MLClientGeo.cpp:228:33: error: no match for ‘operator[]’ (operand types are ‘const google::protobuf::RepeatedField<float>’ and ‘int’)
           item[j] = (*attr.data)[i * dim + j];

Can you tell me which version of protobuf lib are you using to compile the plugin?

mishurov commented 5 years ago

@GreenEminence the parent class Op has the method

virtual Iop * | iop () Cast to an Iop. This is much cheaper and safer than using dynamic_cast.

https://learn.foundry.com/nuke/developers/113/ndkreference/Plugins/classDD_1_1Image_1_1Op.html#a24e0d3db1f380929ed86e4a0a5913c1d

I was compiling the code against Nuke 11.3v1, may be something had changed in the API.

I was building Protobuf from source as the README recommends.

Protobuf (tested with 2.5.0 and 3.5.1)

Unfortunately I can't tell now which version of those two I was using because I've moved to a new Debian since then and I lost my intermediate files. Protobuf has another issue as well, since the code generates two different shared libraries which use the same generated C++ file, there were problems with loading the binaries simultaneously, I had to add option optimize_for = LITE_RUNTIME; into message.proto, it is mentioned in the comments in the file, it was the only workaround I'd found.

P.S. There was also sort of annoying thing with gcc which was hard to figure out. If you're compiling on Linux, you may try to set the -D_GLIBCXX_USE_CXX11_ABI flag to 0 or 1 in CMake so as to match the ABI of Nuke's binaries. E.g.

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -D_GLIBCXX_USE_CXX11_ABI=0")
mateuszwojt commented 5 years ago

Hi @mishurov

Thanks for replying! I never managed to make it work using iop() call, so I decided to update the code with dynamic_cast for now. Thanks for the hint about the Nuke version, we're running an automated build process which compiles plugins for all major Nuke releases and I didn't notice any difference between Nuke 11.2 and 11.3.

Protobuf is another story - I'm not convinced it actually works with 2.5.0, I tried with 2.6.1 and couldn't make it work. I'm currently building 3.6.1 to see if it's going to help (matching the version that Houdini's currently using). I'll get back to you with the results.

Cheers!

mishurov commented 5 years ago

Cheers @GreenEminence!

The initial motivation for the pull request was to point out the issue of returning geometry data from the server and to propose a draft solution. I was developing it on a single Linux machine and I frankly don't know how it would scale to industrial levels on different platforms and versions due to my limited personal resources. I hope Foundry will come up with some elegant solution after all.