Skip to content

Fix signed % wrong for negative operands in GLSL (OpenGL/GLES)#9687

Open
mstampfli wants to merge 1 commit into
gfx-rs:trunkfrom
mstampfli:fix/glsl-signed-int-modulo-negative
Open

Fix signed % wrong for negative operands in GLSL (OpenGL/GLES)#9687
mstampfli wants to merge 1 commit into
gfx-rs:trunkfrom
mstampfli:fix/glsl-signed-int-modulo-negative

Conversation

@mstampfli

@mstampfli mstampfli commented Jun 16, 2026

Copy link
Copy Markdown

Sibling of #9674 and the same class of bug as #8191: signed integer % wrong for negative operands, this time in the GLSL (OpenGL/GLES) backend instead of SPIR-V.

Problem

WGSL defines signed integer % for all non-degenerate operands. -1 % 768 must be -1 (truncated remainder, sign of the dividend; WGSL 8.7). But rendered through the GLSL backend it returns 255, i.e. (-1 as u32) % 768.

naga's GLSL backend emits a raw a % b for integer modulo. GLSL leaves % undefined when either operand is negative (GLSL spec 5.9), so the result is implementation defined. HLSL and Metal already reconstruct around this; GLSL did not. Same gap as #8191 on Vulkan, different backend.

Evidence

WGSL a % b run through the GLSL backend on an NVIDIA RTX 5060 OpenGL driver (610.43.02), before this change:

a         : -4  -3  -2  -1   0   1   2   3
a % 3 (cpu): -1   0  -2  -1   0   1   2   0
a % 3 (gpu):  2   0   1   2   0   1   2   0    WRONG for negatives

a % 768   : -1 -> 255 (should be -1),  -5 -> 251,  -769 -> 255, ...

Every negative dividend was wrong (32 of the 64 tested values), the same unsigned-fold symptom as the Vulkan case.

Fix

Integer division / truncates toward zero and is well defined in GLSL, so signed and unsigned % is lowered as a - b * (a / b), exactly mirroring the existing HLSL and Metal backends. Float modulo (already a trunc-based reconstruction) is unchanged.

Unlike the Vulkan fix this is unconditional rather than driver gated: GLSL's % is undefined for negative operands per spec, so it affects every OpenGL/GLES target, not one vendor.

Testing

  • Regenerated snapshots: only the operators and image GLSL outputs change (integer % becomes the reconstruction). HLSL, MSL, SPIR-V, and WGSL outputs are byte-for-byte unchanged. operators.wgsl already covers scalar, vector, and mixed scalar/vector modulo for signed and unsigned ints.
  • Re-ran the same shaders on the NVIDIA RTX 5060 OpenGL driver after the fix: all negative dividends match the CPU reference (-1 % 3 == -1, -2 % 3 == -2, -1 % 768 == -1), 0 mismatches across the tested range.

Squash or rebase? Squash.

@mstampfli mstampfli force-pushed the fix/glsl-signed-int-modulo-negative branch from b72b897 to cb14c32 Compare June 16, 2026 21:14
…ned)

WGSL defines signed integer `%` for all non-degenerate operands, but the
GLSL backend emitted a raw `a % b`. GLSL leaves `%` undefined when either
operand is negative (GLSL spec 5.9), so `-1 % 768` returned `255` instead
of `-1` on OpenGL/GLES (confirmed on NVIDIA). Same class of bug as gfx-rs#8191
on Vulkan, just a different backend.

Integer division `/` truncates toward zero and is well defined, so lower
signed and unsigned integer `%` as `a - b * (a / b)`, mirroring the HLSL
and Metal backends. Float modulo is unchanged.

Verified on an NVIDIA RTX 5060 GL driver: negative dividends now match
the CPU reference (e.g. -1 % 3 == -1, -1 % 768 == -1).
@mstampfli mstampfli changed the title [naga] glsl: fix signed integer % for negative operands Fix signed % wrong for negative operands in GLSL (OpenGL/GLES) Jun 16, 2026
@mstampfli mstampfli force-pushed the fix/glsl-signed-int-modulo-negative branch from cb14c32 to 82844e3 Compare June 16, 2026 21:19
@inner-daemons

inner-daemons commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

This is a great bug report, thanks for filing. Will take a look at the code soon.

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

Labels

area: correctness We're behaving incorrectly lang: GLSL OpenGL Shading Language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants