[WIP] Octree View Frustum Culling Example by Squareys · Pull Request #27 · mosra/magnum-examples

Squareys · 2016-09-16T20:25:53Z

Finally! The octree implementation, @ACapo and I started working on last year, is working! There is merely some cleanup left, especially on the MagnumExtras side, but I'm putting this out there anyway, just in case someone is in desperate need of Octree View Frustum Culling.

Basically, this should give you an idea what the API looks like at the moment.

This is in no way urgent, I know you're insanely busy for the next two weeks.

Regards, Jonathan.

Squareys · 2016-09-16T20:40:03Z

TODOs:

Change License according to new license
Screenshot and Readme
Explanatory comments
General code cleanup

mosra

Roughly went over the changes, hopefully not complaining too much about code that's meant to be still reworked :)

Can you also update the doc/building.dox documentation page mentioning this example?

mosra · 2016-09-26T09:26:55Z

 option(WITH_BULLET_EXAMPLE "Build Bullet integration example (requires BulletIntegration library)" OFF)
 cmake_dependent_option(WITH_CUBEMAP_EXAMPLE "Build CubeMap example (requires JpegImporter plugin)" OFF "NOT MAGNUM_TARGET_GLES" OFF)
 cmake_dependent_option(WITH_MOTIONBLUR_EXAMPLE "Build MotionBlur example" OFF "NOT MAGNUM_TARGET_GLES" OFF)
+option(WITH_OCTREE_EXAMPLE "Build Octree: example" OFF)


the : is superfluous

mosra · 2016-09-26T09:34:34Z

+    MagnumExtras::SceneGraph
+    Magnum::Magnum)
+
+target_compile_features(magnum-octree PRIVATE cxx_range_for)


Not needed (we expect C++11 conforming compiler).

Interestingly, I put that there, because the c++11 flag was not set for my compiler.

mosra · 2016-09-26T09:35:43Z

+    SceneGraph)
+
+
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CORRADE_CXX_FLAGS}")


Please replace with

set_directory_properties(PROPERTIES CORRADE_USE_PEDANTIC_FLAGS ON)

Also please no more than one successive empty line ;)

mosra · 2016-09-26T09:37:53Z

+    Magnum::Application
+    MagnumExtras::Octree
+    MagnumExtras::SceneGraph
+    Magnum::Magnum)


Can you order the libraries so all Magnum:: are together? If the link order matters, then there is a problem in the FindMagnumExtras.cmake module (each imported target should have its dependencies implicitly and not expect the user to link the dependencies explicitly).

mosra · 2016-09-26T09:40:59Z

+const Color3 PINK{1.0f, 0.0f, 1.0f};
+const Color3 WHITE{1.0f, 1.0f, 1.0f};
+const Color3 LIGHT_GREY{0.5f, 0.5f, 0.5f};
+const Color3 GREEN{0.0f, 0.2f, 0.0f};


Use constexpr if possible and please no UPPERCASE -- these are not macros. Also, maybe it would look better with hex colors? Roughly (not the same colors):

constexpr Color3 Pink = 0xff00ff_rgbf; constexpr Color3 White = 0xffffff_rgbf; constexpr Color3 LightGrey = 0x999999_rgbf; constexpr Color3 Green = 0x003300_rgbf;

mosra · 2016-09-26T09:43:18Z

+    const Vector3 rbf = calculateIntersection(right, bottom, far);
+    const Vector3 lbf = calculateIntersection(left, bottom, far);
+    const Vector3 rtf = calculateIntersection(right, top, far);
+    const Vector3 ltf = calculateIntersection(left, top, far);


I think I saw a very similarly looking code in the extras repo. So again I would suggest making a function / Frustum class out of this.

mosra · 2016-09-26T09:47:24Z

+    const float RAND_MAX_RANGE = float(RAND_MAX/60);
+    for(int i = 0; i < 10000; ++i) {
+        const Vector3 randomVector{float(std::rand()), float(std::rand()), float(std::rand())};
+        const Vector3 min = randomVector/RAND_MAX_RANGE - Vector3{30.0f};


std::rand() is deprecated, please use the C++11 functionality instead. It also has the ability to specify an output range, so you can get rid of the constants (and they shouldn't be uppercase).

mosra · 2016-09-26T09:58:50Z

+
+    const Float x = Matrix3({v1.w(), v2.w(), v3.w()}, {v1.y(), v2.y(), v3.y()}, {v1.z(), v2.z(), v3.z()}).determinant() / det;
+    const Float y = Matrix3({v1.x(), v2.x(), v3.x()}, {v1.w(), v2.w(), v3.w()}, {v1.z(), v2.z(), v3.z()}).determinant() / det;
+    const Float z = Matrix3({v1.x(), v2.x(), v3.x()}, {v1.y(), v2.y(), v3.y()}, {v1.w(), v2.w(), v3.w()}).determinant() / det;


Hmm. I would use Matrix3x3 instead of Matrix3. The difference is that the latter is meant for transformations in 2D (it has all the translation()/scaling() etc. helpers), whether the former is just a general-purpose 3x3 matrix.

And now the code compression fun:

const Float x = Matrix3x3{Math::swizzle<'w', 'y', 'z'>(v1), Math::swizzle<'w', 'y', 'z'>(v2), Math::swizzle<'w', 'y', 'z'>(v3)}.determinant()/det; const Float y = Matrix3x3{Math::swizzle<'x', 'w', 'z'>(v1), Math::swizzle<'x', 'w', 'z'>(v2), Math::swizzle<'x', 'w', 'z'>(v3)}.determinant()/det; const Float z = Matrix3x3{Math::swizzle<'x', 'y', 'w'>(v1), Math::swizzle<'x', 'y', 'w'>(v2), Math::swizzle<'x', 'y', 'w'>(v3)}.determinant()/det;

Determinant of transposed matrix is equal to determinant of original matrix, so it should give the same output. There's still quite a lot of repeated code, but at least there are less places of potential error.

mosra · 2016-09-26T10:00:18Z

@@ -0,0 +1,2 @@
+This example shows how to use octree view frustum culling in magnum.
+![Octree](octree.png)


Please use Magnum instead of magnum in docs.

Also it would be great if this had the key bindings explained.

mosra · 2016-09-26T10:03:21Z

Oh, and also enabling the example in all CIs, please :)

Signed-off-by: Squareys <Squareys@googlemail.com>

still shows some unexpected behaviour under certain circumstances.

Signed-off-by: Squareys <Squareys@googlemail.com>

Signed-off-by: Squareys <squareys@googlemail.com>

mosra · 2020-06-16T20:14:29Z

@Squareys I'd say this is all obsoleted by #86?

Squareys · 2020-06-17T06:35:14Z

@mosra I guess the difference is that this demos the "Official Octress" that's beeing PRd in magnum-extras. (mosra/magnum-extras#1).
Also frustum culling vs general octree is still a difference, so I think this could still have it's own viability should the octree ever make it in.
I think the state of that was that I did some crazy refactor to a. make it dimension-independent and b. faster.

mosra · 2020-06-17T11:38:53Z

I commented because #86 has additional features the implementation in extras doesn't seem to have (loose octree, node allocation etc.) -- but OTOH the implementation in extras has tests, so neither of them is really a winner.

The real question is if the implementation in extras will ever get finished, I guess :) Judging from our priorities lately, I'm not very optimistic :D

Squareys · 2020-06-17T12:54:38Z

in extras will ever get finished

Probably not too soon, no... but on here there are still some unmined gems like getting Frustum corners, intersecting three planes etc. We should save those somehow. (I have some refinements through the Clustered Forward+ stuff, though)

mosra requested changes Sep 26, 2016

View reviewed changes

Squareys and others added 10 commits April 16, 2018 07:59

octree: Initial skeleton for future octree example.

f5b5a7e

Signed-off-by: Squareys <Squareys@googlemail.com>

octree: Implement octree example.

566cf33

still shows some unexpected behaviour under certain circumstances.

Octree: Fix octree example and add S key to switch views

7fe83da

Signed-off-by: Squareys <Squareys@googlemail.com>

octree: Update cmake project

d388931

Signed-off-by: Squareys <squareys@googlemail.com>

octree: Use meshes instead of ShapeRenderer

0dba717

Signed-off-by: Squareys <squareys@googlemail.com>

octree: Cleanup and replace AABB with Range3D.

180161e

Signed-off-by: Squareys <squareys@googlemail.com>

octree: Add toggleable visualization for octree.

74d89b6

Signed-off-by: Squareys <squareys@googlemail.com>

octree: Exit on Esc

2721b95

Signed-off-by: Squareys <squareys@googlemail.com>

octree: Only draw culled drawables when not in external view.

e218d2a

Signed-off-by: Squareys <squareys@googlemail.com>

octree: Adapt to magnum changes

b991446

Squareys force-pushed the octree-example branch from 6ced55e to b991446 Compare April 16, 2018 06:27

		SceneGraph)


		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CORRADE_CXX_FLAGS}")

		@@ -0,0 +1,2 @@
		This example shows how to use octree view frustum culling in magnum.
		![Octree](octree.png)

Conversation

Squareys commented Sep 16, 2016

Uh oh!

Squareys commented Sep 16, 2016

Uh oh!

mosra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mosra commented Sep 26, 2016

Uh oh!

mosra commented Jun 16, 2020

Uh oh!

Squareys commented Jun 17, 2020

Uh oh!

mosra commented Jun 17, 2020

Uh oh!

Squareys commented Jun 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants