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

Backport memory fixes found by ASAN #340

Merged
merged 2 commits into from
Jun 15, 2021
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jun 14, 2021

🦟 Bug fix

Summary

Backport pull request #337 and #338

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 2 commits June 14, 2021 13:48
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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>
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 <dark_sylinc@yahoo.com.ar>
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jun 14, 2021
@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #340 (c253240) into ign-rendering3 (fe96179) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering3     #340      +/-   ##
==================================================
+ Coverage           53.21%   53.23%   +0.01%     
==================================================
  Files                 131      131              
  Lines               12002    12007       +5     
==================================================
+ Hits                 6387     6392       +5     
  Misses               5615     5615              
Impacted Files Coverage Δ
ogre2/src/Ogre2SelectionBuffer.cc 79.68% <100.00%> (+0.32%) ⬆️
src/Image.cc 84.61% <100.00%> (+2.00%) ⬆️

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 fe96179...c253240. Read the comment docs.

@iche033 iche033 merged commit a56c834 into ign-rendering3 Jun 15, 2021
@iche033 iche033 deleted the backport_mem_fix branch June 15, 2021 16:21
ahcorde pushed a commit that referenced this pull request Jul 13, 2021
* Fix floating point precision bug handling alpha channel (#332) (#333)

Fixes #332
Fixes #108

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* test reenabling depth camera test on mac

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>

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

Fixes #332
Fixes #108

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix depth alpha (#316)

* update test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* reenable macos test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* fix typo

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* cherry pick f9f1820 and fix conflicts

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

* Add Ogre2RenderTarget::RenderTarget back

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Mantain ABI compatibility #Ogre2IsRenderWindowABI

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Fix camel case convention

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Make Ogre2RenderTarget::RenderTarget pure virtual again

Hopefully this will prevent ABI breakage

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix deprecation warnings during build

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: darksylinc <dark_sylinc@yahoo.com.ar>

* recreate node only when needed (#342)

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

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

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* relax gaussian test tolerance (#344)

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* Update light map tutorial (#346)

* apply changes

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* remove line

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* add ifdef for apple in integration test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* 🎈 4.8.0 (#348)

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* update ign-rendering version in custom shaders uniform sample

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

* Do not crash on shutdown

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Restore FSAA support in 2.2 branch

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Missing public keyword

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Remove code deprecated in ign-rendering5

Syntax cosmetic changes for consistency

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Remove code deprecated in ign-rendering5

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Code style fixes

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>
Co-authored-by: Louise Poubel <louise@openrobotics.org>
iche033 added a commit that referenced this pull request Sep 20, 2021
* Ogre2.2

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added media files

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added Headless mode

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Add OGRE2.2 to dependencies

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Fixed GPUray and depthCamera test

Co-authored-by: Michael Carroll <michael@openrobotics.org>

Signed-off-by: ahcorde <ahcorde@gmail.com>

* cast based on pf

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* Fix cleanup with packaged debs

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* fix copy raw data

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* fix material test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* Lint and compiler warning

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Trim whitespace

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Apply gamma correction to sky

Signed-off-by: ahcorde <ahcorde@gmail.com>

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

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Revert gamma correction in the sky

Signed-off-by: ahcorde <ahcorde@gmail.com>

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

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

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

* Local updates for Ogre2.2 against main branch

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Co-authored-by: Ian Chen <ichen@osrfoundation.org>

* Fix merge

Signed-off-by: ahcorde <ahcorde@gmail.com>

* fix shadows in ogre 2.2

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* Lint

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* fix toggling sky

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* fix camera test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* make linters happy

Signed-off-by: ahcorde <ahcorde@gmail.com>

* make linters happy

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fix small regression from merge

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* 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 <dark_sylinc@yahoo.com.ar>

* Do not crash on shutdown

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Do not crash on shutdown (#355)

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Removed unused variable

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Restore FSAA support in 2.2 branch

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Missing public keyword

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Remove code deprecated in ign-rendering5

Syntax cosmetic changes for consistency

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Remove code deprecated in ign-rendering5

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Code style fixes

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* test reenabling depth camera test on mac

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>

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

Fixes #332
Fixes #108

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix depth alpha (#316)

* update test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* reenable macos test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* fix typo

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* cherry pick f9f1820 and fix conflicts

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

* Add Ogre2RenderTarget::RenderTarget back

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Mantain ABI compatibility #Ogre2IsRenderWindowABI

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Fix camel case convention

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Make Ogre2RenderTarget::RenderTarget pure virtual again

Hopefully this will prevent ABI breakage

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix deprecation warnings during build

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: darksylinc <dark_sylinc@yahoo.com.ar>

* recreate node only when needed (#342)

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

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

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* relax gaussian test tolerance (#344)

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* Update light map tutorial (#346)

* apply changes

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* remove line

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* add ifdef for apple in integration test

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* 🎈 4.8.0 (#348)

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* update ign-rendering version in custom shaders uniform sample

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* 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 <dark_sylinc@yahoo.com.ar>

* Do not crash on shutdown

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Restore FSAA support in 2.2 branch

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Missing public keyword

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Remove code deprecated in ign-rendering5

Syntax cosmetic changes for consistency

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Remove code deprecated in ign-rendering5

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Code style fixes

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>
Co-authored-by: Louise Poubel <louise@openrobotics.org>

* Began work on Heightmaps with Ogre2. Very WIP

Hardcoded some paths in CMakeLists to get Terra compiling

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Register HlmsTerra

Add its Hlms data files

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Update to latest Terra to get Z-up support

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Better calculation of heightmap size (WIP)

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix shader compiler error

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix alignment issues in terrain

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Move Terra to its own folder and project

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Enable assert that was accidentally disabled

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Customize Terra for Ignition's requirements

Fix barrier assert

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Use custom blending modes to adjust to ign

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Use same blending algorithm for roughness & metalness

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix wrong indentation

Remove dead code

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Forgot to push this change

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Disable alpha blending of detail maps for Terrain

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Add terrain shadows to normal objects

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix copyright year

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Re-enable ImageHeightmap

It shouldn't have been disabled

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Cosmetic changes

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Move Terra's source code into ogre2/src

It is ogre2 specific

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Support ogre1 heightmaps via cropping and warn about it

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Support both ogre2 & ogre engines in heightmap sample

Default to ogre2

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Add license to Terra files

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Update to latest version of Terra

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

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Add terrain shadows from spot & point lights

Use unique_ptr instead of raw ptrs when possible

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Skip Terra headers in cppcheck

They should be treated as external code

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* fixing tests

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* add ifdef

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* suppress warnings

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* update syntax

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* update syntax

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* Fix crash on shutdown due to ptr destroyed too late

It should be destroyed before Ogre Root

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Update to latest Terra to fix bug from upstream

Upstream
OGRECave/ogre-next@d0a01d2

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Fix compiler warning

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* Fix Terra shadows on Spot and Point lights

Update Terra to latest from upstream

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* 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 <dark_sylinc@yahoo.com.ar>

* Fix documentation

Silence warnings from external code to pass build checks

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>

* fixing CI

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* add include path

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* windows warning

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* fix

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: ahcorde <ahcorde@gmail.com>
Co-authored-by: Michael Carroll <michael@openrobotics.org>
Co-authored-by: Ian Chen <ichen@osrfoundation.org>
Co-authored-by: Louise Poubel <louise@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants