Skip to content

fix: emit shortest equivalent f32 literals - #2963

Open
oxura wants to merge 2 commits into
software-mansion:mainfrom
oxura:fix/2772-short-float-literals
Open

fix: emit shortest equivalent f32 literals#2963
oxura wants to merge 2 commits into
software-mansion:mainfrom
oxura:fix/2772-short-float-literals

Conversation

@oxura

@oxura oxura commented Sep 2, 2026

Copy link
Copy Markdown

F32 values currently inherit JavaScript’s full decimal rendering after coercion. That turns a source value such as d.f32(0.3) into 0.30000001192092896f, even though WGSL converts the much clearer 0.3f to the same IEEE-754 value.

The generator now searches the at-most-nine significant decimal digits needed to round-trip an f32, verifies each candidate with Math.fround, and emits the shortest decimal or exponential spelling at the first matching precision. This preserves the exact target f32 value while avoiding spurious binary-to-decimal digits. Existing snapshots covering ordinary values, π, and a subnormal now document the shorter output.

Verification:

  • TypeGPU type tests
  • focused WGSL generator and multiplication suites: 92 passed
  • changed-file Oxlint and Oxfmt checks
  • fast unit suite on Node 24: 2,809 passed, 2 skipped
  • typegpu package build

Closes #2772.

Copilot AI lite review requested due to automatic review settings September 2, 2026 08:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is narrowly scoped to f32 literal rendering, includes updated snapshots and a targeted regression test, and I did not find correctness issues in the implementation.

Pull request overview

This PR improves WGSL code generation for f32 numeric literals by emitting the shortest decimal/exponential spelling that still round-trips to the same IEEE-754 f32 value, avoiding noisy “binary-to-decimal” artifacts in generated shaders.

Changes:

  • Update WgslGenerator.numericLiteral() to emit f32 literals via a new shortestF32() helper (verified with Math.fround).
  • Refresh multiple test snapshots to reflect the shorter f32 renderings (e.g. 0.3f, 3.1415927f, subnormal scientific notation).
  • Add a focused test asserting d.f32(0.3) resolves to return 0.3f;.
File summaries
File Description
packages/typegpu/src/tgsl/wgslGenerator.ts Adds shortestF32() and routes f32 literal emission through it to preserve exact f32 values with shorter text.
packages/typegpu/tests/tgsl/wgslGenerator.test.ts Adds a regression test ensuring f32(0.3) emits 0.3f.
packages/typegpu/tests/tgsl/extensionEnabled.test.ts Updates snapshot to match the new shortest f32 literal rendering.
packages/typegpu/tests/std/texture/textureGather.test.ts Updates snapshot constant idx to the shorter 1.2f.
packages/typegpu/tests/std/bitcast.test.ts Updates snapshot to shorter scientific notation for an f32 subnormal literal.
packages/typegpu/tests/resolve.test.ts Updates snapshot for pi constant to shorter f32 literal output.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/typegpu/src/tgsl/wgslGenerator.ts Outdated
Comment thread packages/typegpu/src/tgsl/wgslGenerator.ts Outdated
Comment thread packages/typegpu/tests/tgsl/wgslGenerator.test.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

This PR breaks the docs example WGSL snapshot suite: 21 tests across apps/typegpu-docs/tests/individual-example-tests/*.test.ts fail because their inline snapshots still pin the long f32 spellings this change intentionally shortens (6.283185307179586f now renders 6.2831855f, 3.141592653589793f3.1415927f, ...). The verification described in the PR body ran test:fast-unit, which explicitly excludes individual-example-tests (see root package.json), so the breakage never surfaced locally — it will fail the PR CI job (test-unit.ymlpnpm test:unit-and-attest).

Reviewed changes — the shortest-f32-literal emitter and its test updates, reviewed against main:

  • wgslGenerator.ts — new shortestF32() for the f32 branch of numericLiteral: walks precisions 1–9 via toPrecision, verifies each rounding with Math.fround against the f32-rounded target, returns the shorter of the decimal/exponential spelling, and special-cases -0.
  • wgslGenerator.test.ts — new regression test asserting d.f32(0.3) emits 0.3f (confirmed to fail on the pre-fix code).
  • Snapshot updates in resolve.test.ts, bitcast.test.ts, textureGather.test.ts, extensionEnabled.test.ts.

I verified the algorithm itself: across 4M random f32 bit patterns every emitted spelling round-trips to the target f32, emitted strings are valid WGSL float syntax, never exceed 9 significant digits, and a ~300k-sample optimality sweep against a brute-force shortest-digit search found zero sub-optimal outputs. The 4 updated snapshots match the emitter's output exactly.

🚨 Docs example WGSL snapshots not regenerated

The change to f32 literal printing only touched the four snapshot sites inside packages/typegpu/tests/. Every docs example snapshot that contains a long f32 literal now mismatches: I ran npx vitest run apps/typegpu-docs/tests/individual-example-tests (21 of 59 tests failing) and reproduced the mismatch on a single file with the exact CI filter (--project=!browser). This is a hard failure for the existing PR CI, not a cosmetic diff.

Technical details
# Regenerate docs example f32 snapshots

## Affected sites
- apps/typegpu-docs/tests/individual-example-tests/{3d-fish,caustics,circles,clouds,gravity,jelly-slider,jelly-switch,jump-flood-distance,log-test,oklab,perlin-noise,point-light-shadow,probability,ripple-cube,shifting-gradient,simple-shadow,slime-mold-3d,slime-mold,smoky-triangle,spinning-triangle,stable-fluid,vaporrave}.test.ts
- e.g. `6.283185307179586f``6.2831855f`, `3.141592653589793f``3.1415927f`, `0.15000000596046448f``0.1f`.

## Repro
- `npx vitest run apps/typegpu-docs/tests/individual-example-tests` → 21 failed of 59.

## Required outcome
- Regenerate the inline snapshots (`npx vitest run apps/typegpu-docs/tests/individual-example-tests --update`) and sanity-check a few of the new values round-trip via `Math.fround`.
- Re-run the full `pnpm test:unit-and-attest` (the `test-unit.yml` PR job), not just `test:fast-unit`, before merging.

ℹ️ Nitpicks

  • The f16 branch of numericLiteral (still base at wgslGenerator.ts:1207) keeps the verbose double spelling — the same shortest-round-trip argument applies to an 11-bit mantissa. Issue #2772 is f32-specific, so this is a scope observation, not a request. Worth an explicit decision in the PR body if it stays out of scope.
  • The -0 handling in shortestF32 is new behavior — previously -0 was emitted as 0f, dropping the sign bit — with no test coverage. Inline note added on the new test.

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

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.

bug: Numeric literals are forced into target precision unnecessarily

2 participants