blender
/
blender
136
406
Fork 589
Code Issues 6.3k Pull Requests 766 Projects 19 Wiki Activity

Fix Light Tree option causing wrong renders on macOS AMD #105022

Merged
Sergey Sharybin merged 3 commits from Sergey/blender:cycles_fix_light_tree_apple_amd into blender-v3.5-release 2023-02-21 16:50:20 +01:00
Conversation 2 Commits 3 Files Changed 3 +9 -5
3 changed files with 9 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
intern/cycles/blender/sync.cpp
View File

@ -349,8 +349,7 @@ void BlenderSync::sync_integrator(BL::ViewLayer &b_view_layer, bool background)
bool use_light_tree = get_boolean(cscene, "use_light_tree");
integrator->set_use_light_tree(use_light_tree);
integrator->set_light_sampling_threshold(
(use_light_tree) ? 0.0f : get_float(cscene, "light_sampling_threshold"));
integrator->set_light_sampling_threshold(get_float(cscene, "light_sampling_threshold"));
if (integrator->use_light_tree_is_modified()) {
scene->light_manager->tag_update(scene, LightManager::UPDATE_ALL);

5
intern/cycles/kernel/light/light.h
View File

@ -113,13 +113,16 @@ ccl_device_noinline bool light_sample(KernelGlobals kg,
{
int prim;
MeshLight mesh_light;
#ifdef __LIGHT_TREE__
if (kernel_data.integrator.use_light_tree) {
ccl_global const KernelLightTreeEmitter *kemitter = &kernel_data_fetch(light_tree_emitters,
emitter_index);
prim = kemitter->prim;
mesh_light = kemitter->mesh_light;
}
else {
else
#endif
{
ccl_global const KernelLightDistribution *kdistribution = &kernel_data_fetch(
light_distribution, emitter_index);
prim = kdistribution->prim;

6
intern/cycles/scene/integrator.cpp
View File

@ -255,8 +255,10 @@ void Integrator::device_update(Device *device, DeviceScene *dscene, Scene *scene
kintegrator->scrambling_distance = scrambling_distance;
kintegrator->sobol_index_mask = reverse_integer_bits(next_power_of_two(aa_samples - 1) - 1);
kintegrator->use_light_tree = scene->integrator->use_light_tree;
if (light_sampling_threshold > 0.0f) {
/* NOTE: The kintegrator->use_light_tree is assigned to the efficient value in the light manager,
Weizhen Huang commented 2023-02-21 15:14:19 +01:00
Review
Copy Link

I think this line can be deleted.

I think this line can be deleted.
Sergey Sharybin commented 2023-02-21 15:38:48 +01:00
Review
Copy Link

The previous logic was focing the light_sampling_threshold to be 0 if when using the light tree. If we remove the kintegrator->use_light_tree = ... from here we would not be able to know if the light trees are actually used or not (the light manager is updated after the integrator).

There is possibility of adding checks in the kernel itself around access light_inv_rr_threshold but those are quite few places to cover.

Another possibility is to remove kintegrator->use_light_tree = ... from the light.cpp. Would make more sense to update integrator from the integrator.cpp. Seems safe, but not sure its safe enough for 3.5.

The previous logic was focing the `light_sampling_threshold` to be 0 if when using the light tree. If we remove the `kintegrator->use_light_tree = ...` from here we would not be able to know if the light trees are actually used or not (the light manager is updated after the integrator). There is possibility of adding checks in the kernel itself around access `light_inv_rr_threshold` but those are quite few places to cover. Another possibility is to remove `kintegrator->use_light_tree = ...` from the `light.cpp`. Would make more sense to update integrator from the `integrator.cpp`. Seems safe, but not sure its safe enough for 3.5.
Weizhen Huang commented 2023-02-21 15:49:36 +01:00
Review
Copy Link

But according to scene.cpp the light manager is updated before the integrator?
If light manager is updated after the integrator I agree it would be better to remove the other one. But yes if this goes into 3.5 it might be safer to keep the duplication for now.

But according to `scene.cpp` the light manager is updated before the integrator? If light manager is updated after the integrator I agree it would be better to remove the other one. But yes if this goes into 3.5 it might be safer to keep the duplication for now.
Sergey Sharybin commented 2023-02-21 16:12:05 +01:00
Review
Copy Link

Ah, you're right. I've misread integrator->tag_modified(); in almost the beginning as something else.

Ah, you're right. I've misread `integrator->tag_modified();` in almost the beginning as something else.
Weizhen Huang commented 2023-02-21 16:18:29 +01:00
Review
Copy Link

Typo in the comment :DD

Typo in the comment :DD
Sergey Sharybin commented 2023-02-21 16:19:36 +01:00
Review
Copy Link

Fixed a typo. Not sure if there is anything else. The VS Code spell checker does not show anything ;)

Fixed a typo. Not sure if there is anything else. The VS Code spell checker does not show anything ;)
* and the synchronization code is expected to tag the light manager for update when the
* `use_light_tree` is changed. */
if (light_sampling_threshold > 0.0f && !kintegrator->use_light_tree) {
kintegrator->light_inv_rr_threshold = scene->film->get_exposure() / light_sampling_threshold;
}
else {