Skip to content

impr: auto-interpolate integer varyings - #2907

Open
huytdps13400 wants to merge 2 commits into
software-mansion:mainfrom
huytdps13400:fix/2147-flat-integer-varyings
Open

impr: auto-interpolate integer varyings#2907
huytdps13400 wants to merge 2 commits into
software-mansion:mainfrom
huytdps13400:fix/2147-flat-integer-varyings

Conversation

@huytdps13400

Copy link
Copy Markdown

Summary

  • automatically add @interpolate(flat) to integer scalar and vector varyings
  • apply the behavior only to vertex outputs and fragment inputs, never integer builtins, vertex attributes, or fragment outputs
  • preserve explicit interpolation and custom location attributes
  • cover shelled and inferred render pipelines plus the integer-builtin boundary

Closes #2147.

Verification

  • RED: focused render-pipeline regressions failed because implicit integer varyings had no flat interpolation
  • pnpm test:fast-unit — 216 files, 2,795 tests passed; 2 baseline skips
  • pnpm --filter typegpu test:types
  • pnpm test:style — zero warnings/errors
  • pnpm --filter typegpu build
  • git diff --check

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.

Pull request overview

Automatically applies @interpolate(flat) to eligible integer vertex outputs and fragment inputs while preserving explicit attributes, locations, and builtins.

Changes:

  • Adds integer scalar/vector interpolation handling.
  • Supports typed and inferred render pipelines.
  • Adds regression coverage and updates snapshots.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/typegpu/tests/renderPipeline.test.ts Adds regression tests and updates snapshots.
packages/typegpu/src/data/autoStruct.ts Propagates interpolation configuration.
packages/typegpu/src/core/function/tgpuVertexFn.ts Enables interpolation for vertex outputs.
packages/typegpu/src/core/function/tgpuFragmentFn.ts Enables interpolation for fragment inputs.
packages/typegpu/src/core/function/ioSchema.ts Applies flat interpolation to eligible integer varyings.
packages/typegpu/src/core/function/autoIO.ts Configures inferred pipeline I/O.

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

@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.

ℹ️ No critical issues — one test-coverage suggestion inline.

Reviewed changes

  • Auto-flat interpolation for integer varyings — new withFlatInterpolationForInteger in ioSchema.ts, gated behind a new autoInterpolateIntegers flag threaded through withLocations/separateBuiltins/createIoSchema. Wraps only non-decorated or location-decorated integer scalars/vectors with @interpolate(flat), and is idempotent when an @interpolate attribute already exists.
  • Scoped to vertex out / fragment in only — the flag is true only on the vertex-output and fragment-input paths (tgpuVertexFn.ts, tgpuFragmentFn.ts, and both AutoStruct paths in autoIO.ts). Vertex attribute inputs, fragment outputs, compute fns, and integer builtins keep the flag off.
  • Tests — two new render-pipeline regressions (explicit struct layout and inferred varyings) plus updated snapshots pinning the emitted @interpolate(flat).

I traced the change through the count of reachable wiring sites and the emission path; the wiring matches the stated scope (tgpuVertexFn.ts:180, tgpuFragmentFn.ts:212, autoIO.ts:89/142). The createIoSchema(returnType) call in resolutionCtx.ts:767 (left at the default false) only fires for primitive returns — fragment color outputs / non-varying primitives — which the PR intentionally excludes, so no coverage gap there. 'flat' is a valid FlatInterpolationType, so auto-generated output is spec-legal.

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

Comment thread packages/typegpu/tests/renderPipeline.test.ts

@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.

✅ No new issues found.

Reviewed changes

  • Added the scope-locking regression the prior review requested — "does not flat interpolate integer vertex inputs or fragment outputs" now asserts a vertex in: integer is emitted with only its @location and a fragment out: integer keeps only its @location, with neither getting @interpolate(flat). This genuinely pins the boundary (a leak to those two paths would fail the assertions), closing the coverage gap previously called out. The earlier positive tests already covered the fragment-input/vertex-output side, so the four-position scope is now fully locked from both directions.

ℹ️ Nitpicks

  • Test-only diff; nothing to nitpick. The explicit vertexFn/fragmentFn forms used here are the right complement to the existing inferred-form regressions.

Pullfrog  | 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.

impr: Auto @interpolate(flat) when integer field encountered in varyings struct

2 participants