Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSL SdrShaderNode.GetInputNames returning combined results from rmanOslParser #1396

Closed
pstuart42 opened this issue Nov 16, 2020 · 3 comments
Closed

Comments

@pstuart42
Copy link

Description of Issue

Hi, there, I ran into a strange issue with the OSL shader node parsing (rmanOslParser). Basically, reading SdrShaderNodes out of the registry causes the input names of the previously loaded nodes to be added to the list.

For example, given that shader1 has an input parm1, and shader2 has an input parm2:

reg = Sdr.Registry()
shader1 = reg.GetShaderNodeByIdentifier("oslshader1")
print(shader1.GetInputNames())
shader2 = reg.GetShaderNodeByIdentifier("oslshader2")
print(shader2.GetInputNames())

will result in:
['parm1']
['parm1', 'parm2']

Steps to Reproduce

I've attached an example (similar to what is in the description). Set RMAN_SHADERPATH to include the unzipped directory (or ".") so the oso files are found, then run the test_osl_parser.py script. It prints:

['Notes', 'Input']
['Notes', 'Input', 'Notes', 'Input']

but should be:

['Notes', 'Input']
['Notes', 'Input']

usd_sdr_bug.zip

This happens with the release branch and dev branch, as well as versions 20.05 and 20.08. It happens with prman 23.3 and 23.4.

I noticed that sdrOsl parser does not have this problem.

Peter.

@pstuart42
Copy link
Author

I did a quick inspection of the code, and noticed a couple suspicious things. The first is that the same RixShaderQuery instance is used with each shader queried. Each shader queried is "opened" but the API doesn't seem to be have a way to "close" it. The query seems to be hanging on to each shader's parameters between opens. I don't know if this is a RIX API bug, or if that is expected behavior. Anyway, recreating the query with each shader queried instead of re-using the query instance resolves the carry-over problem.

The second issue is that RixShaderQuery instance is never deleted, so there's a possible memory leak there. Maybe it isn't a big deal if the parser is only loaded once, but it's still a minor leak.

@spiffmon
Copy link
Member

Thanks for digging in, @pstuart42 - that gives us a great head-start!

@jtran56
Copy link

jtran56 commented Nov 24, 2020

Filed as internal issue #USD-6488

pixar-oss pushed a commit that referenced this issue Dec 24, 2020
Set RMAN_SHADERPATH to the location of the OSL shaders used
by the test. These are typically found automatically by the
rmanDiscovery plugin by looking relative to the hdPrmanLoader
plugin, which is installed in <inst>/plugin. However, in the
monolithic build this plugin is embedded in the single shared
library, which is installed in <inst>/lib. This breaks the
automatic discovery, so we need to hard-code the path to
the shaders instead.

Also remove unnecessary quotes from env setting; these quotes
were being passed all the way through to rmanDiscovery causing
incorrect search paths.

Fixes #1396

(Internal change: 2131436)
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

No branches or pull requests

4 participants