mosra / magnum-bindings

Bindings of the Magnum C++11 graphics engine into other languages
https://magnum.graphics
Other
19 stars 13 forks source link

Changes to enable Windows build #2

Closed cegbertOculus closed 5 years ago

cegbertOculus commented 5 years ago

changes to Python.h fixes: magnum-bindings\src\Magnum/SceneGraph/Python.h(67): error C2614: 'Magnum::SceneGraph::PyObject': illegal member initialization: 'Object' is not a base or member

change to magnum.cpp fixes: magnum-bindings\src\python\magnum\magnum.cpp(220): error C2375: 'PyInit__magnum': redefinition; different linkage

change to glfw.cpp fixes: magnum-bindings\src\python\magnum\platform\glfw.cpp(44): error C3640: 'magnum::platform::glfw::PublicizedApplication::[thunk]: thiscall `void cdecl magnum::platform::glfw(pybind11::module &)'::2'::PublicizedApplication::vcall'{8,{flat}}' }'': a referenced or virtual member function of a local class must be defined

mosra commented 5 years ago

Hi, thank you for the continued push on Windows support!

This is something I didn't even try for the bindings repo yet. I need to set up a Windows CI job to ensure it builds and continues to build there.

For the first thing, is it possible that Object is some silly macro defined in windows.h? :)

cegbertOculus commented 5 years ago

Yes, I think that's the case, though I couldn't find the source of the problem. The errors I was getting referencing 'Object' weren't really making sense, so I just changed the name to Obj.

mosra commented 5 years ago

Finally had the time to set up a Windows CI. Merged the second and third fix as e8198cb97eca492f9a2fa3c7c799c1f0bc4c59a3 and 14fa2247e997c1678a2d7d84aa4ca63d90417794. The Object rename was not needed, but also Magnum is not including windows.h that much anymore, so maybe that fixed it.

Can you confirm current master works for you? Thanks a lot!

mosra commented 5 years ago

Based on private discussion, the C2614-related change was unfortunately really needed as well, comitted as d6f1c50c34292d4db2dfec8ae291aa05415789ef.