-
Notifications
You must be signed in to change notification settings - Fork 463
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
Fix GLSL 1.50 for Mesa #1725
base: noetic-devel
Are you sure you want to change the base?
Fix GLSL 1.50 for Mesa #1725
Conversation
I have a vague memory that there was something about being able to explicitly request a |
that will be |
Thanks a lot for this hint, @paroj! This allowed me to drop the
Probably, we shouldn't mix GLSL150 and GLSL120 shaders as we do right now: rviz/ogre_media/materials/scripts150/point_cloud_box.material Lines 23 to 25 in 697ed3f
|
One more question, @paroj: |
no, those are unpacked by the GPU in hardware |
Crashes here as well it seems |
this should typically work. Maybe ancient Ogre 1.9.0 is doing something strange there. |
Thanks for your very valuable input already @paroj Is there some way to |
@simonschmeisser, do you have a more recent Ogre installation laying around to test this PR against? |
It's more a matter of compilation time ... will test in the evening/night |
recent ogre does that by default. |
1.12.13 is not that helpful, propably not recent enough by Pavels standards, gives the following for "flat square":
"points" works but still gives the error about
"box", the one updated here gives:
|
no, 1.12.13 is fine. That errors are generated by your own logging. They are follow-ups by Ogre marking the material as unavailable by error. The Ogre logging for this looks like: https://github.com/OGRECave/ogre/runs/5611960759?check_suite_focus=true#step:5:948 |
It's surprising how long it took me to find out how to enable debug output ... Anyway, with 1.12.13 it claims that my hardware does not support geometry shaders:
full startup log: https://gist.github.com/simonschmeisser/aad15e2e91e05260d4e49cccacf4219e Upon receiving a point cloud which is supposed to be rendered using the box geometry shader I get this:
|
Indeed it is strange that max. glsl120 is listed:
However, for me (with an Intel UHD Graphics, CML GT2) only |
The legacy GL RenderSystem checks for EXT_geometry_shader4 to enable geometry shaders unfortunately that variant is quite different than what got into GL3.2 and your GPU driver probably did not bother implementing the EXT variant. The legacy GL RenderSystem main goal is to provide fixed-function support. As such it merely targets max GL2. It does not advertise glsl > 120, as it is often used to filter out shaders targeting GL3+, like: If the implemented GL compatibility profile is GL3.2+, you can use the standard geometry shaders as you currently do.
this is to be expected then |
Enabling the GL3+ backend is one of my long term goals, but is there a short term solution? I'm unfortunately not able to come up with the next step from your detailed answer (thanks for that!) |
add |
We (as in the ROS community, not as in my day job) are unfortunately stuck with a released ancient version of 1.9, so we would basically need to find a way to hack this into the Sounds like enabling GL3+ would be the easier way after all. I think I got stuck on the shader changes last time I tried but we should have those available with this PR now |
you should be able to do this using
I am afraid GL3+ is not in a good shape in 1.9 either.. |
That sounds very doable, I'll try to implement that. Thanks! Meanwhile one the patched 1.12.13 I get the following further parsing errors which suggest that we need to fix some more shaders:
I opened rhaschke#3 for that but now it still crashes once it tries to use the shader |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
sorry, new try:
|
it says it crashes here: https://github.com/OGRECave/ogre/blob/v1.12.13/RenderSystems/GL/src/GLSL/src/OgreGLSLLinkProgram.cpp#L43 my best guess would be, that this method got inlined and the actual problem is that glProgramParameteriEXT is NULL, as it is part of EXT_geometry_shader4, which your driver does not support. Try commenting out the glProgramParameteriEXT calls - they are not needed with GL3.2 style shaders. |
Disabling all calls to @simonschmeisser, I would expect that this works on AMD Renoir as well. Essentially, they use the same Mesa driver, don't they? Regarding Ogre, I cannot see a difference: For my Intel GPU, Ogre reports that the Is there any chance to get this running with existing binary releases of OGRE 1.9.0, e.g. by setting some RenderSystem flags? |
Open TODOs:
|
- Drop compatibility mode, which is not supported by Mesa - Don't use gl_ prefixes for user-declared variables - Don't redeclare gl_PerVertex
note, that with GL3+, you can use RenderDoc to debug stuff
NVIDIA implements EXT_geometry_shader4, but Mesa does not.
no applied the changes from this discussion to Ogre 13.3 OGRECave/ogre#2417 |
Thanks for this fix, @paroj! |
yes
the respective check was only added in Ogre 1.10 |
"In charge" is too strong, I don't have any Debian or Ubuntu privileges, I just updated the packages and try to get them uploaded into Debian from where they are snapshoted for Ubuntu. Getting a patch into 20.04 for 1.12 might work or it might fail (fail means no reaction from anybody ever). But would it help? ROS Noetic sticks to 1.9 so we would need to patch that right? For 1.12 and newer we could also switch to GL3+ rendering system to circumvent the issue. |
That's true. Do you see any chance to do so?
Where do you see the benefit of switching to GL 3+? This will exclude older hardware, right? |
Well it detects and uses the newer GS correctly so it doesn't need any patching.
GL rendering system without GS would still be available as a fallback but I also don't think much of that old hardware is still alive. It's more a question of drivers (mobile/embedded) but I think even the "weaker" ones should be sufficient. |
We would need to slightly adapt the patch by @paroj because GLEW was used in 1.9 to check for extensions instead of GLAD. Then we should have no problems getting this into Debian unstable. Maybe we can even get Ubuntu to pull the patched version into 22.04 (freeze exception etc). Then we can open a bug with a patch/debdiff for stable releases like 20.04. Maybe we can also do those things in parallel. Maybe we should also open a debian bug. @jspricke does that sound reasonable? |
Thanks everybody for working on a fix for this! If you need me to test something on AMD Renoir, I can be of help (both Noetic and Melodic). |
One of my colleagues has the amdgpu driver running on 18.04 with mesa 20.3 and encounters the same crashes on Ogre 1.9 due to EXT_geometry_shader4 not being available but also not being checked. (It's a RENOIR just like mine). I'll try backporting OGRECave/ogre#2417 to 1.9 tomorrow |
I uploaded a fixed version of ogre-1.9 for focal to my ubuntu PPA: https://launchpad.net/~s-schmeisser/+archive/ubuntu/ogre-1.9-focal seems to work well with this branch on AMD Renoir but please confirm. I can then try filing bug reports and patches |
@simonschmeisser I've tried your OGRE 1.9 build on 18.04 (from the bionic PPA) and it did not help. |
Ah, my bad, I didn't have this PR in the tested Melodic rviz. I cherry-picked all the commits and it works now! |
Did it still crash or did you still see white points only? There seem to be multiple levels of problems around ... I tested the modified ogre-1.9 on nvidia yesterday and will now start trying to get this fix into Debian/Ubuntu |
MR for Debian: https://salsa.debian.org/games-team/ogre/-/merge_requests/8 Bug (and debdiff) for Ubuntu: https://bugs.launchpad.net/ubuntu/+source/ogre-1.9/+bug/1968010 |
The process for Debian is detailed here: I think this needs analysis: what will change for existing users?, could it break current setups/workarounds?.. |
First, I had updated OGRE and not pulled the changes from this PR. The result was that rviz ran but had the same bug with white/gray dots. Then I reverted OGRE to the stock version and installed the rviz update from this PR. That resulted in a hang of rviz on startup (I was directly launching a config for the pointcloud test, so not sure if that would happen with empty rviz). And it was a hang and not a crash. Finally, with both OGRE and rviz patched, it works as a charm. So it seems the rviz fix release definitely needs to wait for the OGRE release. Or it needs to be somehow better protected for the case OGRE is not patched. |
I tested this again today on a patched 1.12.13 (will test with 13.6.4 later):
The log of https://gist.github.com/simonschmeisser/524366304851f6fb70d7102b1e153b57 |
I tested with ogre 13.6.4 which unfortunately shows grey blocks as well. It's still WIP but I'll open a PR for compatibility of course |
As pointed out in #1721, Mesa doesn't correctly render PointCloud geoms other than points with GLSL 150, while NVIDIA renders them fine.
The problem is that Mesa uses are more strict GLSL compiler than NVIDIA, thus rejecting the existing scripts. Essentially,
Mesa
doesn't support thecompatibility
profile. Further, it's not allowed to redefinegl_PerVertex
or use thegl_
prefix for user variables (#1721 (comment)).I was able to fix most syntax issues, such that
glslangValidator
doesn't complain anymore.However, the
compatibility
issue remains. Using thecompatibility
profile, color values can be passed from CPU to GPU as packed values (uint
):rviz/src/rviz/ogre_helpers/point_cloud.cpp
Line 532 in ce068cb
and be read in GLSL via
gl_Color
- as avec4
:rviz/ogre_media/materials/glsl150/pass_pos_color.vert
Line 13 in 697ed3f
However, when dropping
compatibility
profile, we need to unpack the color ourselves:But, I have no idea, how to accomplish that... I'm stuck here.