Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python swig wrapper bitrotted #283

Closed
perkinslr opened this issue Jul 8, 2022 · 7 comments
Closed

python swig wrapper bitrotted #283

perkinslr opened this issue Jul 8, 2022 · 7 comments

Comments

@perkinslr
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

JulioJerez commented Jul 8, 2022 via email

@perkinslr
Copy link
Contributor Author

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
Copy link
Contributor

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 <ndNewton.h>
#include <newtonWorld.h>
#include "newtonConfig.h"
%}

@perkinslr
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants