Skip to content

Commit

Permalink
fix: reparameter string needed to be ustringhash (#1841)
Browse files Browse the repository at this point in the history
This patch fixes a regression that went unnoticed during the ustringhash conversion.

Right now OSL expects strings to be represented at runtime as ustringhashes, but interactive parameters' values are still being stored as ustrings. This requires fixing in two locations, first the code that sets the initial value of an interactive parameter, and second the code that copies updated parameters into the interactive buffer.

Tests: New reparameter-strings test.

Signed-off-by: Chris Hellmuth <chellmuth@gmail.com>
  • Loading branch information
chellmuth committed Jul 9, 2024
1 parent 27a8bde commit 7f3ff8c
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/cmake/testing.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ macro (osl_add_all_tests)
printf-reg
printf-whole-array
raytype raytype-reg raytype-specialized regex-reg
reparam reparam-arrays testoptix-reparam
reparam reparam-arrays reparam-string testoptix-reparam
render-background render-bumptest
render-cornell render-furnace-diffuse
render-mx-furnace-burley-diffuse
Expand Down
8 changes: 7 additions & 1 deletion src/liboslexec/runtimeoptimize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3290,7 +3290,13 @@ RuntimeOptimizer::run()
offset = OIIO::round_to_multiple_of_pow2(offset, alignment);
interactive_data.resize(offset + totalsize);
// Copy from the instance value to the interactive block
memcpy(&interactive_data[offset], s.data(), typesize);
// If the value is a string, copy its hash.
if (s.typespec().is_string()) {
ustring string_data = *reinterpret_cast<ustring*>(s.data());
ustringhash string_hash(string_data);
memcpy(&interactive_data[offset], &string_hash, typesize);
} else
memcpy(&interactive_data[offset], s.data(), typesize);
if (totalsize > typesize)
memset(&interactive_data[offset] + typesize, 0,
totalsize - typesize);
Expand Down
16 changes: 13 additions & 3 deletions src/liboslexec/shadingsys.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3398,12 +3398,22 @@ ShadingSystemImpl::ReParameter(ShaderGroup& group, string_view layername_,
size_t size = type.size();
m_stat_reparam_calls_total += 1;
m_stat_reparam_bytes_total += size;
if (memcmp(group.interactive_arena_ptr() + offset, val, size)) {
memcpy(group.interactive_arena_ptr() + offset, val, type.size());

// Copy ustringhashes instead of ustrings
const void* payload;
ustringhash string_hash;
if (type == TypeDesc::STRING) {
string_hash = ustringhash_from(*reinterpret_cast<const ustring*>(val));
payload = &string_hash;
} else
payload = val;

if (memcmp(group.interactive_arena_ptr() + offset, payload, size)) {
memcpy(group.interactive_arena_ptr() + offset, payload, type.size());
if (use_optix())
renderer()->copy_to_device(group.device_interactive_arena().d_get()
+ offset,
val, type.size());
payload, type.size());
m_stat_reparam_calls_changed += 1;
m_stat_reparam_bytes_changed += size;
}
Expand Down
Empty file added testsuite/reparam-string/OPTIX
Empty file.
4 changes: 4 additions & 0 deletions testsuite/reparam-string/ref/out.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Compiled test.osl -> test.oso
test_string = initial value
test_string = updated value

15 changes: 15 additions & 0 deletions testsuite/reparam-string/run.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env python

# Copyright Contributors to the Open Shading Language project.
# SPDX-License-Identifier: BSD-3-Clause
# https://github.com/AcademySoftwareFoundation/OpenShadingLanguage

command += testshade(" ".join([
"--layer lay0",
"--param:type=string:interactive=1 test_string 'initial value'",
"test --iters 2",
"--reparam:type=string:interactive=1 lay0 test_string 'updated value'",
]))

outputs = [ "out.txt" ]

8 changes: 8 additions & 0 deletions testsuite/reparam-string/test.osl
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Copyright Contributors to the Open Shading Language project.
// SPDX-License-Identifier: BSD-3-Clause
// https://github.com/AcademySoftwareFoundation/OpenShadingLanguage

shader test(string test_string = "override me" [[ int interactive = 1 ]])
{
printf("test_string = %s\n", test_string);
}

0 comments on commit 7f3ff8c

Please sign in to comment.