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

Fix heap overflow when reading #337

Merged

Conversation

darksylinc
Copy link
Contributor

@darksylinc darksylinc commented Jun 14, 2021

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]

Summary

This is too simple of a fix and self explanatory. We were allocating 3 bytes of data and then reading 4 bytes in Ogre2SelectionBuffer::OnSelectionClick.

It may be possible to backport to older versions.

Detected by ASAN, just add to ign-rendering/ogre2/src/CMakeLists.txt:

set( CMAKE_C_FLAGS_DEBUG
	"${CMAKE_C_FLAGS_DEBUG} -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address" )
set( CMAKE_CXX_FLAGS_DEBUG
	"${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address" )
set( CMAKE_LINKER_FLAGS_DEBUG
	"${CMAKE_LINKER_FLAGS_DEBUG} -fsanitize=address" )

Checklist

@darksylinc darksylinc requested a review from iche033 as a code owner June 14, 2021 00:34
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Jun 14, 2021
@darksylinc darksylinc force-pushed the matias-selBufferReadOverflowFix5 branch from 598aca1 to 973cd42 Compare June 14, 2021 01:13
@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #337 (f0afe3b) into ign-rendering5 (f1994c7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-rendering5     #337   +/-   ##
===============================================
  Coverage           57.83%   57.83%           
===============================================
  Files                 161      161           
  Lines               15806    15808    +2     
===============================================
+ Hits                 9141     9143    +2     
  Misses               6665     6665           
Impacted Files Coverage Δ
ogre2/src/Ogre2SelectionBuffer.cc 79.68% <100.00%> (+0.32%) ⬆️

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 f1994c7...f0afe3b. Read the comment docs.

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]>
@darksylinc darksylinc force-pushed the matias-selBufferReadOverflowFix5 branch from 973cd42 to 2c70dfb Compare June 14, 2021 18:20
@iche033 iche033 merged commit 1421901 into gazebosim:ign-rendering5 Jun 14, 2021
iche033 added a commit that referenced this pull request Jun 14, 2021
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]>
@iche033 iche033 mentioned this pull request Jun 14, 2021
7 tasks
iche033 added a commit that referenced this pull request Jun 15, 2021
* 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]>
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 <[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]>
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
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants