mjcarroll / pixi_gazebodistro

0 stars 1 forks source link

Enable use in Windows #1

Closed traversaro closed 4 months ago

traversaro commented 5 months ago

To complete the migration to gz-sim in my lab I need to eventually attack https://github.com/gazebosim/gz-sim/issues/2089 . So I needed some way to quickly compile all gazebo distro easily.

Modifications:

Actually requires https://github.com/gazebosim/gz-cmake/issues/417 or https://github.com/dartsim/dart/issues/1795 to be used out of the box, but it is still useful in this form, at least to me.

mjcarroll commented 4 months ago

Perfect, I had a few of these in a scratch workspace that I had forgotten to push, this is very helpful for completeness.

mjcarroll commented 4 months ago

I think that we need to bump the pixi-action version here as well to get CI to work?

traversaro commented 4 months ago

I think that we need to bump the pixi-action version here as well to get CI to work?

Done: https://github.com/mjcarroll/pixi_gazebodistro/pull/1/commits/336447e7d1c7b38298ac3eda8297550600308981 .

traversaro commented 4 months ago

@mjcarroll no hurry or anything, but I guess the CI is awaiting for an approval on your side.

mjcarroll commented 4 months ago

@mjcarroll no hurry or anything, but I guess the CI is awaiting for an approval on your side.

I figured once I approved it once, it was good to go. Doesn't seem to be the case here.

traversaro commented 4 months ago

I compiled locally and found a few more fixes. The Linux build now fails on gz-rendering due to the issue mentioned in https://github.com/conda-forge/staged-recipes/pull/13677#issuecomment-759986297, that is actually a workaround as the glext.h shipped by conda-forge is actually quite old and contains a bug, as it comes from CentOS7 (due to opengl being a CDT package, see https://conda-forge.org/docs/maintainer/knowledge_base/#core-dependency-tree-packages-cdts). In conda-forge recipes we always handled this by setting GLX_GLXEXT_LEGACY, see https://github.com/conda-forge/gz-rendering-feedstock/blob/fcd1312dddbc52ec39772372e034144587a246e2/recipe/build_cxx.sh#L17, but in VTK they had some regression by doing that (see https://gitlab.kitware.com/vtk/vtk/-/blob/v9.3.0/Rendering/OpenGL2/vtkXOpenGLRenderWindow.cxx?ref_type=tags#L18), so I am not sure how to proceed here.

traversaro commented 4 months ago

I compiled locally and found a few more fixes. The Linux build now fails on gz-rendering due to the issue mentioned in conda-forge/staged-recipes#13677 (comment), that is actually a workaround as the glext.h shipped by conda-forge is actually quite old and contains a bug, as it comes from CentOS7 (due to opengl being a CDT package, see https://conda-forge.org/docs/maintainer/knowledge_base/#core-dependency-tree-packages-cdts). In conda-forge recipes we always handled this by setting GLX_GLXEXT_LEGACY, see https://github.com/conda-forge/gz-rendering-feedstock/blob/fcd1312dddbc52ec39772372e034144587a246e2/recipe/build_cxx.sh#L17, but in VTK they had some regression by doing that (see https://gitlab.kitware.com/vtk/vtk/-/blob/v9.3.0/Rendering/OpenGL2/vtkXOpenGLRenderWindow.cxx?ref_type=tags#L18), so I am not sure how to proceed here.

To give you an idea, the least intrusive fix I was able to find was:

diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc
index 7352e747..5c07bef2 100644
--- a/ogre/src/OgreRenderEngine.cc
+++ b/ogre/src/OgreRenderEngine.cc
@@ -19,6 +19,9 @@
 #if !defined(__APPLE__) && !defined(_WIN32)
 # include <X11/Xlib.h>
 # include <X11/Xutil.h>
+# include <KHR/khrplatform.h>
+typedef khronos_ssize_t GLsizeiptr;
+typedef khronos_intptr_t GLintptr;
 # include <GL/glx.h>
 #endif

on distros with modern OpenGL headers the typedef will just be redefined in GL/glx.h to exactly the same values, so that should be ok.

mjcarroll commented 4 months ago

To give you an idea, the least intrusive fix I was able to find was:

I'm fine with this. I also don't think it blocks this PR. I know CI is broken, but would prefer we can keep iterating, since this is a low-stakes repo.

traversaro commented 4 months ago

By the way, eventually conda-forge may be some modern opengl headers (see https://github.com/conda-forge/staged-recipes/pull/25919), so perhaps we do not even need to workaround that anymore.