fix: reparameter string needed to be ustringhash (#1841)

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>
This commit is contained in:
Chris Hellmuth
2024-07-09 08:52:12 -07:00
committed by GitHub
parent 27a8bdebca
commit 7f3ff8ce38
7 changed files with 48 additions and 5 deletions
+1 -1
View File
@@ -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
+7 -1
View File
@@ -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);
+13 -3
View File
@@ -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;
}
View File
+4
View File
@@ -0,0 +1,4 @@
Compiled test.osl -> test.oso
test_string = initial value
test_string = updated value
+15
View File
@@ -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
View File
@@ -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);
}