Port ign-rendering to Ogre 2.3

[darksylinc]

🦟 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]

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]

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]

@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]

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]

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]

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]

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

I manually updated Ogre2RenderTarget.cc to use Vulkan :)

[darksylinc]

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]

nice, confirmed that heightmap and particles issues are fixed!

[acxz]

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]

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.

[darksylinc]

I fixed the Metal crash.

I also rebased with latest changes.

[mjcarroll]

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]

@osrf-jenkins retest this please

[darksylinc]

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]

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]

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]

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]

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))

[iche033]

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.

[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]

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

i want to merge

[acxz]

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!