Skip to content

Fix medium layer base weight in testrender - #2153

Merged
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
tdavidovicNV:fix/testrender-medium-layer-weight
Aug 25, 2026
Merged

Fix medium layer base weight in testrender#2153
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
tdavidovicNV:fix/testrender-medium-layer-weight

Conversation

@tdavidovicNV

Copy link
Copy Markdown
Contributor

Restore the pre-rewrite medium traversal behavior by avoiding a second application of the incoming weight to a layer base. Add a render regression comparing directly weighted and layered media.

Assisted-by: OpenAI Codex / GPT-5

Description

The non-recursive process_medium_closure traversal introduced in #1897
applies the incoming weight twice when it saves the base of a MaterialX layer.

base_w already includes the incoming weight:

base_w = weight * (1 - opacity(top));

The iterative walker then saves weight * base_w, giving a layer reached
with weight s an s * s factor on its base medium. The previous recursive
walker passed base_w directly, as does the current BSDF walker.

This change restores that behavior by saving base_w without multiplying by
the incoming weight again.

This is an internal testrender correction with no OSL language or public API
change, so no documentation update is needed.

Tests

The render-mx-medium-vdf test now renders a directly weighted absorbing
medium beside the same medium under a negligible layer top:

0.5 * medium
0.5 * layer(negligible_top, medium)

With the fix, the two spheres match. Before the fix, the layered sphere uses
0.5 * 0.5 for its medium weight and is visibly less dense; 22.7% of pixels
exceed the test's idiff threshold.

Local validation on Windows:

  • Release builds of testrender and oslc completed successfully.
  • render-mx-medium-vdf.opt passes through the standard test harness.
  • The same scene rendered with the pre-fix binary fails the new comparison.
  • clang-format 17 and git diff --check pass.
  • OptiX validation is left to CI.

Codex was used to audit the iterative traversal against its recursive
predecessor, prepare the regression scene, and draft the fix and 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.

Restore the pre-rewrite medium traversal behavior by avoiding a second
application of the incoming weight to a layer base. Add a render regression
comparing directly weighted and layered media.

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 (dc6500d)

@aconty

aconty commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Good catch. LGTM!

@tgrant-nv

Copy link
Copy Markdown
Contributor

Yes, good catch! Thanks for cleaning up my mess.

@lgritz

lgritz commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

I'm seeing that sanitizer failure on multiple branches, so I'm sure it's not related to this PR and will not let that hold things up.

Great fix, merging.

@lgritz
lgritz merged commit a97af52 into AcademySoftwareFoundation:main Aug 25, 2026
25 of 26 checks passed
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.

4 participants