Skip to content

Conversation

chellmuth
Copy link
Collaborator

Description

This PR 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.

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format v17 before submitting, I definitely will look at
    the CI test that runs clang-format and fix anything that it highlights as
    being nonconforming.

Signed-off-by: Chris Hellmuth <chellmuth@gmail.com>
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM2!

@lgritz lgritz merged commit 7f3ff8c into AcademySoftwareFoundation:main Jul 9, 2024
chellmuth pushed a commit to chellmuth/OpenShadingLanguage that referenced this pull request Sep 6, 2024
* fix: reparameter string needed to be ustringhash (AcademySoftwareFoundation#1841)
* fix: make backfacing shadeop indicate backfacing shader-global is needed (AcademySoftwareFoundation#1827)
* Fix the subsurface_bssrdf parameters (AcademySoftwareFoundation#1823)
* Implement OpenPBR's sheen BRDF in testrender (AcademySoftwareFoundation#1819)
* cleanup: OIIO deprecations (AcademySoftwareFoundation#1834)

See merge request spi/dev/3rd-party/osl-feedstock!61
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants