Clamp the discarded select arm's index in prefix-sum lookups - #1817
Open
lexoliu wants to merge 1 commit into
Open
Clamp the discarded select arm's index in prefix-sum lookups#1817lexoliu wants to merge 1 commit into
lexoliu wants to merge 1 commit into
Conversation
Both arms of a select are evaluated, so lookups of the form select(default, sh_array[ix - 1u], ix > 0u) compute sh_array[0u - 1u] on lanes where ix == 0. Mali lowers workgroup arrays to ordinary memory, so the underflowed index becomes a wild address and the driver reports a device loss; on a Pixel 9 Pro (Immortalis-G715, Android 16) any scene containing a path lost the device on the first frame. The shaders run unchecked since linebender#1093, so no bounds check masks it. Clamping the index in the discarded arm changes nothing when ix > 0 and keeps the dead access in bounds when ix == 0. tile_alloc, backdrop_dyn and coarse fault on the device; the two fine.wgsl sites share the pattern and are fixed for consistency.
lexoliu
force-pushed
the
fix-select-index-underflow
branch
from
August 13, 2026 03:17
a302016 to
5e5f538
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a Vulkan device-loss on Mali GPUs caused by underflowed workgroup-array indexing in the discarded arm of WGSL select expressions used for prefix-sum lookups.
Changes:
- Clamp the index used in the non-selected
selectarm (max(ix, 1u) - 1u) across six prefix-sum lookup sites to prevent underflow onix == 0. - Update the Unreleased changelog with a note about the Mali device-loss fix.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vello_shaders/shader/tile_alloc.wgsl | Clamp discarded-arm prefix-sum index to avoid - 1u underflow. |
| vello_shaders/shader/coarse.wgsl | Clamp discarded-arm prefix-sum indices in two prefix-sum lookups. |
| vello_shaders/shader/backdrop_dyn.wgsl | Clamp discarded-arm prefix-sum index in row prefix-sum lookup. |
| vello_shaders/shader/fine.wgsl | Clamp discarded-arm prefix-sum indices in two prefix-sum lookups. |
| CHANGELOG.md | Add Unreleased entry describing the Mali device-loss fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ### Fixed | ||
|
|
||
| - Rendering scenes whose binning requires more than 256 bins. ([#1700][] by [@b0nes164][]) | ||
| - Prefix-sum lookups no longer index workgroup arrays with an underflowed index in the discarded arm of a `select`, which caused a device loss on Mali GPUs for any scene containing a path. ([#XXXX][] by [@lexoliu][]) |
lexoliu
added a commit
to water-rs/waterui
that referenced
this pull request
Aug 13, 2026
Points vello at lexoliu/vello#fix-select-index-underflow (upstream PR linebender/vello#1817): the discarded arm of a select computed an underflowed workgroup-array index and Mali page-faults on it, losing the Vulkan device for any scene containing a path. Drop this patch when a vello release containing the fix ships.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1816.
Several prefix-sum lookups have the shape
select(default, sh_array[ix - 1u], ix > 0u). Both arms ofselectareevaluated, so lanes with
ix == 0computesh_array[0u - 1u]beforediscarding it. Mali lowers workgroup arrays to ordinary memory, so the
underflowed index becomes a wild address and the driver reports a device loss;
on a Pixel 9 Pro (Immortalis-G715, Android 16) any scene containing a path
lost the device on the first frame. The shaders run unchecked since #1093, so
there is no bounds check to mask it.
This clamps the index in the discarded arm,
sh_array[max(ix, 1u) - 1u],which changes nothing when
ix > 0and keeps the dead access in bounds whenix == 0. Six sites: one each intile_allocandbackdrop_dyn, two incoarse(these four fault on the device), and two infinewhich have thesame pattern but didn't fault in my scenes.
Verified on the Pixel 9 Pro with fully unchecked shaders: the scenes that
previously lost the device render correctly.
vello_testspasses on Metal, sothe selected values are unchanged. An explicit
ifwould avoid the dualevaluation altogether; I kept the clamp since it's the smallest diff and it's
what I tested on hardware, but happy to change the shape.