Skip to content

Commit

Permalink
Merge main branch into Ogre 2.2 and fixes (#359)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people authored Jul 13, 2021
1 parent 970d862 commit d3ce87d
Show file tree
Hide file tree
Showing 19 changed files with 525 additions and 347 deletions.
42 changes: 42 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,48 @@

### Ignition Rendering 4.X

### Ignition Rendering 4.8.0 (2021-06-18)

1. relax gaussian test tolerance
* [Pull request #344](https://github.com/ignitionrobotics/ign-rendering/pull/344)

1. recreate node only when needed
* [Pull request #342](https://github.com/ignitionrobotics/ign-rendering/pull/342)

1. Backport memory fixes found by ASAN
* [Pull request #340](https://github.com/ignitionrobotics/ign-rendering/pull/340)

1. Re-enable part of depth camera test on macOS
* [Pull request #335](https://github.com/ignitionrobotics/ign-rendering/pull/335)
* [Pull request #347](https://github.com/ignitionrobotics/ign-rendering/pull/347)

1. Fix depth alpha
* [Pull request #316](https://github.com/ignitionrobotics/ign-rendering/pull/316)

1. Fix floating point precision bug handling alpha channel (#332)
* [Pull request #333](https://github.com/ignitionrobotics/ign-rendering/pull/333)

1. Include MoveTo Helper class to ign-rendering
* [Pull request #311](https://github.com/ignitionrobotics/ign-rendering/pull/311)

1. Remove `tools/code_check` and update codecov
* [Pull request #321](https://github.com/ignitionrobotics/ign-rendering/pull/321)

1. [OGRE 1.x] Uniform buffer shader support
* [Pull request #294](https://github.com/ignitionrobotics/ign-rendering/pull/294)

1. Helper function to get a scene
* [Pull request #320](https://github.com/ignitionrobotics/ign-rendering/pull/320)

1. Reduce lidar data discretization
* [Pull request #296](https://github.com/ignitionrobotics/ign-rendering/pull/296)

1. Prevent console warnings when multiple texture coordinates are present
* [Pull request #301](https://github.com/ignitionrobotics/ign-rendering/pull/301)

1. Added command line argument to pick version of Ogre
* [Pull request #277](https://github.com/ignitionrobotics/ign-rendering/pull/277)

### Ignition Rendering 4.7.0 (2021-03-17)

1. Enable depth write in particles example
Expand Down
2 changes: 1 addition & 1 deletion examples/custom_shaders_uniforms/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ include_directories(SYSTEM
${PROJECT_BINARY_DIR}
)

find_package(ignition-rendering4)
find_package(ignition-rendering6)

set(TARGET_THIRD_PARTY_DEPENDS "")

Expand Down
1 change: 0 additions & 1 deletion ogre2/include/ignition/rendering/ogre2/Ogre2Includes.hh
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
#include <Compositor/OgreCompositorShadowNode.h>
#include <Compositor/OgreCompositorWorkspace.h>
#include <Compositor/OgreCompositorWorkspaceListener.h>
#include <Compositor/Pass/PassClear/OgreCompositorPassClearDef.h>
#include <Compositor/Pass/PassQuad/OgreCompositorPassQuadDef.h>
#include <Compositor/Pass/PassScene/OgreCompositorPassScene.h>
#include <Compositor/Pass/PassScene/OgreCompositorPassSceneDef.h>
Expand Down
50 changes: 41 additions & 9 deletions ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,36 @@ namespace ignition
/// \see Camera::SetShadowsNodeDefDirty
public: void SetShadowsNodeDefDirty();

/// \brief Get a pointer to the ogre render target
/// \brief Returns the FSAA to use based on supported specs by HW
/// and value specified in Ogre2RenderTarget::AntiAliasing
/// \return Value in range [1; 256). 1 means no antialiasing.
protected: uint8_t TargetFSAA() const;

/// \brief Get a pointer to the ogre render target containing
/// the results of the render (implemented separately
/// to avoid breaking ABI of the pure virtual function)
protected: Ogre::TextureGpu *RenderTargetImpl() const;

/// \brief Get a pointer to the ogre render target containing
/// the results of the render
public: virtual Ogre::TextureGpu *RenderTarget() const = 0;

/// \brief Returns true if this is a render window
/// TODO(anyone): this function should be virtual.
/// We didn't do it to preserve ABI.
/// Look in commit history for '#Ogre2IsRenderWindowABI' to
/// see changes made and revert
public: bool IsRenderWindow() const;

// Documentation inherited
public: unsigned int GLIdImpl() const;

/// \brief Destroy the render texture
protected: void DestroyTargetImpl();

/// \brief Build the render texture
protected: void BuildTargetImpl();

/// \brief Get visibility mask for the viewport associated with this
/// render target
/// \return Visibility mask
Expand All @@ -133,7 +160,10 @@ namespace ignition
Ogre::CompositorWorkspace *_workspace,
const std::string &_workspaceDefName,
const std::string &_baseNode, const std::string &_finalNode,
const std::vector<RenderPassPtr> &_renderPasses, bool _recreateNodes);
const std::vector<RenderPassPtr> &_renderPasses,
bool _recreateNodes,
Ogre::TextureGpu *(*_ogreTextures)[2],
bool _isRenderWindow);

/// \brief Update the background color
protected: virtual void UpdateBackgroundColor();
Expand All @@ -144,9 +174,6 @@ namespace ignition
/// \brief Update the render pass chain
protected: virtual void UpdateRenderPassChain();

/// \brief Deprecated. Use Ogre2Scene:UpdateShadowNode instead
protected: void IGN_DEPRECATED(5) UpdateShadowNode();

/// \brief Implementation of the Rebuild function
protected: virtual void RebuildImpl() override;

Expand Down Expand Up @@ -229,7 +256,9 @@ namespace ignition
// Documentation inherited
public: virtual unsigned int GLId() const override;

// Documentation inherited.
// Documentation inherited
// TODO(anyone): this function should be removed.
// We didn't do it to preserve ABI.
public: virtual Ogre::TextureGpu *RenderTarget() const override;

// Documentation inherited.
Expand All @@ -241,9 +270,6 @@ namespace ignition
/// \brief Build the render texture
protected: virtual void BuildTarget();

/// \brief Pointer to the internal ogre render texture object
protected: Ogre::TextureGpu *ogreTexture = nullptr;

/// \brief Make scene our friend so it can create a ogre2 render texture
private: friend class Ogre2Scene;
};
Expand All @@ -261,6 +287,12 @@ namespace ignition
// Documentation inherited.
public: virtual void Destroy() override;

// TODO(anyone): this function should be virtual.
// We didn't do it to preserve ABI.
// Looks in commit history for '#Ogre2IsRenderWindowABI' to
// see changes made and revert
public: bool IsRenderWindow() const;

// Documentation inherited.
public: virtual Ogre::TextureGpu *RenderTarget() const override;

Expand Down
Loading

0 comments on commit d3ce87d

Please sign in to comment.