Light tree #1505

gospodnetic · 2017-07-05T14:03:20Z

Initial integration of the LightTree sampling. The tree can handle only point source lights. The point lights are flagged as LightTreeCompatible and, as such, extracted into a separate vector within the LightSampler

Since we know the lights within the LightTree can only be non physical for now, contributions from the LightTree within the DirectLightingIntegrator are added using the add_non_physical_light_sample_contribution(). It is something that works now but should be changed/expanded once I add other types of lights to the tree. I wouldn't change it now because I am not sure yet what and how should I change. I can duplicate the add_non_physical_light_sample_contribution() function and rename it into add_light_tree_sample_contribution() just to mark the spot that will be expanded.

…heap

Compare the retrieved position and calculated center of the bbox for both NPL and EM.

…ain.cpp`

…ring in leaves.

…4 non-physical lights)

dictoon · 2017-07-06T14:17:09Z

src/appleseed/renderer/kernel/lighting/lighttree.cpp

@@ -285,8 +285,8 @@ namespace
 {
    float node_probability(
            const LightTreeNode<foundation::AABB3d>&    node,
-            const foundation::AABB3d                    bbox,
-            const foundation::Vector3d                  surface_point)
+            const foundation::AABB3d&                   bbox,


Fix indentation (1 indentation = 4 spaces only).

dictoon · 2017-07-06T14:18:30Z

src/appleseed/renderer/kernel/lighting/lighttree.cpp

-    const foundation::Vector3d    surface_point,
-    float                         s) const
+    const foundation::Vector3d&     surface_point,
+    float                           s) const


One minor thing I don't think I mentioned: whenver we indent stuff, we try to stick to tab "stops", i.e. multiples of 4 spaces.

The spacing between the first argument and it's type should be multiple of 4 or the character column? (I tend to stick to column).

dictoon · 2017-07-06T14:21:13Z

src/appleseed/renderer/modeling/light/light.h

    // Retrieve the importance multiplier.
    float get_uncached_importance_multiplier() const;

    // Set the light transformation.
    void set_transform(const foundation::Transformd& transform);

+    // Set the flag that the light is LightTreeCompatible
+    bool is_light_tree_compatible() const;


I respectfully disagree, I think it somewhat clutters the renderer::Light interface by giving this flag a predominant place and by duplicating what can already be accomplished by checking the flags.

I think that

if (m_flags & LightTreeCompatible)`

is fairly readable, don't you think?

dictoon · 2017-07-06T15:00:42Z

src/appleseed/renderer/modeling/light/light.cpp

@@ -118,9 +113,13 @@ bool Light::on_frame_begin(
    if (!ConnectableEntity::on_frame_begin(project, parent, recorder, abort_switch))
        return false;

-    m_flags = 0;
-    if (is_light_tree_compatible())
+    if (m_flags & LightTreeCompatible)


Ah, that's another approach indeed. The downside is that it won't work if we add another flag.

What I proposed is different, see the description on Slack.

Fixed as proposed

…o both white and colored lights

… not available publicly.

… lights

dictoon

Second round of reviews. Almost there!!

dictoon · 2017-07-06T20:42:02Z

sandbox/samples/python/basic/4_point_lights_test_scene.py

+import os
+
+import appleseed as asr
+


Remove redundant blank line.

dictoon · 2017-07-06T20:44:04Z

sandbox/samples/python/basic/grid_point_lights.py

+import numpy as np
+
+import appleseed as asr
+


Remove redundant blank line.

dictoon · 2017-07-06T20:44:10Z

sandbox/samples/python/basic/grid_point_lights.py

+output_scene_name = "output/" + str(grid_lights_count) + "x" + str(grid_lights_count) + "_" + color + "_point_lights"
+
+def build_project():
+


Remove redundant blank line.

dictoon · 2017-07-06T20:47:37Z

sandbox/samples/python/basic/grid_point_lights.py

+                light.set_transform(asr.Transformd(mat))
+                assembly.lights().insert(light)
+    else:
+        print "Unknown color: ", color


It's best to use Python 3's print syntax.

Add:

from __future__ import print_function

and then:

print("Unknown color: " + color)

or:

print("Unknown color: {0}".format(color))

dictoon · 2017-07-06T20:48:17Z

sandbox/samples/python/basic/grid_point_lights.py

+                                                                'intensity_multiplier': "3"
+
+                                                             })
+                light_position = asr.Vector3d( i, j, 0.2 )


Like in C++, no spaces before or after parentheses:

light_position = asr.Vector3d(i, j, 0.2)

dictoon · 2017-07-06T21:28:16Z

src/appleseed/renderer/kernel/lighting/lighttypes.h

+{
+  public:
+    const AssemblyInstance*     m_assembly_instance;
+    size_t                      m_object_instance_index;


Include <cstddef>.

dictoon · 2017-07-06T21:28:27Z

src/appleseed/renderer/kernel/lighting/lighttypes.h

+    size_t                      m_object_instance_index;
+    size_t                      m_region_index;
+    size_t                      m_triangle_index;
+    foundation::Vector3d        m_v0, m_v1, m_v2;               // world space vertices of the triangle


Include foundation/math/vector.h.

dictoon · 2017-07-06T21:29:01Z

src/appleseed/renderer/kernel/lighting/lighttypes.h

+//
+
+class LightSource
+  : public foundation::NonCopyable


Include foundation/core/concepts/noncopyable.h.

dictoon · 2017-07-06T21:29:20Z

src/appleseed/renderer/kernel/lighting/lighttypes.h

+    // Destructor
+    virtual ~LightSource();
+
+    // Get the reference to the source position.


// Get the light source position.

dictoon · 2017-07-06T21:30:18Z

src/appleseed/renderer/modeling/light/pointlight.cpp

@@ -77,6 +77,9 @@ namespace
            m_inputs.declare("intensity", InputFormatSpectralIlluminance);
            m_inputs.declare("intensity_multiplier", InputFormatFloat, "1.0");
            m_inputs.declare("exposure", InputFormatFloat, "0.0");
+
+            // Point lights can be used by the LightTree


End full line comment with a .

…rch path

gospodnetic · 2017-07-07T10:09:30Z

Updated with the requested changes :)

gospodnetic added 30 commits May 29, 2017 15:54

Initial set of code

457c1c1

Use appleseed foreach, start building Item

54a557b

Prepare simple LightSource base class

3d65ee2

Structure holding pointers to LightSource

d5f87c3

Use heap allocation for storing found lights

3c7d662

Use heap allocation for adding EmittingTriangleLightSource as well

4bb240c

Test fetching light data

8402f1f

Make LightSource destructor virtual and delete lights created on the …

ee59568

…heap

Get non physical light position

f1063ad

Return the position as a centroid of the emmiting triangle

3a76952

Add comments

b47f80a

Make it easier to follow

0544fcd

Get bbox for each lightsource

05af796

Prepare use of tree items

047d6a3

Add bbox to item

07fdadf

Create the AABB using invalidation and vertex insertion.

14f9032

Edit NPL bbox fixed values to be formed relative to the light position.

9a5aac5

Compare the retrieved position and calculated center of the bbox for both NPL and EM.

Add SAH partitioner and builder

9e30aea

Temporary fix for changed locale and output to test file

f68896b

Locale bug tracked down and proposed correction with hardcoding in `m…

aced2a8

…ain.cpp`

Filling tree with lights - Partitioning, building, reordering and sto…

4d63021

…ring in leaves.

Modified metalbrdf scene with 8 light sources (4 emitting triangles, …

1918d43

…4 non-physical lights)

Switch to MedianPartitioner

47bb569

Added a test scene containing only point lights

c1d10a0

Add output to file in order to track the BBOX sorting algorithm

e7aafc2

MiddlePartititioner implementation

03eb31d

Remove output of sorting values to file

338abf7

get_intensity() placeholder

c29dc98

Retrieve actual intensity of the NPL light (works for point lights)

c522d30

Change type to predefined Spectrum type

dc14544

dictoon reviewed Jul 6, 2017

View reviewed changes

gospodnetic added 2 commits July 6, 2017 16:45

The method for ight-tree compatibility check is redundant.

0c7e5e3

Cosmetics.

e556a54

dictoon reviewed Jul 6, 2017

View reviewed changes

gospodnetic added 7 commits July 6, 2017 17:13

Add switch between white lights and random colored lights.

a18bd8d

Reset only CastIndirectLight flag.

cf5a6f1

Copyright. grid white lights file is redundant as it is generalized t…

d2009c5

…o both white and colored lights

Light flags should be accessible to the derived classes directly, but…

4669ef3

… not available publicly.

Light to plane distance is a common variable for both white and mixed…

ed1237b

… lights

Use three color peaks and choose them randomly.

ae2c57d

Remove skip middle position. Unnecessary.

323951a

dictoon reviewed Jul 6, 2017

View reviewed changes

gospodnetic added 12 commits July 7, 2017 10:34

Move scene generator files to the test directory and use relative sea…

2505563

…rch path

Python generator cosmetics.

60c4c5c

Remove redundant namespace qualifiers.

3d09d4e

Spell constants in CamelCase

8d58427

Divide the tree probability by number of possible candidates as well.

3425335

Use format() for string handling.y

80e42b9

Try to explain better.

b996f2c

Fix comments.

38198da

Use the internal averaging function.

6faa98c

Remove redundant forward declarations.

a6715b7

Remove intermediary vars.

f939335

Missing includes and comments.

afb6832

gospodnetic added 3 commits July 7, 2017 12:25

Code duplification.

23c6e80

Pass by reference.

fc918f3

Use the type instead of auto for now.

483e462

dictoon approved these changes Jul 7, 2017

View reviewed changes

gospodnetic closed this Jul 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Light tree #1505

Light tree #1505

gospodnetic commented Jul 5, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

gospodnetic Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

gospodnetic Jul 6, 2017

dictoon left a comment

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

dictoon Jul 6, 2017

gospodnetic commented Jul 7, 2017

		output_scene_name = "output/" + str(grid_lights_count) + "x" + str(grid_lights_count) + "_" + color + "_point_lights"

		def build_project():

Light tree #1505

Light tree #1505

Conversation

gospodnetic commented Jul 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dictoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gospodnetic commented Jul 7, 2017