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

Enable use in Windows #1

Merged
merged 4 commits into from
Apr 2, 2024
Merged

Enable use in Windows #1

merged 4 commits into from
Apr 2, 2024

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented Mar 23, 2024

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

Modifications:

  • Add python to helper.py invocations as in Windows there is no shebang
  • Add missing dependencies
  • Do not hardcode specific version for dependencies in pixi.toml unless necessary

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

@mjcarroll
Copy link
Owner

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

@mjcarroll
Copy link
Owner

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

@traversaro
Copy link
Contributor Author

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

Done: 336447e .

@traversaro
Copy link
Contributor Author

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

@mjcarroll
Copy link
Owner

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

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.

@traversaro
Copy link
Contributor Author

traversaro commented Mar 27, 2024

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

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.

@mjcarroll mjcarroll merged commit 687baa5 into mjcarroll:main Apr 2, 2024
1 check failed
@traversaro
Copy link
Contributor Author

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

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

Successfully merging this pull request may close these issues.

2 participants