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

Merge main branch into Ogre 2.2 and fixes #359

Merged

Conversation

darksylinc
Copy link
Contributor

🦟 Bug fix

  • Merged main branch into Ogre 2.2 branch
    • Rather do this now than later, while I still have fresh in my mind the code written for the main branch
  • Restored FSAA (this bug affected main branch as well, but another minor bug in 2.2 branch prevented the fixes from main branch from working). Also went for explicit resolve route which was planned from the start, so that VRAM saving feature could be implemented.
  • Save VRAM if FSAA is used + no postprocessing. This optimization is exclusive to Ogre 2.2+
    • This feature deprecates another overload of UpdateRenderPassChain
  • Fix bug where changing background colour would not take immediate effect unless the compositor was rebuilt. See changes to Ogre2RenderTarget::UpdateBackgroundColor
  • Removed Clear plasses in favour of LoadActions, which are more efficient in modern GPUs

I apologize for this monolith of a PR. There were many fixes and improvements to implement which relied on merging main -> 2.2 and this merge causes too many conflicts to easily split this into multiple smaller PRs.

All ign-rendering tests are passing.

Summary

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

darksylinc and others added 30 commits June 8, 2021 19:58
* Fix floating point precision bug handling alpha channel (gazebosim#332)

Fixes gazebosim#332
Fixes gazebosim#108

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

* test reenabling depth camera test on mac

Signed-off-by: Ian Chen <[email protected]>

Co-authored-by: Matias N. Goldberg <[email protected]>
Using a custom deallocator to avoid breaking ABI

Alternatively C++17 supports the following syntax, which was not used:

`typedef std::shared_ptr<unsigned char[]> DataPtr;`

Signed-off-by: Matias N. Goldberg <[email protected]>
PF_RGB is 3 bytes. But later on Ogre2SelectionBuffer::OnSelectionClick
will try to read 4 bytes from it.

Fixed by ensuring it's always at least 4 bytes and zero-initializing
those 4 bytes.

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

Co-authored-by: Ian Chen <[email protected]>
* update test

Signed-off-by: Ian Chen <[email protected]>

* reenable macos test

Signed-off-by: Ian Chen <[email protected]>

* fix typo

Signed-off-by: Ian Chen <[email protected]>
* Fix FSAA in UI and lower VRAM consumption

FSAA was being requested however due to how the compositor was setup,
this effect was not taking effect.

Additionally, the Compositor setup was improved to lower memory
consumption. Originally the setup was taken from Ogre samples which
assume they will ultimately be rendering to a window.

However this is not the case and thus IGN was creating 3 render targets
(two for ping-ponging between postprocess FXs + one for storing the
final result)
This was optimized so that we only create 2 render targets: two for
ping-ponging between postprocess FXs and we pick at runtime which one is
storing the final result via the new variable renderTargetResultsIdx

Further performance optimizations could be made in Ogre 2.2 to improve
unnecessary MSAA resolving when doing postprocess, though considering
there's currently only one postprocessing effect (the Gaussian filter) I
doubt this optimization would make much of a difference

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

* Add Ogre2RenderTarget::RenderTarget back

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

* Rewrote the compositor changes to support RenderWindows

As a bonus this new method breaks ABI far less.
Fix leak: DestroyCompositor would often not be called

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

* Mantain ABI compatibility #Ogre2IsRenderWindowABI

When merging to newer branches that can break the ABI,
revert this commit

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

* Fix ABI problems

I gave up on commit undoing

It's clear that on the next release, Ogre2RenderTarget and
Ogre2RenderTexture need to be merged together.

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

* Fix camel case convention

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

* Make Ogre2RenderTarget::RenderTarget pure virtual again

Hopefully this will prevent ABI breakage

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

* Fix deprecation warnings during build

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

* Fix invalid write of size 8

This was causing heap corruption. At best it would crash. At worst it
would manifeset in subtle weird behaviors

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

Co-authored-by: Ian Chen <[email protected]>
* Fix heap overflow when reading (gazebosim#337)

PF_RGB is 3 bytes. But later on Ogre2SelectionBuffer::OnSelectionClick
will try to read 4 bytes from it.

Fixed by ensuring it's always at least 4 bytes and zero-initializing
those 4 bytes.

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

Co-authored-by: Ian Chen <[email protected]>

* Fix new [] / delete mismatch (gazebosim#338)

Using a custom deallocator to avoid breaking ABI

Alternatively C++17 supports the following syntax, which was not used:

`typedef std::shared_ptr<unsigned char[]> DataPtr;`

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

Co-authored-by: darksylinc <[email protected]>
* apply changes

Signed-off-by: Ian Chen <[email protected]>

* remove line

Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
The mere presence of these files can cause incorrect shader generation
or unknown visual bugs.

IMPORTANT: The installation folder (i.e. CMAKE_INSTALL_PREFIX) must
remove them as well.

`make install` won't be enough because it won't remove files, only add
new ones or update existing ones.

This folder is usually installed in
ign/install/share/ignition/ignition-rendering6/ogre2/media/Hlms

Signed-off-by: Matias N. Goldberg <[email protected]>
- ogreRoot may be nullptr
- Do not destroy textures already scheduled for destruction

Signed-off-by: Matias N. Goldberg <[email protected]>
Signed-off-by: Matias N. Goldberg <[email protected]>
Changed pass_clear in favour of LoadAction::Clear which is more
efficient on very modern GPUs and specially TBDR ones.

Signed-off-by: Matias N. Goldberg <[email protected]>
which is more efficient on very modern GPUs and specially TBDR ones.

Signed-off-by: Matias N. Goldberg <[email protected]>
Signed-off-by: Matias N. Goldberg <[email protected]>
There's an unused texture when these conditions are met (which are
fairly common)
This memory optimization could not be performed in Ogre 2.1, it needs
Ogre 2.2+

Signed-off-by: Matias N. Goldberg <[email protected]>
@darksylinc darksylinc requested a review from iche033 as a code owner July 3, 2021 21:11
@codecov
Copy link

codecov bot commented Jul 3, 2021

Codecov Report

Merging #359 (c19ec56) into ahcorde/update/ogre2.2 (970d862) will increase coverage by 0.03%.
The diff coverage is 93.83%.

Impacted file tree graph

@@                    Coverage Diff                     @@
##           ahcorde/update/ogre2.2     #359      +/-   ##
==========================================================
+ Coverage                   57.99%   58.03%   +0.03%     
==========================================================
  Files                         166      166              
  Lines                       16528    16550      +22     
==========================================================
+ Hits                         9586     9605      +19     
- Misses                       6942     6945       +3     
Impacted Files Coverage Δ
ogre2/src/Ogre2RenderTargetMaterial.cc 0.00% <ø> (ø)
ogre2/src/Ogre2RenderTarget.cc 85.53% <90.99%> (+0.82%) ⬆️
ogre2/src/Ogre2DepthCamera.cc 86.92% <95.45%> (-0.80%) ⬇️
ogre2/src/Ogre2GpuRays.cc 95.52% <100.00%> (ø)
ogre2/src/Ogre2ThermalCamera.cc 89.31% <100.00%> (ø)
src/Image.cc 76.92% <100.00%> (+3.01%) ⬆️
...e/ignition/rendering/base/BaseGaussianNoisePass.hh 100.00% <0.00%> (+3.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 970d862...c19ec56. Read the comment docs.

ogre2/src/Ogre2DepthCamera.cc Outdated Show resolved Hide resolved
Syntax cosmetic changes for consistency

Signed-off-by: Matias N. Goldberg <[email protected]>
@darksylinc
Copy link
Contributor Author

darksylinc commented Jul 5, 2021

I've updated the PR with the suggested changes and removed code deprecated for ign-rendering5.

Now DCO bot says my tweets are unverified though Github disagrees; DCO bot is expecting me to use my github handle as username, for some reason.

Strange is, it had been working until now

Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

Did some quick tests with a few worlds in ign-gazebo and seems to be working fine.

The ogre2_demo couldn't start though. I'm getting:

terminate called after throwing an instance of 'Ogre::InvalidParametersException'
  what():  OGRE EXCEPTION(2:InvalidParametersException): RenderTexture 'scene::RenderTexture(65435)0' must be resident! in GL3PlusRenderPassDescriptor::updateColourFbo at /home/osrf/code/ign-ogre_ws/src/ogre-next/RenderSystems/GL3Plus/src/OgreGL3PlusRenderPassDescriptor.cpp (line 223)
Aborted (core dumped)

ogre2/src/Ogre2RenderTarget.cc Show resolved Hide resolved
Signed-off-by: Matias N. Goldberg <[email protected]>
The "Final Composition" node requires both textures to be resident.
Thus 2nd texture must always be resident.

The optimization could still be applied if we create two Final
Composition nodes (one for when 2 textures are needed, another for when
only MSAA + 1 texture is needed) but this would:

  1. Hurt code readability too much (i.e. what is going on?)
  2. Increase debuggability difficulty too much because codepaths taken
would differ depending on whether optimization was applied. Also certain
bugs could remain hidden until compositors are toggled.

This was causing ogre2_demo to fail.

Signed-off-by: Matias N. Goldberg <[email protected]>
@darksylinc
Copy link
Contributor Author

Did some quick tests with a few worlds in ign-gazebo and seems to be working fine.

The ogre2_demo couldn't start though. I'm getting:

terminate called after throwing an instance of 'Ogre::InvalidParametersException'
  what():  OGRE EXCEPTION(2:InvalidParametersException): RenderTexture 'scene::RenderTexture(65435)0' must be resident! in GL3PlusRenderPassDescriptor::updateColourFbo at /home/osrf/code/ign-ogre_ws/src/ogre-next/RenderSystems/GL3Plus/src/OgreGL3PlusRenderPassDescriptor.cpp (line 223)
Aborted (core dumped)

Fixed. That slipped through my testing. I had to undo the VRAM saving code (reasons explained in the commit). It's just too much over-engineering to be worth it.

I thought it'd be easier as in, just flip a texture off and reroute a few nodes.

Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

works well for me now! I think windows CI is not yet working for ogre 2.2. Otherwise, other builds look good.

@iche033 iche033 requested a review from ahcorde July 12, 2021 22:53
@ahcorde ahcorde merged commit d3ce87d into gazebosim:ahcorde/update/ogre2.2 Jul 13, 2021
iche033 added a commit that referenced this pull request Sep 20, 2021
* Ogre2.2

Signed-off-by: ahcorde <[email protected]>

* Added media files

Signed-off-by: ahcorde <[email protected]>

* Added Headless mode

Signed-off-by: ahcorde <[email protected]>

* Added feedback

Signed-off-by: ahcorde <[email protected]>

* Added feedback

Signed-off-by: ahcorde <[email protected]>

* Add OGRE2.2 to dependencies

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

* Fixed GPUray and depthCamera test

Co-authored-by: Michael Carroll <[email protected]>

Signed-off-by: ahcorde <[email protected]>

* cast based on pf

Signed-off-by: Ian Chen <[email protected]>

* Fix cleanup with packaged debs

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

* fix copy raw data

Signed-off-by: Ian Chen <[email protected]>

* fix material test

Signed-off-by: Ian Chen <[email protected]>

* Lint and compiler warning

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

* Trim whitespace

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

* Apply gamma correction to sky

Signed-off-by: ahcorde <[email protected]>

* Test fixes to Ogre2.2 branch (#279)

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

* Revert gamma correction in the sky

Signed-off-by: ahcorde <[email protected]>

* enable grayscale albedo map to fix red color highlights and disable hardware gamma to fix dark sky color

Signed-off-by: Ian Chen <[email protected]>

* Local updates for Ogre2.2 against main branch (#317)

* Local updates for Ogre2.2 against main branch

Signed-off-by: Ian Chen <[email protected]>
Co-authored-by: Ian Chen <[email protected]>

* Fix merge

Signed-off-by: ahcorde <[email protected]>

* fix shadows in ogre 2.2

Signed-off-by: Ian Chen <[email protected]>

* Lint

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

* fix toggling sky

Signed-off-by: Ian Chen <[email protected]>

* fix camera test

Signed-off-by: Ian Chen <[email protected]>

* make linters happy

Signed-off-by: ahcorde <[email protected]>

* make linters happy

Signed-off-by: ahcorde <[email protected]>

* Fix small regression from merge

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

* Remove problematic leftover files from 2.1

The mere presence of these files can cause incorrect shader generation
or unknown visual bugs.

IMPORTANT: The installation folder (i.e. CMAKE_INSTALL_PREFIX) must
remove them as well.

`make install` won't be enough because it won't remove files, only add
new ones or update existing ones.

This folder is usually installed in
ign/install/share/ignition/ignition-rendering6/ogre2/media/Hlms

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

* Do not crash on shutdown

- ogreRoot may be nullptr
- Do not destroy textures already scheduled for destruction

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

* Remove problematic leftover files from 2.1 (#354)

The mere presence of these files can cause incorrect shader generation
or unknown visual bugs.

IMPORTANT: The installation folder (i.e. CMAKE_INSTALL_PREFIX) must
remove them as well.

`make install` won't be enough because it won't remove files, only add
new ones or update existing ones.

This folder is usually installed in
ign/install/share/ignition/ignition-rendering6/ogre2/media/Hlms

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

* Do not crash on shutdown (#355)

- ogreRoot may be nullptr
- Do not destroy textures already scheduled for destruction

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

* Removed unused variable

Signed-off-by: ahcorde <[email protected]>

* Restore FSAA support in 2.2 branch

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

* Fix changing background color not always taking immediate effect

Changed pass_clear in favour of LoadAction::Clear which is more
efficient on very modern GPUs and specially TBDR ones.

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

* Changed pass_clear in favour of LoadAction::Clear

which is more efficient on very modern GPUs and specially TBDR ones.

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

* Missing public keyword

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

* Save VRAM when FSAA is used and no postprocessing

There's an unused texture when these conditions are met (which are
fairly common)
This memory optimization could not be performed in Ogre 2.1, it needs
Ogre 2.2+

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

* Remove code deprecated in ign-rendering5

Syntax cosmetic changes for consistency

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

* Remove code deprecated in ign-rendering5

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

* Code style fixes

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

* Undo VRAM saving optimization: It cannot be applied

The "Final Composition" node requires both textures to be resident.
Thus 2nd texture must always be resident.

The optimization could still be applied if we create two Final
Composition nodes (one for when 2 textures are needed, another for when
only MSAA + 1 texture is needed) but this would:

  1. Hurt code readability too much (i.e. what is going on?)
  2. Increase debuggability difficulty too much because codepaths taken
would differ depending on whether optimization was applied. Also certain
bugs could remain hidden until compositors are toggled.

This was causing ogre2_demo to fail.

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

* Merge main branch into Ogre 2.2 and fixes (#359)

* Fix floating point precision bug handling alpha channel (#332) (#333)

Fixes #332
Fixes #108

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

* Test re-enabling depth camera integration test on mac (#335)

* Fix floating point precision bug handling alpha channel (#332)

Fixes #332
Fixes #108

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

* test reenabling depth camera test on mac

Signed-off-by: Ian Chen <[email protected]>

Co-authored-by: Matias N. Goldberg <[email protected]>

* Fix new [] / delete mismatch (#338)

Using a custom deallocator to avoid breaking ABI

Alternatively C++17 supports the following syntax, which was not used:

`typedef std::shared_ptr<unsigned char[]> DataPtr;`

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

* Fix heap overflow when reading (#337)

PF_RGB is 3 bytes. But later on Ogre2SelectionBuffer::OnSelectionClick
will try to read 4 bytes from it.

Fixed by ensuring it's always at least 4 bytes and zero-initializing
those 4 bytes.

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

Co-authored-by: Ian Chen <[email protected]>

* Fix floating point precision bug handling alpha channel (#332) (#333)

Fixes #332
Fixes #108

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

* Fix depth alpha (#316)

* update test

Signed-off-by: Ian Chen <[email protected]>

* reenable macos test

Signed-off-by: Ian Chen <[email protected]>

* fix typo

Signed-off-by: Ian Chen <[email protected]>

* cherry pick f9f1820 and fix conflicts

Signed-off-by: Ian Chen <[email protected]>

* Fix FSAA in UI and lower VRAM consumption (#313)

* Fix FSAA in UI and lower VRAM consumption

FSAA was being requested however due to how the compositor was setup,
this effect was not taking effect.

Additionally, the Compositor setup was improved to lower memory
consumption. Originally the setup was taken from Ogre samples which
assume they will ultimately be rendering to a window.

However this is not the case and thus IGN was creating 3 render targets
(two for ping-ponging between postprocess FXs + one for storing the
final result)
This was optimized so that we only create 2 render targets: two for
ping-ponging between postprocess FXs and we pick at runtime which one is
storing the final result via the new variable renderTargetResultsIdx

Further performance optimizations could be made in Ogre 2.2 to improve
unnecessary MSAA resolving when doing postprocess, though considering
there's currently only one postprocessing effect (the Gaussian filter) I
doubt this optimization would make much of a difference

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

* Add Ogre2RenderTarget::RenderTarget back

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

* Rewrote the compositor changes to support RenderWindows

As a bonus this new method breaks ABI far less.
Fix leak: DestroyCompositor would often not be called

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

* Mantain ABI compatibility #Ogre2IsRenderWindowABI

When merging to newer branches that can break the ABI,
revert this commit

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

* Fix ABI problems

I gave up on commit undoing

It's clear that on the next release, Ogre2RenderTarget and
Ogre2RenderTexture need to be merged together.

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

* Fix camel case convention

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

* Make Ogre2RenderTarget::RenderTarget pure virtual again

Hopefully this will prevent ABI breakage

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

* Fix deprecation warnings during build

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

* Fix invalid write of size 8

This was causing heap corruption. At best it would crash. At worst it
would manifeset in subtle weird behaviors

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

Co-authored-by: Ian Chen <[email protected]>

* Backport memory fixes found by ASAN (#340)

* Fix heap overflow when reading (#337)

PF_RGB is 3 bytes. But later on Ogre2SelectionBuffer::OnSelectionClick
will try to read 4 bytes from it.

Fixed by ensuring it's always at least 4 bytes and zero-initializing
those 4 bytes.

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

Co-authored-by: Ian Chen <[email protected]>

* Fix new [] / delete mismatch (#338)

Using a custom deallocator to avoid breaking ABI

Alternatively C++17 supports the following syntax, which was not used:

`typedef std::shared_ptr<unsigned char[]> DataPtr;`

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

Co-authored-by: darksylinc <[email protected]>

* recreate node only when needed (#342)

Signed-off-by: Ian Chen <[email protected]>

* Fix custom shaders uniforms ign version number (#343)

Signed-off-by: Ian Chen <[email protected]>

* relax gaussian test tolerance (#344)

Signed-off-by: Ian Chen <[email protected]>

* Update light map tutorial (#346)

* apply changes

Signed-off-by: Ian Chen <[email protected]>

* remove line

Signed-off-by: Ian Chen <[email protected]>

* add ifdef for apple in integration test

Signed-off-by: Ian Chen <[email protected]>

* 🎈 4.8.0 (#348)

Signed-off-by: Louise Poubel <[email protected]>

* update ign-rendering version in custom shaders uniform sample

Signed-off-by: Ian Chen <[email protected]>

* Remove problematic leftover files from 2.1

The mere presence of these files can cause incorrect shader generation
or unknown visual bugs.

IMPORTANT: The installation folder (i.e. CMAKE_INSTALL_PREFIX) must
remove them as well.

`make install` won't be enough because it won't remove files, only add
new ones or update existing ones.

This folder is usually installed in
ign/install/share/ignition/ignition-rendering6/ogre2/media/Hlms

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

* Do not crash on shutdown

- ogreRoot may be nullptr
- Do not destroy textures already scheduled for destruction

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

* Restore FSAA support in 2.2 branch

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

* Fix changing background color not always taking immediate effect

Changed pass_clear in favour of LoadAction::Clear which is more
efficient on very modern GPUs and specially TBDR ones.

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

* Changed pass_clear in favour of LoadAction::Clear

which is more efficient on very modern GPUs and specially TBDR ones.

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

* Missing public keyword

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

* Save VRAM when FSAA is used and no postprocessing

There's an unused texture when these conditions are met (which are
fairly common)
This memory optimization could not be performed in Ogre 2.1, it needs
Ogre 2.2+

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

* Remove code deprecated in ign-rendering5

Syntax cosmetic changes for consistency

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

* Remove code deprecated in ign-rendering5

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

* Code style fixes

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

* Undo VRAM saving optimization: It cannot be applied

The "Final Composition" node requires both textures to be resident.
Thus 2nd texture must always be resident.

The optimization could still be applied if we create two Final
Composition nodes (one for when 2 textures are needed, another for when
only MSAA + 1 texture is needed) but this would:

  1. Hurt code readability too much (i.e. what is going on?)
  2. Increase debuggability difficulty too much because codepaths taken
would differ depending on whether optimization was applied. Also certain
bugs could remain hidden until compositors are toggled.

This was causing ogre2_demo to fail.

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

Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>

* Began work on Heightmaps with Ogre2. Very WIP

Hardcoded some paths in CMakeLists to get Terra compiling

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

* Register HlmsTerra

Add its Hlms data files

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

* Update all Terras and show them on screen (WIP)

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

* Update to latest Terra to get Z-up support

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

* Better calculation of heightmap size (WIP)

Fix crash when heightmap is deleted
Update to latest Terra for setCustomSkirtMinHeight

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

* Fix shader compiler error

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

* Fix alignment issues in terrain

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

* Move Terra to its own folder and project

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

* Enable assert that was accidentally disabled

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

* Customize Terra for Ignition's requirements

Fix barrier assert

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

* Use custom blending modes to adjust to ign

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

* HeightmapTexture::Size is in meters, it's not a scale

Now it looks close to the reference in ogre1
No longer normalize weights, it's not needed anymore
Also fix bug when 5 texture maps could not be supported

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

* Use same blending algorithm for roughness & metalness

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

* Deal with edge case where terrain skirts could be above ground

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

* Fix wrong indentation

Remove dead code

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

* Forgot to push this change

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

* Disable alpha blending of detail maps for Terrain

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

* Add terrain shadows to normal objects

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

* Fix copyright year

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

* Re-enable ImageHeightmap

It shouldn't have been disabled

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

* Cosmetic changes

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

* Move Terra's source code into ogre2/src

It is ogre2 specific

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

* Support ogre1 heightmaps via cropping and warn about it

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

* Support both ogre2 & ogre engines in heightmap sample

Default to ogre2

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

* Force-disable JSON parts of Terra

It's not used by ignition.
This avoids detecting rapidjson during ign build
Fix Windows build

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

* Add license to Terra files

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

* Update to latest version of Terra

Adds spotlight & point shadow map support (not working/set yet)

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

* Add terrain shadows from spot & point lights

Use unique_ptr instead of raw ptrs when possible

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

* Skip Terra headers in cppcheck

They should be treated as external code

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

* fixing tests

Signed-off-by: Ian Chen <[email protected]>

* add ifdef

Signed-off-by: Ian Chen <[email protected]>

* suppress warnings

Signed-off-by: Ian Chen <[email protected]>

* update syntax

Signed-off-by: Ian Chen <[email protected]>

* update syntax

Signed-off-by: Ian Chen <[email protected]>

* Fix crash on shutdown due to ptr destroyed too late

It should be destroyed before Ogre Root

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

* Update to latest Terra to fix bug from upstream

Upstream
OGRECave/ogre-next@d0a01d2

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

* Update to latest Hlms from upstream

This fixes some compiler bugs with Terra
Also adds supports for HlmsPbs::setShadowReceiversInPixelShader which
was written specifically for Gazebo

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

* Fix compiler warning

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

* Fix Terra shadows on Spot and Point lights

Update Terra to latest from upstream

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

* Fix Terra's point light shadows

Terra's workspace listener was being executed before the camera is
rotated for cubemap rendering

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

* Fix documentation

Silence warnings from external code to pass build checks

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

* fixing CI

Signed-off-by: Ian Chen <[email protected]>

* add include path

Signed-off-by: Ian Chen <[email protected]>

* windows warning

Signed-off-by: Ian Chen <[email protected]>

* fix

Signed-off-by: Ian Chen <[email protected]>

Co-authored-by: ahcorde <[email protected]>
Co-authored-by: Michael Carroll <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
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.

4 participants