MADEAPPS / newton-dynamics

Newton Dynamics is an integrated solution for real time simulation of physics environments.
http://www.newtondynamics.com
Other
928 stars 183 forks source link

python swig wrapper bitrotted #283

Closed perkinslr closed 2 years ago

perkinslr commented 2 years ago

It looks like some of the files and variable names have changed since the last time the swig python bindings were built.

For starters newton.i starting at line 63 needs this change

-%include "../../../sdk/dCore/dVectorSimd.h"
-%include "../../../sdk/dCore/dMatrix.h"
-%include "../../../sdk/dCore/dQuaternion.h"
+%include "../../../sdk/dCore/ndVectorSimd.h"
+%include "../../../sdk/dCore/ndMatrix.h"
+%include "../../../sdk/dCore/ndQuaternion.h"

And a number of types have changed from (for example, dMin to ndMin. Additionally, the CMakeLists.txt files in newtonPy generates a makefile (on linux) or msvc project file (on windows) which fails to build with /tmp/newton-dynamics/newton-4.00/applications/toolsAndWrapers/newtonPy/../../../sdk/dNewton/ndWorld.h:29:10: fatal error: ndBodyParticleSetList.h: No such file or directory

(Or similar errors depending on the compiler used).

I did successfully generate the wrapper on linux, by manually invoking the compiler and linker, but it fails on windows even if manually called complaining about missing ??_GndNodeBase@ndShapeCompound@@QEAAPEAXI@Z (among other symbols).

Some of the missing symbols could be found in build/sdk/ndNewton.dir/Release/ndVector.obj, but not all of them, and I wouldn't expect to need to link in an obj file directly.

Anyway, I have a bit of time to keep poking at this, but would like some advice on where to proceed next. Unfortunately, I need it working on windows for the current project.

JulioJerez commented 2 years ago

oh yes somethings had changed, it is up to date now. please try again.

when building with visual studio, the cmake script copy a plugin to the blender folder, is you do has blender installed in the default folder you have to launch visual studio as administrator.

I have not added a way to disable that part, since I never though anyone would use it for anything other than blender.

perkinslr commented 2 years ago

Thanks for the warning, on linux it tries to put the output in /scripts/addons/newtonPy, which would have confused me without it. It also didn't automatically figure out it needed to include or link the python stuff, but telling CMake to add those is trivial.

I'll report how it goes on windows presently.

JulioJerez commented 2 years ago

Ah yes in Linux it will not know how to find the Python dependency. In window, I just read it fron the env variable.

If you get it to work in Linux, maybe you can submit a pull request. Or just tell me how to do it and I add it.

On Fri, Jul 8, 2022, 11:03 AM Logan Perkins @.***> wrote:

Thanks for the warning, on linux it tries to put the output in /scripts/addons/newtonPy, which would have confused me without it. It also didn't automatically figure out it needed to include or link the python stuff, but telling CMake to add those is trivial.

I'll report how it goes on windows presently.

— Reply to this email directly, view it on GitHub https://github.com/MADEAPPS/newton-dynamics/issues/283#issuecomment-1179241446, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6EPJAYQM4CJTDE3U4ZYQTVTBUQHANCNFSM527YUAIQ . You are receiving this because you commented.Message ID: @.***>

perkinslr commented 2 years ago

Easy enough: #284 You can probably just have cmake find python on windows too, but that assumes it's installed in a way cmake can find it.

Also, it looks like the interface may have bitrotted a bit more here, as the newton_wrap.cxx included in the repo compiles fine, but if I have swig regenerate it, it can't find the declaration for void dGetWorkingFileName (const char* const name, char* const outPathName); Adding that to one of the headers in newtonPy lets it compile, and it does appear to work.

It may be worth setting up some github actions to build with all the options on to catch these while the changes are super fresh. I have some experience with GH actions if you want me to do that. It would also make cutting release builds easier.

JulioJerez commented 2 years ago

I merged the pull request. thanks on funtion dGetWorkingFileName it is defined in file "newtonConfig.h", it just needs to be added to the wrapper section swig script. It did it if you sync, it should build and compile fine now.

%{

include

#include <newtonWorld.h>
#include "newtonConfig.h"

%}

perkinslr commented 2 years ago

I can confirm cmake can find python on windows.
Unfortunately we aren't quite there yet.
First, on Linux, the libnewtonPy.so file gets renamed to _newton.pyd, which should be _newton.so. I can open a PR to clean up the cmake a bit more with regard to that. Second, on both Linux and Windows, when I import newton; w = newton.NewtonWorld(); w.Update(0.1), I get

TypeError: in method 'NewtonWorld_Update', argument 2 of type 'ndFloat32'

This appears to be because swig doesn't see where ndFloat32 is typedeffed or defined to float, I assume there's a header with it, but for immediate testing I just added -DndFloat32=float to the swig line. With that, the most basic tests work, both on Linux and Windows.

perkinslr commented 2 years ago

With the latest changes, the python wrapper appears to be working nicely. I opened #286 to finish resolving some linux vs windows issues with it, but I think this issue is otherwise ready to close.