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

Drop Mesa workaround #1714

Conversation

rhaschke
Copy link
Contributor

Addressing #1701. Needs to be tested on various systems!


std::string gl_version_string = (const char*)glGetString(GL_VERSION);
// The "Mesa 2" string is intended to match "Mesa 20.", "Mesa 21." and so on
mesa_workaround = gl_version_string.find("Mesa 2") != std::string::npos && gl_version_ >= 320;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion would have been to just change this to "Mesa 20"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the Mesa 20 part of some HWE or part of the stock 18.04, ie are there likely going to be people with non HWE systems running into this again?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ LANG=C apt-cache policy mesa-va-drivers 
mesa-va-drivers:
  Installed: 21.3.6~kisak1~b
  Candidate: 21.3.6~kisak1~b
  Version table:
 *** 21.3.6~kisak1~b 500
        500 http://ppa.launchpad.net/kisak/kisak-mesa/ubuntu bionic/main amd64 Packages
        100 /var/lib/dpkg/status
     20.0.8-0ubuntu1~18.04.1 500
        500 http://cz.archive.ubuntu.com/ubuntu bionic-updates/universe amd64 Packages
     19.2.8-0ubuntu0~18.04.2 500
        500 http://security.ubuntu.com/ubuntu bionic-security/universe amd64 Packages
     18.0.0~rc5-1ubuntu1 500
        500 http://cz.archive.ubuntu.com/ubuntu bionic/universe amd64 Packages

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's on Bionic HWE with kisak-mesa PPA. So it seems 20.x is available via bionic-updates for everyone.

@simonschmeisser
Copy link
Contributor

I was just testing around with the PC2 display on AMD Renoir and noticed that most of the more fancy Styles seem to be unsupported with GLSL 1.5. Changing the Size has no effect for anything besides Points and it's always displayed as super tiny dots. I was not able to trigger any unusual behavior with the Points style

We have serious performance regressions with high resolution octomaps when limiting to the older shader versions though, that's why I opened the discussion.

[ INFO] [1644874202.850198482]: rviz version 1.14.14
[ INFO] [1644874202.850307278]: compiled against Qt version 5.12.8
[ INFO] [1644874202.850323799]: compiled against OGRE version 1.9.0 (Ghadamon)
[ INFO] [1644874202.865892892]: Forcing OpenGl version 0.
[ INFO] [1644874203.187769729]: Stereo is NOT SUPPORTED
[ INFO] [1644874203.187885107]: OpenGL device: AMD RENOIR (DRM 3.41.0, 5.13.0-28-generic, LLVM 12.0.0)
[ INFO] [1644874203.187928259]: OpenGl version: 4,6 (GLSL 4,6).

@peci1
Copy link
Contributor

peci1 commented Feb 15, 2022

Interesting, I also tested that on Renoir and it did not work for me with Mesa 21, that's why I requested the workaround to be extended to all 2x versions.

#1598

@simonschmeisser
Copy link
Contributor

Your test was with Mesa 21.0.0 while the fix appeared in 21.0.3 (I'm not sure it has been pinpointed exactly but maybe I misunderstood something ...)

@peci1
Copy link
Contributor

peci1 commented Feb 15, 2022

@simonschmeisser Good point, haven't noticed that... Could you put together a minimum working example? It's been some time since I was solving this, so I can't remember what were the steps to receive a failure...

@rhaschke
Copy link
Contributor Author

rhaschke commented Mar 7, 2022

I'm not sure I fully understood your comments. Thus, let me try to summarize. @peci1 and @simonschmeisser, please correct!

@peci1
Copy link
Contributor

peci1 commented Mar 7, 2022

  • Mesa 21.0.3 was fixing this and it is released, both for Bionic (via bionic-updates)

Bionic has only Mesa 20. So this fix hasn't been released there. I (and probably others) are using a customized Mesa PPA to get a newer Mesa on Bionic, as that's the only way to get the new Ryzen GPUs working (together with mainline kernels).

@peci1
Copy link
Contributor

peci1 commented Mar 7, 2022

I tested on Bionic+Ryzen GPU+Mesa 21 with and without the Mesa workaround:

With workaround: everything works (I haven't tested performance, though).

Without workaround: Points are displayed with correct size, but there is this (new?) issue with everything except points displaying as a single white dot:
rviz_screenshot_2022_03_07-12_35_45

I've opened an issue for the "newly discovered" bug (#1721), but I think this PR needs to be hold off until that issue is solved.

@rhaschke
Copy link
Contributor Author

rhaschke commented Mar 7, 2022

  • Mesa 21.0.3 was fixing this and it is released, both for Bionic (via bionic-updates)

Bionic has only Mesa 20. So this fix hasn't been released there.

You are right. I was writing my comment in a rush obviously...

@rhaschke
Copy link
Contributor Author

rhaschke commented Mar 8, 2022

@peci1, note that you can manually enforce an older OpenGL standard: rviz --opengl 310.
That's what the workaround automatized.

@peci1
Copy link
Contributor

peci1 commented Mar 8, 2022

note that you can manually enforce an older OpenGL standard: rviz --opengl 310.

Yes, but this combines very badly with already existing launch files starting rviz. Maybe if it were an environment variable, it would be easier to set globally.

@rhaschke
Copy link
Contributor Author

rhaschke commented Mar 8, 2022

What about alias rviz='rviz --opengl 310' in your .bashrc?

@peci1
Copy link
Contributor

peci1 commented Mar 8, 2022

<node name="rviz" pkg="rviz" type="rviz" args="-d path/to/config.rviz /> inside an (externally provided) launch file is the hard thing... the alias won't help there

@rhaschke rhaschke marked this pull request as draft March 11, 2022 17:27
@rhaschke
Copy link
Contributor Author

Closing for now, as our GLSL 1.50 scripts are not Mesa-compatible yet.

@rhaschke rhaschke closed this Mar 17, 2022
@rhaschke rhaschke deleted the drop-mesa-workaround branch March 17, 2022 20:11
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.

3 participants