Global illumination VCT & CI VCT #675
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 tasks
ahcorde
previously requested changes
Jul 18, 2022
Comment on lines
+47
to
+55
| target_compile_definitions(${ogre2_target} PRIVATE $<$<CONFIG:Debug>:DEBUG=1 _DEBUG=1>) | ||
|
|
||
|
|
There was a problem hiding this comment.
force debug ? can we remove this ?
darksylinc
force-pushed
the
matias-2.3-global-illumination-gz-renamed
branch
2 times, most recently
from
6562654
to
b4430ec
Compare
chapulina
added
the
Breaking change
darksylinc
force-pushed
the
matias-2.3-global-illumination-gz-renamed
branch
from
b4430ec
to
140bb27
Compare
|
Ahhhh!!! Finally! Now that Ogre 2.3 PR has been merged, I rebased the changes to main I see @ahcorde left some feedback already that I didn't see for some reason. I have to go through each one of them. |
darksylinc
force-pushed
the
matias-2.3-global-illumination-gz-renamed
branch
from
b387b8b
to
be35564
Compare
iche033
reviewed
Aug 5, 2022
| * | ||
| */ | ||
| #ifndef IGNITION_RENDERING_EXAMPLES_OGRE2_DEMO_GLUTWINDOW_HH_ | ||
| #define IGNITION_RENDERING_EXAMPLES_OGRE2_DEMO_GLUTWINDOW_HH_ |
There was a problem hiding this comment.
change to IGNITION to GZ and OGRE2_DEMO to GLOBAL_ILLUMINATION
There was a problem hiding this comment.
I also saw that it's now SDL instead of GLUT. Update the filename?
| #define IGNITION_RENDERING_EXAMPLES_OGRE2_DEMO_GLUTWINDOW_HH_ | ||
|
|
||
| #include <vector> | ||
| #include "ignition/rendering/RenderTypes.hh" |
| #include <vector> | ||
| #include "ignition/rendering/RenderTypes.hh" | ||
|
|
||
| namespace ir = ignition::rendering; |
examples/global_illumination/Main.cc
Outdated
| light3->SetCastShadows(false); | ||
| root->AddChild(light3); | ||
|
|
||
| // spot light test |
There was a problem hiding this comment.
I think we can just remove the spot light test (spot lights and models here) for the GI demo.
| SensorPtr sensor = scene->SensorByName("camera"); | ||
| // get render pass system | ||
| CameraPtr camera = std::dynamic_pointer_cast<Camera>(sensor); | ||
| RenderPassSystemPtr rpSystem = engine->RenderPassSystem(); |
There was a problem hiding this comment.
we can probably also just remove the render pass systems here to focus the demo on GI.
| /// will be updated. Thus if a single static object is constantly | ||
| /// changing, it will affect everything and the performance | ||
| /// profile will be as if all objects were dynamic. | ||
| /// \remark (INTERNAL) For implementations: |
There was a problem hiding this comment.
can you add a note to mention ogre2 here, ie. For ogre2 implementations
include/gz/rendering/Visual.hh
Outdated
| /// rarely makes sense | ||
| /// Dynamic Scene Node + Static MovableObject = Invalid | ||
| /// \param[in] _static True if this visual should be made static | ||
| public: virtual void SetVisualStatic(bool _static) = 0; |
There was a problem hiding this comment.
how about just calling this SetStatic since the function is part of the Visual class so implicitly it's implying that the function is setting a visual static?
| @@ -118,6 +118,28 @@ namespace gz | |||
| /// \param[in] _visible True if this visual should be made visible | |||
| public: virtual void SetVisible(bool _visible) = 0; | |||
|
|
|||
| /// \brief Tells Render Engine this Visual will be static (i.e. | |||
| /// won't move, rotate or scale) | |||
There was a problem hiding this comment.
can you add brief doc here on what happens when the node is static, e.g. subsequent calls to setting the node position / orientation won't have any effect?
From reading the ogre doc, it seems like the node position won't be updated until we explicitly request it ( by calling notifyStaticDirty?).
I have no problem with that but I have to ask... is there something you have in mind? Like what / how should I test? |
|
Removed the This PR can either be merged into |
That's fine. I received an email from Ian about this news and I agree. It otherwise feels too rushed. It's a lot of changes that could use feedback, testing, etc |
I'm thinking of simple unit tests for testing the GI APIs. For example: auto gi = _scene->CreateGlobalIlluminationVct();
const uint32_t resolution[3]{ 128u, 128u, 32u };
const uint32_t octantCount[3]{ 4, 4, 2 };
gi->SetResolution(resolution);
gi->SetAnisotropic(true);
gi->SetHighQuality(false);
gi->SetThinWallCounter(1.0f);
gi->SetOctantCount(octantCount);
gi->SetAnisotropic(false);
gi->Build();Just add checks to make sure the values are set correctly EXPECT_EQ(128u, gi->Resolution()[0]);
EXPECT_EQ(128u, gi->Resolution()[1]);
EXPECT_EQ(32u, gi->Resolution()[2]);
EXPECT_TRUE(gi->Anisotropic());
... |
|
waiting on version bumps |
iche033
force-pushed
the
matias-2.3-global-illumination-gz-renamed
branch
from
d531640
to
19bb3ea
Compare
It uses a script now. Textures are passed via the Compositor or else Ogre cannot setup proper layout transitions for Vulkan for these textures Fix INTEGRATION_gpu_rays under Vulkan Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
It uses a script now. Textures are passed via the Compositor or else Ogre cannot setup proper layout transitions for Vulkan for these textures Fix INTEGRATION_gpu_rays under Vulkan Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Add VCT required compute shaders Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Anisotropic VCT wasn't always taking effect Disable setAllowMultipleBounces on SetBounceCount(0) Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Add SetDebugVisualization Add Visual::SetVisualStatic Update to latest VCT shaders from upstream (needs updating Ogre 2.2 to latest version for them to work) Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Use d key to toggle debug visualization Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Ign -> gz naming changes Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
iche033
force-pushed
the
matias-2.3-global-illumination-gz-renamed
branch
from
19bb3ea
to
b831b35
Compare
…o, ign->gz Signed-off-by: Ian Chen <ichen@osrfoundation.org>
|
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Get some rest and take care! No hurry. Thanks for the tip. I'll look into it afterwards. |
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Codecov Report
@@ Coverage Diff @@
## main #675 +/- ##
==========================================
- Coverage 76.47% 73.62% -2.86%
==========================================
Files 168 177 +9
Lines 15289 15949 +660
==========================================
+ Hits 11693 11743 +50
- Misses 3596 4206 +610
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
alright this should be fixed. Turns out it's just the |
iche033
approved these changes
Dec 22, 2022
|
While testing the GI gui plugins in gazebosim/gz-sim#1597, I ran into a crash with GI CiVct. I'm able to reproduce it using with the here's the backtrace: backtraceThread 1 "global_illumina" received signal SIGSEGV, Segmentation fault.
0x00007ffff1edd966 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
(gdb) bt
#0 0x00007ffff1edd966 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#1 0x00007ffff1edda50 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#2 0x00007ffff1efc79e in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#3 0x00007ffff1e9a6ca in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#4 0x00007ffff1edcafa in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#5 0x00007ffff1e9aadd in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#6 0x00007ffff1eed245 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#7 0x00007ffff1f3185c in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#8 0x00007ffff1e90b39 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#9 0x00007ffff1e94941 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#10 0x00007ffff28883e8 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#11 0x00007ffff2892f00 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.510.85.02
#12 0x00007ffff1035b0a in Ogre::GLSLShader::compile(bool) () from /usr/lib/x86_64-linux-gnu/OGRE-2.3/OGRE/RenderSystem_GL3Plus.so
#13 0x00007ffff102945e in Ogre::GLSLMonolithicProgram::compileAndLink() () from /usr/lib/x86_64-linux-gnu/OGRE-2.3/OGRE/RenderSystem_GL3Plus.so
#14 0x00007ffff1029b1b in Ogre::GLSLMonolithicProgram::activate() () from /usr/lib/x86_64-linux-gnu/OGRE-2.3/OGRE/RenderSystem_GL3Plus.so
#15 0x00007ffff10272f9 in Ogre::GLSLMonolithicProgramManager::getActiveMonolithicProgram() () from /usr/lib/x86_64-linux-gnu/OGRE-2.3/OGRE/RenderSystem_GL3Plus.so
#16 0x00007ffff14a1cf4 in Ogre::HlmsCompute::dispatch(Ogre::HlmsComputeJob*, Ogre::SceneManager*, Ogre::Camera*) ()
from /usr/lib/x86_64-linux-gnu/OGRE-2.3/libOgreNextMain.so.2.3.1
#17 0x00007ffff4729b65 in Ogre::VctImageVoxelizer::build(Ogre::SceneManager*) () from /usr/lib/x86_64-linux-gnu/OGRE-2.3/libOgreNextHlmsPbs.so.2.3.1
#18 0x00007ffff472b044 in Ogre::VctImageVoxelizer::buildRelative(Ogre::SceneManager*, int, int, int, unsigned int, unsigned int, unsigned int) ()
from /usr/lib/x86_64-linux-gnu/OGRE-2.3/libOgreNextHlmsPbs.so.2.3.1
#19 0x00007ffff47237c5 in Ogre::VctCascadedVoxelizer::update(Ogre::SceneManager*) () from /usr/lib/x86_64-linux-gnu/OGRE-2.3/libOgreNextHlmsPbs.so.2.3.1
#20 0x00007ffff1a2a830 in gz::rendering::v8::Ogre2GlobalIlluminationCiVct::Build (this=0x555556e0bc50)
at /home/ian/code/gz_h_ws/src/gz-rendering/ogre2/src/Ogre2GlobalIlluminationCiVct.cc:403
#21 0x000055555555cfdb in buildScene(std::shared_ptr<gz::rendering::v8::Scene>) ()
#22 0x000055555555df3a in createCamera(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
#23 0x000055555555e44a in main () |
|
Does Ogre.log say something useful? |
|
I have nvidia drivers 510 installed through Ubuntu (Jammy) here's my ogre log. It's does not reveal much though ogre2.log |
|
I'm reviewing the data and this might be an OgreNext bug. The CiVct is already very taxing on resource limits imposed by GL3+ and thus is very hard to support. Are you able to run it if you force Vulkan? (set |
cool, just tested with vulkan and CiVct works |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🦟 Bug fix
Fixes #435
Summary
This PR adds Real Time Global Illumination based on one of these two techniques:
VCT can be more accurate but likes static objects (static objects can bounce GI, while dynamic objects can only receive).
CI VCT can be less accurate, but it is much faster dealing with objects and deal with very large scenes (and adapt more easily to any of them) and even though it prefers static objects, you can force all objects to bounce GI and it may just work. CI VCT is still under development on the Ogre-Next side because real time global illumination is still an unsolved problem being actively (and heavily) researched on.
This PR needs Ogre 2.3 PR #553 to be merged first. At the moment Github is showing the changes from 2.3 PR + VCT PR which makes it look huge.
A sample showing how to use both techniques has been added.
New features
Visual::SetVisualStatic_d(debug Ogre plugins) in a DEBUG build configuration.DEBUGmacro in a debug buildChecklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.