Use EPEL-provided assimp now that it's available

[cottsay]

As of last week, we no longer need to force vendoring in rviz_assimp_vendor: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-d3f80c2d82

[cottsay]

Looks like there are some missing dependencies. I'm guessing that either the rosdep rule needs to be updated to include some additional dependencies or the assimp-devel RPM subpackage needs to declare them if the aren't optional.

Converting to draft until I get a green build.

[cottsay]

Upstream fix for EPEL 9 assimp: https://src.fedoraproject.org/rpms/assimp/pull-request/13

[cottsay]

The fix just landed in EPEL 9 stable. Here's a rebuild:

EDIT: Rebuild:

[clalancette]

The fix just landed in EPEL 9 stable. Here's a rebuild:

Hm, the failure in the mesh_loader test is highly suggestive that something is different with this version of assimp. But I'm not sure what.

[cottsay]

Hm, the failure in the mesh_loader test is highly suggestive that something is different with this version of assimp.

Indeed, this looks like a real issue to me. In order to get assimp into EPEL 9, I needed to update it in Fedora to a pretty recent release so there's a strong chance that this is a "canary" issue that we could see down the line on other platforms. I'll dig a little deeper and report back.

[cottsay]

Fun stuff. This same test is failing on Noble, so it's almost certainly rooted in the newer version of assimp that RHEL 9 and and Noble have that Jammy doesn't.

[cottsay]

I'm tempted to say that we should take this change and regress the test for now, since it's going to be a priority for Rolling soon and we can backport the fix to Iron later.

[clalancette]

Fun stuff. This same test is failing on Noble, so it's almost certainly rooted in the newer version of assimp that RHEL 9 and and Noble have that Jammy doesn't.

Hm, it seems to succeed for me locally on Noble. I currently have libassimp-dev 5.3.1 on Noble, and that seems to be the latest that is packaged in Debian as well: https://packages.debian.org/sid/libassimp-dev . What version are you working with?

[cottsay]

$ dpkg-query --show libassimp-dev
libassimp-dev:amd64 5.3.1+ds-1

You need a display to run those tests - they're automatically skipped if you don't have DISPLAY set during the build. We're using a virtual framebuffer on ci.ros2.org, I think. I made it work by forwarding the appropriate devices and variables through to podman.

$ colcon test --packages-select rviz_rendering_tests --event-handler console_direct+ --ctest-args -R mesh

...

1: [ RUN      ] MeshLoaderTestFixture.loading_ascii_stl_files_fail
1: [rviz_rendering:debug] Available Renderers(1): OpenGL Rendering Subsystem, at /ws/src/ros2/rviz/rviz_rendering/src/rviz_rendering/render_system.cpp:289
1: [rviz_rendering:info] Stereo is NOT SUPPORTED, at /ws/src/ros2/rviz/rviz_rendering/src/rviz_rendering/render_system.cpp:531
1: [rviz_rendering:info] OpenGl version: 4.5 (GLSL 4.5), at /ws/src/ros2/rviz/rviz_rendering/src/rviz_rendering/render_system.cpp:272
1: /ws/src/ros2/rviz/rviz_rendering_tests/test/mesh_loader_test.cpp:111: Failure
1: Value of: rviz_rendering::loadMeshFromResource(mesh_path)
1:   Actual: true
1: Expected: false
1: [  FAILED  ] MeshLoaderTestFixture.loading_ascii_stl_files_fail (79 ms)

...

[clalancette]

You need a display to run those tests - they're automatically skipped if you don't have DISPLAY set during the build

Ah yeah, you are totally right. Now I see it failing, thanks. I'm going to spend a few minutes and look deeper into the test.

[clalancette]

Ah yeah, you are totally right. Now I see it failing, thanks. I'm going to spend a few minutes and look deeper into the test.

I spent a bit of time trying to trace down when and how it started working, without success.

But stepping back, I think we can safely remove the test. It is clear that previously ASCII STL files failed, and now they succeed. So the behavior can (and does) change, and thus I don't think the test serves it's original purpose. Maybe we can eventually add the test back in a strictly positive way (that is, we expect it to always succeed), but for now I'm OK with getting rid of it. I'll open a PR to do that and we can discuss more over there.

[cottsay]

I spent a bit of time trying to trace down when and how it started working, without success.

I traced it a little bit yesterday and it appears that at the time the test was added to rviz there were functions for loading STL files that aren't there anymore. It doesn't explain why the newer assimp changed that behavior, but I agree that dropping the test is probably the right path forward.

[cottsay]

Another rebuild now that the rviz change is merged: