fix(shaders): improve compatibility with older Adreno and Mesa GLSL compilers - #115
Open
mrrobot97 wants to merge 2 commits into
Open
fix(shaders): improve compatibility with older Adreno and Mesa GLSL compilers#115mrrobot97 wants to merge 2 commits into
mrrobot97 wants to merge 2 commits into
Conversation
Some older Adreno GLSL compilers silently miscompile non-atomic path shaders when they contain the unused unsigned shifts in swizzle_image_buffer_idx. Restrict the helper to storage-buffer PLS and clockwise-atomic variants. This preserves its WebGPU use and the previous atomic shader behavior while keeping it out of unrelated path shaders.
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.
Summary
This PR contains two independent Android/OpenGL shader compatibility fixes, kept as separate commits:
Adreno path-shader fix
Emit
swizzle_image_buffer_idxonly for storage-buffer PLS and clockwise-atomic shader variants. This keeps its unused unsigned shifts out of unrelated path shaders that older Adreno GLSL compilers can silently miscompile.Mesa minifier fix
Preserve a token separator when a closing parenthesis is immediately followed by an identifier. This prevents older Mesa preprocessors from merging the final token of a function-like macro expansion with the following identifier.
These fixes address different driver failures and do not depend on device-model checks or runtime allowlists.
Problem 1: silent path-shader miscompile on older Adreno
After the storage-buffer PLS work in
365421d6,swizzle_image_buffer_idxbecame part of every path shader, even when it was unused.On Android 8.1 with an Adreno 616, the older GLSL compiler silently miscompiles non-atomic path shaders containing the helper's unused unsigned shifts. Filled and stroked vector paths disappear, while image meshes continue to render.
The regression was bisected to:
runtime-v0.1.118: renders correctlyruntime-v0.1.119: first broken release365421d6177757772a911f176241af8ea6d5e64eThe helper is required by
PLS_IMPL_STORAGE_BUFFER. Clockwise-atomic variants also contained the previous swizzle helper before that commit. Restricting it to those two configurations restores the previous shader contents for unrelated path variants while preserving its required uses.Problem 2: macro-token merge on older Mesa
The GLSL minifier previously removed whitespace between a closing parenthesis and a following identifier. That is normally safe because the preprocessor operates on tokens, but Mesa 20.3.4 under virgl can incorrectly merge the final token of a function-like macro expansion with the following identifier.
For example:
A conforming preprocessor treats the declaration as:
On the affected Mesa compiler, the minified form can instead be interpreted as if it contained
vec2v_position, producing anunexpected NEW_IDENTIFIERshader compilation error.The second commit makes the minifier emit:
OUT(vec2) v_position;This preserves the intended token boundary. It adds only semantically redundant whitespace for conforming compilers and does not change shader logic.
Reproduction
A small static
.rivartboard is sufficient. It should contain:A state machine is not required.
With the Rive GPU renderer:
The Android Canvas renderer can be used as a control because it does not use these GLSL path shaders.
The Mesa preprocessing failure can also be reproduced independently with the minimal function-like macro example above; no
.rivasset is needed for that compiler-level reproduction.Validation
unexpected NEW_IDENTIFIER;runtime-v0.1.169.Automated visual coverage for the Adreno failure is not practical because it is a silent device-driver miscompile. The Mesa change is isolated to token separation in generated GLSL and does not alter shader semantics.