Fix #146223: Cycles: Wrong output with multi-device OSL rendering

We used to set shader->osl_surface_ref during shader compilation and then
pushed it into the shared vectors.

This worked as long as everything was serial - but after the multithreading
change, we a) compile everything and then b) build the shared vectors since
just pushing into them from multiple threads would not work.

However, if there are multiple devices, then each shader will be compiled
multiple times - so in the end, shader->osl_surface_ref etc. will be set
to the last device's value. Then, we end up pushing that value into every
device's vectors, which breaks for the earler devices.

The fix is simple - just preallocate the vectors and pass the correct index
into the compilation function. This way, each thread can safely store its
result and we can get rid of shader->osl_surface_ref entirely.

Note that while multiple shaders are compiled in parallel, the loop over
devices for a given shader is serial, so there's no concern of conflicts
over other shader internals.

Pull Request: https://projects.blender.org/blender/blender/pulls/146617
This commit is contained in:
Lukas Stockner
2025-09-29 03:12:18 +02:00
parent 2bd06093c7
commit fc5c6ab374
3 changed files with 21 additions and 55 deletions

View File

@@ -661,22 +661,31 @@ void OSLShaderManager::device_update_specific(Device *device,
og->displacement_state.clear();
og->bump_state.clear();
og->background_state.reset();
/* Allocate space for the shader groups. Needs to be done here so the multithreaded
* compilation can write to it safely. */
og->surface_state.resize(scene->shaders.size());
og->volume_state.resize(scene->shaders.size());
og->displacement_state.resize(scene->shaders.size());
og->bump_state.resize(scene->shaders.size());
});
/* create shaders */
Shader *background_shader = scene->background->get_shader(scene);
/* compile each shader to OSL shader groups */
/* Compile each shader to OSL shader groups. */
TaskPool task_pool;
for (Shader *shader : scene->shaders) {
assert(shader->graph);
for (int shader_id = 0; shader_id < scene->shaders.size(); shader_id++) {
auto compile = [scene, shader_id, background_shader](Device *sub_device, OSLGlobals *og) {
Shader *shader = scene->shaders[shader_id];
assert(shader->graph);
auto compile = [scene, shader, background_shader](Device *sub_device, OSLGlobals *) {
OSL::ShadingSystem *ss = scene->osl_manager->get_shading_system(sub_device);
OSLCompiler compiler(ss, scene);
compiler.background = (shader == background_shader);
compiler.compile(shader);
compiler.compile(og, shader_id, shader);
};
task_pool.push([device, compile] { OSLManager::foreach_osl_device(device, compile); });
@@ -687,21 +696,8 @@ void OSLShaderManager::device_update_specific(Device *device,
return;
}
/* collect shader groups from all shaders */
/* Tag updates after shader compilation is done. */
for (Shader *shader : scene->shaders) {
OSLManager::OSLManager::foreach_osl_device(
device, [shader, background_shader](Device *, OSLGlobals *og) {
/* push state to array for lookup */
og->surface_state.push_back(shader->osl_surface_ref);
og->volume_state.push_back(shader->osl_volume_ref);
og->displacement_state.push_back(shader->osl_displacement_ref);
og->bump_state.push_back(shader->osl_surface_bump_ref);
if (shader == background_shader) {
og->background_state = shader->osl_surface_ref;
}
});
if (shader->emission_sampling != EMISSION_SAMPLING_NONE) {
scene->light_manager->tag_update(scene, LightManager::SHADER_COMPILED);
}
@@ -1540,7 +1536,7 @@ OSL::ShaderGroupRef OSLCompiler::compile_type(Shader *shader, ShaderGraph *graph
return std::move(current_group);
}
void OSLCompiler::compile(Shader *shader)
void OSLCompiler::compile(OSLGlobals *og, int shader_id, Shader *shader)
{
if (shader->is_modified()) {
ShaderGraph *graph = shader->graph.get();
@@ -1550,34 +1546,21 @@ void OSLCompiler::compile(Shader *shader)
/* generate surface shader */
if (shader->reference_count() && shader->has_surface) {
shader->osl_surface_ref = compile_type(shader, graph, SHADER_TYPE_SURFACE);
og->surface_state[shader_id] = compile_type(shader, graph, SHADER_TYPE_SURFACE);
if (has_bump) {
shader->osl_surface_bump_ref = compile_type(shader, graph, SHADER_TYPE_BUMP);
og->bump_state[shader_id] = compile_type(shader, graph, SHADER_TYPE_BUMP);
}
else {
shader->osl_surface_bump_ref = OSL::ShaderGroupRef();
}
}
else {
shader->osl_surface_ref = OSL::ShaderGroupRef();
shader->osl_surface_bump_ref = OSL::ShaderGroupRef();
}
/* generate volume shader */
if (shader->reference_count() && shader->has_volume) {
shader->osl_volume_ref = compile_type(shader, graph, SHADER_TYPE_VOLUME);
}
else {
shader->osl_volume_ref = OSL::ShaderGroupRef();
og->volume_state[shader_id] = compile_type(shader, graph, SHADER_TYPE_VOLUME);
}
/* generate displacement shader */
if (shader->reference_count() && shader->has_displacement) {
shader->osl_displacement_ref = compile_type(shader, graph, SHADER_TYPE_DISPLACEMENT);
}
else {
shader->osl_displacement_ref = OSL::ShaderGroupRef();
og->displacement_state[shader_id] = compile_type(shader, graph, SHADER_TYPE_DISPLACEMENT);
}
/* Estimate emission for MIS. */

View File

@@ -153,7 +153,7 @@ class OSLCompiler {
#ifdef WITH_OSL
OSLCompiler(OSL::ShadingSystem *ss, Scene *scene);
#endif
void compile(Shader *shader);
void compile(OSLGlobals *og, int shader_id, Shader *shader);
void add(ShaderNode *node, const char *name, bool isfilepath = false);

View File

@@ -4,15 +4,6 @@
#pragma once
#ifdef WITH_OSL
# include <cstdint> /* Needed before `sdlexec.h` for `int32_t` with GCC 15.1. */
/* So no context pollution happens from indirectly included windows.h */
# ifdef _WIN32
# include "util/windows.h"
# endif
# include <OSL/oslexec.h>
#endif
#include "kernel/types.h"
#include "scene/attribute.h"
@@ -128,14 +119,6 @@ class Shader : public Node {
/* determined before compiling */
uint id;
#ifdef WITH_OSL
/* osl shading state references */
OSL::ShaderGroupRef osl_surface_ref;
OSL::ShaderGroupRef osl_surface_bump_ref;
OSL::ShaderGroupRef osl_volume_ref;
OSL::ShaderGroupRef osl_displacement_ref;
#endif
Shader();
/* Estimate emission of this shader based on the shader graph. This works only in very simple