Again for posterity, Brecht investigated the issue and discovered that I was overwriting the same entries of the triangle lookup table into the light tree. The latest commit should resolve this issue, but the solution did require some structural changes, so for anyone testing scenes, please let me know if anything working before is now broken.
3 Likes
There might still be a brightness issue at play. It’s almost as if the “Indirect Light” clamping value is not being respected.
Master with Indirect Light clamp = 10:
Many lights with Indirect Light clamp = 10:
Master with Indirect Light clamp = 0:
You can use the scene I provided from last time (make sure to enable LGT_Sun if it’s not already).
1 Like
It is expected for there to be intensity differences when clamping is used. If there is less noise due to improved sampling, less light will be clamped away.
In your example the many lights result has similar intensity as the unclamped result, and similar noise to the clamped result, which is what you’d want to see.
3 Likes
That’s what I thought, too.
No longer needing to clamp light to get noise free results with fewer samples is actually a good thing.
1 Like
Indeed, that’s good to know for comparisons and double-good for improved results.
3 Likes
I’m going to report some more limitations (and bugs?) I found in Many Lights Sampling.
I’m sorry @Jebbly. I know getting bug reports is good. But getting a list of multiple bug reports can be disheartening, which is why I’m sorry.
Spot Light Large Radius Importance Issue
When a spot light has a large radius, it gets given too much importance in some areas where it shouldn’t (the edges of the spot light).
Here is the .blend file:
Renders
Here is a render with three lights at 128 samples.
Many Lights Sampling Off:
Many Lights Sampling On:
Mesh Face Count Importance Issue
As the face count of a mesh light increases, the object ends up with an overal increase in importance (which it shouldn’t have).
Renders
Here are two renders of the same scene rendered at 16 samples per pixel with Many Lights Sampling On. The only difference between the two is the level of sub-division surface modifier on the cube (light emitting mesh)
Low face count (16spp - Many Lights Sampling On):
High face count (16spp - Many Lights Sampling On):
Lights With Textures Importance Issue
Lights with textures that drive their look and some complex node setups result in the light having the wrong importance in certain areas. I can understand how fixing this could be a complex task and could be outside the scope of this GSOC.
Here is the .blend file:
Renders
Here are two renders at 128 samples. There is a spot light in the middle with a texture driving it’s output.
Many Lights Sampling Off:
Many Lights Sampling On:
Area Lights Spread Importance Issue
The “Spread” parameter on area lights doesn’t appear to be taken into consideration when calculating the importance of a area light.
Here is a .blend file:
Renders
Here is a render at 16spp show casing the differences between Many Lights Sampling On vs Off. The spread on this area light is “1 degree” so it should only cast light directly in-front of it. However Many Lights Sampling appears to treat the area light as if it it a much larger “spread”.
Many Lights Sampling Off:
Many Lights Sampling On:
Two Different Brightness Lights Noise Issue
When two lights of very different brightnesses light up a overlaping area on a surface, the result is an increase in noise with Many Lights Sampling On. Maybe the bright light is given too much importance?
Here is the .blend file:
Renders
Here is a render at 16spp show casing the differences between Many Lights Sampling On vs Off.
Many Lights Sampling Off:
Many Lights Sampling On:
Ray Visibility Issue
With Many Lights Sampling On, lights will still be considered as “emitting light” and “important” even when they are not visible to that specific shader. For example, if I make it so a light is not visible to diffuse surfaces, when Cycles samples that diffuse surface, Many Lights Sampling acts as if the light IS visible to the diffuse surface and thus priorities rays towards it even though it shouldn’t.
Renders
Here are two renders at 16spp show casing the issue.
Many Lights Sampling Off:
Many Lights Sampling On:
Mesh lights behave differently compared to the point light from above and some times do work as expected. This is why I only disabled the Diffuse and Glossy ray visibility here.
Many Lights Sampling Off:
Many Lights Sampling On:
6 Likes
No worries, thanks again for the bug reports. Some of these issues seem relatively straightforward to debug. I have a few notes for each one:
- I have a general idea of what’s going on for the spotlight - I believe it has to do with the relationship between the bounding box and the emission profile. This one might be a little tricky to fix because it may be inherently related to the algorithm used in the paper.
- The mesh face count importance issue confirms that triangle area should be used in the energy heuristic. I’m hopeful that this will fix the issue. (RESOLVED)
- Textures are something that Brecht mentioned before. It will be a little tricky too, and we’ll have to figure out a heuristic for that end.
- The area light spread is an error on my part, I don’t think I’ve taken the area light spread into account for the orientation bounds. (RESOLVED*)
- The two different brightness issue is actually quite surprising to me, because given the intensity of the left light, I would’ve expected it to have a much larger impact myself. The heuristic being used may be weighting energy much more than it should be.
- The ray visibility issue seems to be the trickiest issue because there may be lights with the options enabled/disabled mixed together, which could skew the importances for some cases. The most straightforward solution I can think of is to create a tree for each visibility option, but I’ll have to explore this further.
The easier-ish solutions are things that’d I’d like to resolve in the coming week, so this does mean I’ll probably have to push back plans for volumes until later (I think this is a good thing because I’d like to understand the theory better as well). The trickier issues are things that I’ll have to discuss further with Brecht and Lukas at some point.
Update: I’m editing this post to mark which issues have been fixed.
* After resolving the first area light issue, it’s turning out to have a similar issue as the spot light with a large radius. I’m almost positive about what the issue is, but if it is indeed what I suspect, coming up with a solution may be difficult. My suspicion is that the importance heuristics are being too optimistic about what these lights will contribute, and the hard cutoff is messing it up. If this is the case, we may have to deviate from what the original paper proposes (perhaps they didn’t have these types of lights).
4 Likes
Just for refernece, this is the issue you’re talking about (the circle around the area light)?
The fix seems to be working. I tried “breaking it” with some different things and couldn’t break it. So that’s good!
1 Like
I also found a limitation. The “Point cloud” object type does not appear to be included in the light tree construction?
Here is a .blend file:
Renders
These renders are done at 16 samples per pixel.
Note: At higher sample counts they converge to effectively same result.
Point cloud lights - Many Lights Sampling On:
Mesh lights - Many Lights Sampling On: (I would expect the point cloud lights to render similarly to this)
It should be noted, that even without Many Lights Sampling, point cloud lights are already poorly sampled.
For reference, point cloud rendering was added to Cycles with this patch: ⚙ D9887 Cycles: pointcloud geometry type
Note: There was a note on here about “direct light sampling of emissive points”. Not sure if it’s useful? ⚙ D13249 Cycles Render PointCloud as Spheres
1 Like
Yes, I believe this is the same issue as the spot light with the large radius. In short, the importance heuristic is generally optimistic about how much a light source will contribute, which is backfiring in this case. That darker region is essentially the area that still approximates the spot/area light as an important contributor even though it’s actually cutoff.
Just as a note for this, there’s still some details I’d like to figure out for the other light sources (i.e. how much “area” should be assigned to each light type). There may be some discrepancies on that end until this is resolved.
2 Likes
By the way, just to follow up on this:
I don’t believe I made any specific fixes to resolve this issue, but the renders seem to be working for me locally. I haven’t made many commits since then, so I’m a bit surprised.
If you have the time, could you clone the latest version to see if this is resolved for you as well?
1 Like
I can still reproduce the issue. This time I tested with a Windows build rather than my usual Linux build.
I am using commit rBf9897d746242 which is from about a week ago as no new commits have been publicly published to the Many Lights Sampling branch since then.
Sorry, I’m not sure if I’m fully understanding. Could you clarify what the discrepancy is? My initial testing with the many lights sampling build seems to replicate the same behavior as that of Blender 3.2 Release. Here’s what I’m seeing:
I am seeing the exact same results as you. The issue I was talking about was how points (what you see when the node is muted) don’t appear to benefit from the Many Lights Sampling feature.
1 Like
Oh ok, I see what you’re saying now. I’ll try to investigate this and see how simple it will be to resolve.
UPDATE:
According to Brecht’s last comment on this diff, he mentions that emissive points are not directly sampled yet. I debugged the light distribution construction a little bit, and this seems to still be the case. As such, this may be out of the scope of the project for now.
That said, I’ll bring this up to him as well and see how we would like to approach this.
2 Likes
Edit 2:
Upon further thinking about this, I believe this may not be a bug, but instead it might be something I don’t fully understand about the adaptive splitting system implemented, along with the fact the adaptive splitting system is limited at the moment.
I decided to test the latest changes (rBca7e09f7fc2d). If I understand correctly, this behavior is not expected when the splitting threshold is set to 0. (I’m sorry, I don’t know how to properly describe the issue)
Here’s a .blend file:
Renders
This was at 4spp:
Note: The area light is really bright (1,000,000 W) while the point light is relatively dim (1,000 W)
And here’s an image where I point at the issue.
Edit:
Upon further investigation, I found this which might be useful in figuring out what’s going on.
1 Like
Hi,
Thanks for the report. Sorry I wasn’t very clear about the adaptive splitting.
To provide some more insight, this issue is actually expected but it certainly isn’t ideal. It’s one reason why the authors of the original paper were concerned about the usage of the splitting threshold.
The basic idea of splitting (i.e. checking the other side of the light tree) is that we do it when we’re not too confident about the importance heuristics. So we normalize a variance heuristic and see if it’s less than the splitting threshold. Here are some calculations that the past GSoC contributor made:
variance: 0 1 10 100 1000 10000 100000
normalized variance: 1 0.8 0.7 0.5 0.4 0.3 0.2
The specific values aren’t too important to consider. In the context of the first scene, the variance is high enough such that the normalized value is less than 0.85, so the algorithm starts checking the point light too. However, this is no longer the case when the threshold is at 0.00, so the algorithm goes back to only checking the area light at those darker regions.
The TLDR is that a splitting threshold of 0.00 is doing no better than the old implementation because this implies that the variance will never be high enough to split. We’ll either have to do some documentation, or fix the threshold, so that users don’t need to worry too much about this value.
1 Like
Thank you for making this comment. I think I understand things better.
One thing with regard to “documentation”. I think the tooltip for the splitting threshold should probably be changed as it can introduce some confusion. At the moment it’s:
Amount of lights to sample at a time, from one light at 0.0, to adaptively more lights as needed, to all lights at 1.0.
Based on what you’ve described, Cycles isn’t sampling multiple lights at the same time, instead it’s considering more lights “than usual”?
1 Like
Yes, you’re right about this. I believe I used the same comment as a previous GSoC contributor and things have changed since then. I’ll commit something as soon as possible to update it.
2 Likes
I just thought I’d let you know. I tried compiling the Many Lights Sampling branch on MacOS with a ARM CPU (Apple Silicon) and got these build errors.
In file included from /blender-git/blender/intern/cycles/kernel/device/cpu/kernel.cpp:48:
In file included from /blender-git/blender/intern/cycles/kernel/../kernel/device/cpu/kernel_arch_impl.h:29:
In file included from /blender-git/blender/intern/cycles/kernel/../kernel/integrator/shade_background.h:9:
**/blender-git/blender/intern/cycles/kernel/../kernel/light/light_tree.h:285:9:** **error:** **implicit conversion turns floating-point number into integer: 'float' to 'bool' [-Werror,-Wfloat-conversion]**
if (light_tree_should_split(kg, P, knode) &&
**^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~**
**/blender-git/blender/intern/cycles/kernel/../kernel/light/light_tree.h:565:11: note:** in instantiation of function template specialization 'ccl::light_tree_sample<false>' requested here
ret = light_tree_sample<false>(
**^**