Skip to content

Fix ADD accumulation in testrender closure walkers - #2152

Open
tdavidovicNV wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
tdavidovicNV:fix/testrender-add-accumulation
Open

Fix ADD accumulation in testrender closure walkers#2152
tdavidovicNV wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
tdavidovicNV:fix/testrender-add-accumulation

Conversation

@tdavidovicNV

@tdavidovicNV tdavidovicNV commented Aug 24, 2026

Copy link
Copy Markdown

Assisted-by: OpenAI Codex / GPT-5

Description

Fixes #2151.

The non-recursive closure traversal introduced in #1897 stopped combining
completed ADD branches in two places:

  • evaluate_layer_opacity
  • process_background_closure

Both functions still visit the full closure tree, but return only the final
branch contribution. This can make a weighted sum of opaque closures appear
partially transparent when used as the top of a MaterialX layer, and makes an
added pair of background closures return only the second color.

This change tracks the active branch_weight separately from the
accumulated_weight of completed branches. At an ADD, the second branch and
its incoming weight are saved on the stack. When a branch completes, its final
weight is accumulated before traversal resumes from the stack. The traversal
remains non-recursive for OptiX.

This is an internal testrender bug fix and does not change the OSL language or
public APIs, so no documentation update is needed.

Tests

render-mx-layer renders these expressions side by side, using opaque red and
blue top closures and a green base:

layer(0.5 * red + 0.5 * blue, green)
0.5 * layer(red, green) + 0.5 * layer(blue, green)

They should match, with no contribution from the green base. Before the fix,
the first expression is assigned a top opacity of 0.5.

render-background expresses the existing environment result as two
complementary background branches:

Ci = Kb * c * color(1, 1, 0) * background()
   + Kb * c * color(0, 0, 1) * background();

Their sum is the previous Kb * c * background() result, so the existing
reference remains valid. Before the fix, only the blue branch is returned.

Local validation on Windows:

  • Release builds of testrender and oslc completed successfully.
  • Both new outputs pass their idiff comparisons.
  • The same scenes fail with the pre-fix testrender, showing the expected
    green base leak and missing background branch.
  • OptiX validation is left to CI.

Codex was used to compare the recursive and non-recursive traversals, prepare
the reproducers, and draft the fix and regression tests. I reviewed the
resulting changes and test output.

Checklist

  • I have read the guidelines on contributions and code review procedures.
  • I have read the Policy on AI Coding Assistants
    and if I used AI coding assistants, I have an Assisted-by: TOOL / MODEL
    line in the pull request description above.
  • I have updated the documentation if my PR adds features or changes
    behavior.
  • I am sure that this PR's changes are tested in the testsuite.
  • I have run and passed the testsuite in CI before submitting the
    PR, by pushing the changes to my fork and seeing that the automated CI
    passed there. (Exceptions: If most tests pass and you can't figure out why
    the remaining ones fail, it's ok to submit the PR and ask for help. Or if
    any failures seem entirely unrelated to your change; sometimes things break
    on the GitHub runners.)
  • My code follows the prevailing code style of this project and I
    fixed any problems reported by the clang-format CI test.

Assisted-by: OpenAI Codex / GPT-5
Signed-off-by: Tomas Davidovic <tdavidovic@nvidia.com>
@linux-foundation-easycla

linux-foundation-easycla Bot commented Aug 24, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: tdavidovicNV / name: Tomas Davidovic (3c553fe)

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.

[BUG] testrender drops earlier ADD branches in layer-opacity and background evaluation

1 participant