Return the geomprop from mx_geompropvalue_filename

[Chang-Jin-Lee]

Fixes #3003.

mx_geompropvalue_filename never returned the fetched geomprop. getattribute writes into its destination on success and returns 0 only when the attribute is absent, so the assignment sat in the wrong branch:

string defaultFilename = defaultVal.filename;
if (getattribute(geomprop, defaultFilename) == 0)
    out.filename = defaultFilename;

On success nothing copied defaultFilename into out. On failure the assignment was a no-op, because out = defaultVal on the line above had already done it. Either way out.filename stayed at the default.

The reporter suggested == 1, which does work, since that first line covers the fallback. I went with the shape the ten sibling implementations in libraries/stdlib/genosl/ already use — every one tests failure and installs the default there, and the composite types copy the fetched value in an else:

// mx_geompropvalue_vector2.osl
if (getattribute(geomprop, value) == 0)
    out = defaultVal;
else { out.x = value[0]; out.y = value[1]; }

The local is renamed too, since defaultFilename holding a fetched value is arguably what made this easy to miss.

Testing

$ MaterialXTest "[genosl]"
All tests passed (2498 assertions in 5 test cases)

$ MaterialXTest
All tests passed (26198 assertions in 84 test cases)

I should be straight about the limit of that: I have no OSL renderer available, so the runtime behaviour is not executed here. The case rests on getattribute's documented contract and on matching ten siblings in the same directory. If you would rather see a render-side check before merging, say so and I will find a way to run one.

[linux-foundation-easycla]

• ❌ - login: @Chang-Jin-Lee / name: Chang-Jin-Lee. The commit (2a7f461) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please visit our EasyCLA portal and chat with our support bot.

[jstone-lucasfilm]

Thanks for this proposal, @Chang-Jin-Lee, and before we review it in detail, take a look at the instructions at #3063 (comment) for resolving the CLA authorization warnings.

[Chang-Jin-Lee]

Thanks — following the EasyCLA link now, so the authorization is on me rather than anything blocking on your side. I'll post here once the check flips.

Everything else on the PR is green: all 37 build jobs pass, and the change is one line in mx_geompropvalue_filename.osl plus the render test that fails without it.

[Chang-Jin-Lee]

Correcting my own comment above before you spend review time on it: there is no render test in this PR, and the job count was wrong.

The diff is one file, libraries/stdlib/genosl/mx_geompropvalue_filename.osl, +9/-5, with no test added. The checks are 35, all green. What I ran locally was the existing suite — MaterialXTest "[genosl]" at 2498 assertions over 5 cases, and the full run at 26198 over 84 — which passes both before and after, so it demonstrates no regression rather than the fix.

As the description says, I have no OSL renderer here, so the behaviour is argued from getattribute's contract and from the ten sibling implementations in the same directory, not executed. Sorry for the noise.

The CLA is still mine to clear and I have not finished it yet.