Port ign-rendering to Ogre 2.3 #553

darksylinc · 2022-02-01T04:22:33Z

🦟 Bug fix

There's no ticket associated

Summary

This PR updates ign-rendering to Ogre 2.3

Main changes:

Updated shaders to work on both GL and Vulkan
Ogre2GpuRays uses compositor scripts
- This results in less code and is more readable
- It was necessary to support Vulkan, as the Compositor needs to know certain texture dependencies to insert proper texture layout transitions/barriers
Ogre2Material needs to request glslvk shader profile instead of glsl
Minor fixes to code to accommodate for Ogre 2.3 and/or Vulkan (e.g. setupRootLayouts and other virtual overloads that slightly changed signature, a function call that had to be removed)
The patch looks big due to Hlms folder being updated with upstream's contents
- Hlms/Ignition folder is ours though!
- Re-applied Terra changes for Ignition to Terra's shaders
- Re-applied Terra changes for Ignition to Terra's C++ code
Depth clamp override is now API-agnostic since Ogre 2.3 is aware of it (previously Ignition was performing GL calls directly as a hack)
- Some Metal tests that were previously failing due to this reason should now pass
All tests are passing under OpenGL
Under Vulkan, INTEGRATION_depth_camera is randomly failing. Could be either Ignition's or Ogre's bug.

It's easier to check the commits than the changes. Actual diff from Ignition's code is not that big.

Reviewers could try checking out the new branch, and diff against main. That will be far easier to review since you can ignore/skip upstream files.

Checklist

Signed all commits for DCO
Added tests
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

ahcorde

Linters: https://github.com/ignitionrobotics/ign-rendering/runs/5016366064?check_suite_focus=true

darksylinc · 2022-02-05T18:06:08Z

Linters: https://github.com/ignitionrobotics/ign-rendering/runs/5016366064?check_suite_focus=true

Ooops!!! Thanks! Fixed codecheck errors. I thought I had ran it but obviously I didn't.

darksylinc · 2022-02-07T15:13:12Z

PR was recently updated. Reasons:

Fixed feedback (codecheck)
Added -vulkan to command line
It is now possible to use GraphicsAPI::VULKAN
Integrated XLib code from ignition to be used by Vulkan

srmainwaring · 2022-03-01T13:24:27Z

@darksylinc I'm happy to test on macOS / Metal once I've got a local build of Ogre2.3.

We should add a note that there is an upstream dependency on https://github.com/osrf/homebrew-simulation having a formula for ogre2.3.

Update 5 March

Built ogre-next v2.3 on macOS (Big Sur 11.6.2, Xcode 13.2.1)
1 test fails: 59 - UNIT_RenderingIface_TEST (SEGFAULT), but this test fails running locally for v2.2 as well
Ran examples with the additional arguments ogre2 metal and working (custom_shaders_uniforms is missing the metal shaders in this branch so that was not tested).
Particle demo has correct colours

One change in CMakeLists.txt needed:

 #--------------------------------------
 # Find OGRE2
-ign_find_package(IgnOGRE2 VERSION 2.2.0
+ign_find_package(IgnOGRE2 VERSION 2.3.0
     COMPONENTS HlmsPbs HlmsUnlit Overlay
     REQUIRED_BY ogre2
     PRIVATE_FOR ogre2)

This is the shell script I use to build and install ogre2.3 into /usr/local/Cellar. It mimics the osrf/simulation/ogre2.x.rb formula. It should be copied to the ogre-next project root and run from there.

brew_install.sh.zip

# clone ogre-next v2-3
$ git clone https://github.com/OGRECave/ogre-next.git ogre-next2.3 -b v2-3
$ cd ogre-next2.3

# unlink ogre2.2 (there will be some conflicts in /usr/local/lib otherwise)
$ brew unlink ogre2.2

# build and install
$ ./brew_install.sh

iche033 · 2022-03-05T01:13:04Z

I started testing this. Here's my first pass at it.

All tests are passing!

Ran the examples with GL3+. Demos are working except simple_demo_qml, which crashes with this shader compile error:

relevant lines from ogre2.log

16:58:44: GLSL compile log: 100000000VertexShader_vs
0(317) : error C0000: syntax error, unexpected identifier, expecting reserved word or reserved word "in" or reserved word 
"out" or reserved word "uniform" at token "restrict"
0(344) : error C1503: undefined variable "worldMatBuf"

Ran the examples with Vulkan:

simple_demo_qml shows black screen. I think it needs similar treatment as Add fallback rendering for other APIs gz-gui#357?

heightmap demo crashes with this error:

relevant lines from ogre2.log

OGRE EXCEPTION(3:RenderingAPIException): Vertex Program 100000000VertexShader_vs failed to compile. See compile log above for details. in VulkanProgram::compile at /home/ian/code/ogre_2.3_ws/src/ogre-next/RenderSystems/Vulkan/src/OgreVulkanProgram.cpp (line 667)
17:03:45: Vulkan GLSL compiler error in 100000000VertexShader_vs:
ERROR: 0:396: 'ogre_t0' : unrecognized layout identifier, or qualifier requires assignment (e.g., binding = 4) 
ERROR: 0:396: '' : compilation terminated 
ERROR: 2 compilation errors.  No code generated.

particles_demo renders incorrect color - looks like R and B values are swapped?

That's it. Everything else seems to be working well!

iche033 · 2022-03-05T01:20:26Z

Here are my notes for getting ogre-next v2-3 branch built with vulkan support on Ubuntu in case it helps someone reviewing this PR:

Install vulkan-sdk following the instruction here: https://vulkan.lunarg.com/doc/sdk/1.3.204.1/linux/getting_started_ubuntu.html
Install libxcb-randr0-dev
Build and install ogre-next

To build ign-rendering with ogre2.3, for now you may need to uninstall libogre-2.2* libraries or just test this in docker. We'll need to update FindIgnOgre2.cmake in ign-cmake so we can find both ogre 2.2. and ogre 2.3. After that, update this line to use GraphicsAPI::VULKAN

darksylinc · 2022-03-05T01:21:28Z

Thanks for the feedback! I'll take a look at them tomorrow.

particles_demo renders incorrect color - looks like R and B values are swapped?

Interestingly this was reported a few days ago. However this is a Vulkan upstream bug; did I accidentally leave Vulkan enabled? (It should default to OpenGL). Or maybe I accidentally introduced an OpenGL bug? Anyways I'll take a look

iche033 · 2022-03-05T01:22:47Z

did I accidentally leave Vulkan enabled? (It should default to OpenGL)

I manually updated Ogre2RenderTarget.cc to use Vulkan :)

darksylinc · 2022-03-05T20:41:02Z

simple_demo_qml shows black screen. I think it needs similar treatment as

I didn't yet look at this one

heightmap demo crashes with this error:
relevant lines from ogre2.log

Fixed, it will need you to pull latest changes from ign-rendering branch AND latest changes from Ogre 2.3 (there was an upstream bug + various bugs in the new ign-rendering code)

particles_demo renders incorrect color - looks like R and B values are swapped?

Fixed, you will need to pull latest changes from Ogre 2.3 (it was purely an upstream bug)

iche033 · 2022-03-08T02:32:08Z

nice, confirmed that heightmap and particles issues are fixed!

acxz · 2022-06-14T12:56:28Z

It seems like there are some merge conflicts in this PR due to the renaming of ign to gz. @darksylinc would these be easy to fix and update the PR?

mjcarroll · 2022-06-15T19:47:57Z

We now have some OGRE 2.3 debs ready for focal and jammy testing. I'm going to pull this into a branch on this repository and fix the differences introduced by the renaming.

plaincolor_fs_Metal depends on plaincolor_vs_Metal which causes a crash with OgreNext 2.3.1 Signed-off-by: Matias N. Goldberg <[email protected]>

Signed-off-by: Matias N. Goldberg <[email protected]>

darksylinc · 2022-07-23T19:10:19Z

I fixed the Metal crash.

I also rebased with latest changes.

Signed-off-by: Matias N. Goldberg <[email protected]>

mjcarroll · 2022-07-26T18:43:46Z

we have homebrew working in #681 , but getting a new series of errors that are centered around:

: [ RUN      ] Camera/CameraTest.Track/ogre2
85: [Msg] Loading plugin [gz-rendering-ogre2]
85: unknown file: Failure
85: C++ exception with description "OGRE EXCEPTION(3:RenderingAPIException): Fragment Program 100000000PixelShader_ps failed to compile. See compile log above for details. in GLSLShader::compile at /tmp/ogre2.3-20220725-29639-6uzcl/ogre-next-2.3.1/RenderSystems/GL3Plus/src/GLSL/OgreGLSLShader.cpp (line 364)" thrown in the test body.
85: [  FAILED  ] Camera/CameraTest.Track/ogre2, where GetParam() = "ogre2" (215 ms)

mjcarroll · 2022-07-26T19:14:10Z

@osrf-jenkins retest this please

darksylinc · 2022-07-26T20:18:57Z

Do you know how to repro?

That issue popped up after last weekend's merges with latest main where more 'ign' stuff was changed into 'gz' so code from this PR stopped working

I thought I found them all

mjcarroll · 2022-07-26T20:50:52Z

Do you know how to repro?

I can consistently reproduce with the ogre2.3 that we have built for homebrew https://github.com/osrf/homebrew-simulation/blob/master/Formula/ogre2.3.rb

I don't think that we have done anything exotic here, @scpeters included some more logs in the other issue as well. #681

darksylinc · 2022-07-26T22:09:00Z

I can consistently reproduce with the ogre2.3 that we have built for homebrew https://github.com/osrf/homebrew-simulation/blob/master/Formula/ogre2.3.rb

But I mean how to repro. Is this opening the GUI? opening a particular SDF? The unit tests?

mjcarroll · 2022-07-26T22:13:05Z

But I mean how to repro. Is this opening the GUI? opening a particular SDF? The unit tests?

Aha, sorry. The INTEGRATION_Camera_TEST will fail.

scpeters · 2022-07-26T22:19:28Z

But I mean how to repro. Is this opening the GUI? opening a particular SDF? The unit tests?

Aha, sorry. The INTEGRATION_Camera_TEST will fail.

here is the full list of failing tests:

https://build.osrfoundation.org/job/ignition_rendering-ci-pr_any-homebrew-amd64/2618/testReport/

some info can be seen in the console log, but the ogre2.log file is most informative (see #681 (comment) and #681 (comment))

Signed-off-by: Michael Carroll <[email protected]>

iche033 · 2022-07-29T00:40:08Z

I went through all the examples on macOS M1 with metal backend. All the ones that I expect to work in ogre2 are working well. There is a small flickering issue with transparent objects in the visualization_demo that I see on mac but not on ubuntu. I think it could be due to shadows. This is minor and can be addressed later.

…ebosim#685) As we have increased our number of potential configurations, we needed a bit of a refactor to be able to test all of the configurations. This changes the tests to no longer use gtest's parameterization, but rather use ctest's Signed-off-by: Michael Carroll <[email protected]>

* Port remaining tests to common framework Signed-off-by: Michael Carroll <[email protected]>

mjcarroll

Letting CI turn over one more time with all changes incorporated.

iche033

unless the CI comes back with unexpected test failures, I think this is good to go. Let's get it in to unblock other PRs.

mjcarroll · 2022-07-29T04:59:07Z

As a sanity check, sensors_demo.sdf running in gz-sim garden with OGRE2.3 OpenGL

i want to merge

acxz · 2022-07-29T11:26:40Z

Just want to say thanks everyone for their hard work!!! Especially @darksylinc for iterating on everyone's reviews and comments! I'm gonna be excited for the Garden release!

darksylinc requested a review from iche033 as a code owner February 1, 2022 04:22

github-actions bot added the 🌱 garden Ignition Garden label Feb 1, 2022

ahcorde requested changes Feb 1, 2022

View reviewed changes

darksylinc mentioned this pull request Feb 19, 2022

Add fallback rendering for other APIs gazebosim/gz-gui#357

Merged

7 tasks

darksylinc force-pushed the matias-ogre2.3 branch from 6529b42 to 9630aa7 Compare February 26, 2022 15:39

iche033 mentioned this pull request Mar 2, 2022

Make sure shader param exists before setting its value #558

Merged

7 tasks

srmainwaring mentioned this pull request Mar 5, 2022

Add formula for ogre2.3 osrf/homebrew-simulation#1825

Closed

darksylinc force-pushed the matias-ogre2.3 branch from 345336f to efdaff5 Compare March 12, 2022 23:47

srmainwaring mentioned this pull request Mar 18, 2022

Add Metal support for the Ogre2 Render Engine #461

Closed

18 tasks

darksylinc force-pushed the matias-ogre2.3 branch from efdaff5 to 3b2c864 Compare May 1, 2022 19:16

iche033 mentioned this pull request May 5, 2022

Potential non-deterministic behavior in particle emitter (ogre2) #556

Open

acxz mentioned this pull request Jun 14, 2022

[ignition-gazebo] unable to initialize renderer acxz/gazebo-arch#2

Closed

darksylinc added 2 commits July 23, 2022 15:18

Fix macOS / Metal crash

775c423

plaincolor_fs_Metal depends on plaincolor_vs_Metal which causes a crash with OgreNext 2.3.1 Signed-off-by: Matias N. Goldberg <[email protected]>

Fix bad rebase

983638b

Signed-off-by: Matias N. Goldberg <[email protected]>

darksylinc force-pushed the matias-ogre2.3 branch from 6471798 to 983638b Compare July 23, 2022 19:09

Fix failing tests due to ign -> gz rename

72b59de

Signed-off-by: Matias N. Goldberg <[email protected]>

chapulina added the Breaking change Breaks API, ABI or behavior. Must target unstable version. label Jul 25, 2022

mjcarroll mentioned this pull request Jul 26, 2022

Combination of #685 and #694 #681

Merged

Update dependencies and add to RPath for debs

3beb99c

Signed-off-by: Michael Carroll <[email protected]>

mjcarroll mentioned this pull request Jul 29, 2022

Bump actions dependencies to Ogre 2.3 gazebosim/gz-sim#1620

Merged

mjcarroll added 2 commits July 28, 2022 23:43

Port remaining tests to common framework (gazebosim#694)

37da046

* Port remaining tests to common framework Signed-off-by: Michael Carroll <[email protected]>

mjcarroll requested a review from ahcorde July 29, 2022 04:47

mjcarroll approved these changes Jul 29, 2022

View reviewed changes

iche033 approved these changes Jul 29, 2022

View reviewed changes

mjcarroll merged commit dd95d0b into gazebosim:main Jul 29, 2022

This was referenced Aug 29, 2022

Add water-mass tracking support for DVL osrf/lrauv#241

Merged

GPU lidar sensor misses heightmap visuals #712

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port ign-rendering to Ogre 2.3 #553

Port ign-rendering to Ogre 2.3 #553

darksylinc commented Feb 1, 2022 •

edited

Loading

ahcorde left a comment

darksylinc commented Feb 5, 2022

darksylinc commented Feb 7, 2022

srmainwaring commented Mar 1, 2022 •

edited

Loading

iche033 commented Mar 5, 2022 •

edited

Loading

iche033 commented Mar 5, 2022 •

edited

Loading

darksylinc commented Mar 5, 2022 •

edited

Loading

iche033 commented Mar 5, 2022

darksylinc commented Mar 5, 2022

iche033 commented Mar 8, 2022

acxz commented Jun 14, 2022

mjcarroll commented Jun 15, 2022

darksylinc commented Jul 23, 2022

mjcarroll commented Jul 26, 2022

mjcarroll commented Jul 26, 2022

darksylinc commented Jul 26, 2022

mjcarroll commented Jul 26, 2022

darksylinc commented Jul 26, 2022

mjcarroll commented Jul 26, 2022

scpeters commented Jul 26, 2022

iche033 commented Jul 29, 2022 •

edited

Loading

mjcarroll left a comment

iche033 left a comment

mjcarroll commented Jul 29, 2022

acxz commented Jul 29, 2022

Port ign-rendering to Ogre 2.3 #553

Port ign-rendering to Ogre 2.3 #553

Conversation

darksylinc commented Feb 1, 2022 • edited Loading

🦟 Bug fix

Summary

Main changes:

Checklist

ahcorde left a comment

Choose a reason for hiding this comment

darksylinc commented Feb 5, 2022

darksylinc commented Feb 7, 2022

srmainwaring commented Mar 1, 2022 • edited Loading

iche033 commented Mar 5, 2022 • edited Loading

iche033 commented Mar 5, 2022 • edited Loading

darksylinc commented Mar 5, 2022 • edited Loading

iche033 commented Mar 5, 2022

darksylinc commented Mar 5, 2022

iche033 commented Mar 8, 2022

acxz commented Jun 14, 2022

mjcarroll commented Jun 15, 2022

darksylinc commented Jul 23, 2022

mjcarroll commented Jul 26, 2022

mjcarroll commented Jul 26, 2022

darksylinc commented Jul 26, 2022

mjcarroll commented Jul 26, 2022

darksylinc commented Jul 26, 2022

mjcarroll commented Jul 26, 2022

scpeters commented Jul 26, 2022

iche033 commented Jul 29, 2022 • edited Loading

mjcarroll left a comment

Choose a reason for hiding this comment

iche033 left a comment

Choose a reason for hiding this comment

mjcarroll commented Jul 29, 2022

acxz commented Jul 29, 2022

darksylinc commented Feb 1, 2022 •

edited

Loading

srmainwaring commented Mar 1, 2022 •

edited

Loading

iche033 commented Mar 5, 2022 •

edited

Loading

iche033 commented Mar 5, 2022 •

edited

Loading

darksylinc commented Mar 5, 2022 •

edited

Loading

iche033 commented Jul 29, 2022 •

edited

Loading