Skip to content

Always trigger material needsUpdate in scene .material()#5983

Draft
Jepson2k wants to merge 1 commit intozauberzeug:mainfrom
Jepson2k:scene-bugfixes
Draft

Always trigger material needsUpdate in scene .material()#5983
Jepson2k wants to merge 1 commit intozauberzeug:mainfrom
Jepson2k:scene-bugfixes

Conversation

@Jepson2k
Copy link
Copy Markdown

@Jepson2k Jepson2k commented Apr 23, 2026

Motivation

The condition in scene.material(...) was

material.needsUpdate = material.vertexColors != vertexColors;

which only marked the material dirty when the vertex-color flag flipped.
But the same call also writes color, opacity, and side, so calling .material('red') followed by .material('blue') left needsUpdate=false for the second call and the color change was silently dropped on the next render.

Implementation

Set material.needsUpdate = true unconditionally so all property changes take effect on the next frame.

One-line change in nicegui/elements/scene/scene.js. No public API change.
This is one of several small slices being carved out of #5964 — opening it as a standalone PR per the splitting discussion on that thread.

Progress

  • The PR title is a short phrase starting with a verb like "Add ...", "Fix ...", "Update ...", "Remove ...", etc.
  • The implementation is complete. (Otherwise, open a draft PR.)
  • This PR does not address a security issue. (Security fixes must be coordinated via the security advisory process before opening a PR.)
  • Pytests are not necessary. (One-line correctness fix to a render-state flag; existing scene tests exercise this path.)
  • Documentation is not necessary.
  • No breaking changes to the public API.

The condition `material.needsUpdate = material.vertexColors != vertexColors`
only marked the material dirty when the vertex-color flag flipped, but the
same call also writes `color`, `opacity`, and `side`. Calling
`.material('red')` followed by `.material('blue')` left the material with
`needsUpdate=false` for the second call, so the color change was silently
dropped on the next render.

Set `needsUpdate = true` unconditionally so all property changes take
effect on the next frame.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@falkoschindler falkoschindler added bug Type/scope: Incorrect behavior in existing functionality review Status: PR is open and needs review labels Apr 24, 2026
@falkoschindler falkoschindler added this to the Next milestone Apr 24, 2026
@falkoschindler falkoschindler added analysis Status: Requires team/community input and removed bug Type/scope: Incorrect behavior in existing functionality review Status: PR is open and needs review labels Apr 24, 2026
@falkoschindler
Copy link
Copy Markdown
Contributor

Hi Jake, thanks for slicing this out. Before signing off I'd like to nail down what's actually being fixed, because I can't reproduce the described symptom.

The repro in the description — calling .material('red') then .material('blue') silently dropping the color change — doesn't match the mechanics: material.color.set(...) mutates the diffuse uniform, which is re-read every frame regardless of needsUpdate. A minimal test with two solid colors toggles correctly on main. Same story for opacity (uniform, and transparent: true is already set at mesh creation):

with ui.scene() as scene:
    box = scene.box().material('red', side='front')

ui.button('red', on_click=lambda: box.material('red'))
ui.button('green', on_click=lambda: box.material('green'))
ui.button('side=front', on_click=lambda: box.material('red', side='front'))
ui.button('side=back', on_click=lambda: box.material('red', side='back'))
ui.button('50% transparent', on_click=lambda: box.material('red', opacity=0.5))
ui.button('0% transparent', on_click=lambda: box.material('red', opacity=1.0))

The only mutations in this function that could genuinely need a shader recompile are:

  • material.side — for MeshPhongMaterial, switching between FrontSide/BackSide/DoubleSide affects the FLIP_SIDED / DOUBLE_SIDED defines used for normal flipping in lit back faces. Culling is a per-draw render state (so the visible effect changes immediately), but lighting on the newly-visible face stays wrong until the shader is recompiled. A simple side toggle on a box also rendered fine for me, though — possibly because the lighting difference is too subtle on a uniformly-lit primitive.
  • material.vertexColors — but the old expression already covered this case correctly (false != true and true != false both fire).

There's also a separate, clearer problem with the old line: material.needsUpdate = material.vertexColors != vertexColors can clobber a previously-requested needsUpdate = true with false. That's a latent bug independent of whether this call itself needs a recompile.

Could you share the exact scenario where you saw the problem? A minimal script we can run on main would help — if it's a side-under-lighting case (or something specific to the #5964 context with new material types), the description should say so. If it isn't reproducible in isolation, the PR still makes sense reframed as a defensive tightening ("the old conditional could clobber needsUpdate from true to false"), but the commit message should match what's actually being fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

analysis Status: Requires team/community input

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants