Skip to content

chore(Convolution2DPass): fix incorrect initial render in example - #3608

Open
Jo-Byr wants to merge 1 commit into
Kitware:masterfrom
Jo-Byr:fix-convolution-2d-pass-example
Open

chore(Convolution2DPass): fix incorrect initial render in example#3608
Jo-Byr wants to merge 1 commit into
Kitware:masterfrom
Jo-Byr:fix-convolution-2d-pass-example

Conversation

@Jo-Byr

@Jo-Byr Jo-Byr commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Add missing renderer.updateLightsGeometryToFollowCamera() to fix incorrect initial render and the first camera interaction changing the lighting of the scene

Context

In Convolution2DPass example, the ligting of the scene changes after the first camera interaction (rotation or zoom).

image image

Results

The lighting is correct at the first render.

Changes

Added missing renderer.updateLightsGeometryToFollowCamera();

@Jo-Byr
Jo-Byr requested a review from finetjul August 18, 2026 14:26
@Jo-Byr Jo-Byr self-assigned this Aug 18, 2026

@finetjul finetjul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PaulHax

PaulHax commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Drive-by observation: this looks like a general gap rather than an example issue. renderer.updateLightGeometry() exists but nothing calls it, so lights only get refreshed from the interaction path. Anything that moves the camera without a mouse ends up stale: view preset buttons, restored camera state, and a fair number of examples in here that call elevation/azimuth after setup.

For reference, VTK C++ calls it from vtkOpenGLRenderer::DeviceRender(), right next to UpdateLights():

this->UpdateCamera();
this->UpdateLightGeometry();
this->UpdateLights();
this->UpdateGeometry();

vtk.js has the equivalent OpenGL/Renderer buildPass calling only updateLights(), so the light geometry half never happens.

#2536 may be the same root cause. This one looks related too: https://discourse.vtk.org/t/issues-with-lighting-and-clipping-in-multi-renderer-vtk-js-setup/15495 where the workaround was setFocusedRenderer(), which fits since only the poked renderer gets its lights updated.

Might be fixable in one place, either there or by updating lights when the camera is modified. Happy to open an issue if that is useful.

@Jo-Byr

Jo-Byr commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

The issue I have is that in C++, vtkOpenGLRenderer inherits from vtkRenderer, and as such has access to UpdateLightGeometry, but in vtk-js, vtkOpenGLRenderer extends vtkViewNode.

vtkOpenGLRenderer holds vtkRenderer instances indirectly through model.renderable.getRenderWindow().getRenderers().

Should we call updateLightGeometry on each of them ?

@finetjul

Copy link
Copy Markdown
Member

vtkOpenGLRenderer holds vtkRenderer instances indirectly through model.renderable.getRenderWindow().getRenderers().

Isn't model.renderable what you are looking for (i.e. the vtkRenderer paired with vtkOpenGLRenderer)?

Update renderers' light geometry on every pass to fix incoherent lighting
@Jo-Byr
Jo-Byr force-pushed the fix-convolution-2d-pass-example branch from c1cac65 to 721b360 Compare August 21, 2026 07:04
@Jo-Byr

Jo-Byr commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

vtkOpenGLRenderer holds vtkRenderer instances indirectly through model.renderable.getRenderWindow().getRenderers().

Isn't model.renderable what you are looking for (i.e. the vtkRenderer paired with vtkOpenGLRenderer)?

Absolutely, I fixed it

@finetjul finetjul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the light geometry updated sufficiently often ?
i.e. is buildPass() called for each new camera position ?

@Jo-Byr

Jo-Byr commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

is buildPass() called for each new camera position ?

Yes, maybe even too much. It's called every few ms as long as the left click is held down (in Convolution2DPass and SimpleCone examples).

@finetjul

Copy link
Copy Markdown
Member

is buildPass() called for each new camera position ?

Yes, maybe even too much. It's called every few ms as long as the left click is held down (in Convolution2DPass and SimpleCone examples).

Sounds good then.

@finetjul finetjul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@finetjul

Copy link
Copy Markdown
Member

Is there an example to test if #2536 is fixed ?

@finetjul finetjul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently some test(s) are failing

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.

3 participants