Conversation
That wasn't happening before
There was a problem hiding this comment.
Pull request overview
Adjusts the Vispy backend blending setup to avoid depth testing for non-opaque blend modes, addressing transparency artifacts caused by depth testing during blending.
Changes:
- Update Vispy node blending configuration to disable
depth_testfor non-opaque blend modes. - Update Vispy volume adaptor tests to assert the new
set_gl_state(..., depth_test=False)calls.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/scenex/adaptors/_vispy/_node.py |
Changes how GL state is set for blended modes by adding depth_test=False. |
tests/adaptors/_vispy/test_volume.py |
Updates blending assertions to match the new GL-state call signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| adaptor._vispy_node.set_gl_state.assert_called_with("translucent", depth_test=False) | ||
|
|
||
| volume.blending = BlendMode.OPAQUE | ||
| adaptor._vispy_node.set_gl_state.assert_called_with(None, blend=False) |
There was a problem hiding this comment.
The test currently asserts the opaque transition calls set_gl_state(None, blend=False) without restoring depth testing. If the implementation is updated to explicitly re-enable depth testing for BlendMode.OPAQUE (to avoid state leaking from prior translucent/additive modes), this assertion should also require depth_test=True so the regression is covered.
| adaptor._vispy_node.set_gl_state.assert_called_with(None, blend=False) | |
| adaptor._vispy_node.set_gl_state.assert_called_with( | |
| None, blend=False, depth_test=True | |
| ) |
| # for opaque, we need to disable blending | ||
| self._vispy_node.set_gl_state(None, blend=False) # pyright: ignore |
There was a problem hiding this comment.
When switching from a blended mode (where you now set depth_test=False) back to BlendMode.OPAQUE, the current call set_gl_state(None, blend=False) does not explicitly restore depth_test=True. In vispy, set_gl_state updates the stored GL-state, so omitting depth_test here can leave depth testing disabled for opaque rendering after a mode toggle. Explicitly set depth_test=True (and consider also restoring other depth-related state if needed) in the opaque branch, or use the appropriate vispy preset that re-enables depth testing.
| # for opaque, we need to disable blending | |
| self._vispy_node.set_gl_state(None, blend=False) # pyright: ignore | |
| # for opaque, we need to disable blending and restore depth testing | |
| self._vispy_node.set_gl_state( # pyright: ignore | |
| None, blend=False, depth_test=True | |
| ) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #69 +/- ##
=======================================
Coverage 88.41% 88.41%
=======================================
Files 62 62
Lines 3074 3074
=======================================
Hits 2718 2718
Misses 356 356 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The vispy backend does depth testing for all blending modes, which, bluntly speaking, breaks transparency. This PR disables depth testing for blending modes where some sort of blending should occur.