Skip to content

fix(@typegpu/gl): support array texture loads - #2961

Open
oxura wants to merge 3 commits into
software-mansion:mainfrom
oxura:fix/2943-texture-load-arrays
Open

fix(@typegpu/gl): support array texture loads#2961
oxura wants to merge 3 commits into
software-mansion:mainfrom
oxura:fix/2943-texture-load-arrays

Conversation

@oxura

@oxura oxura commented Sep 2, 2026

Copy link
Copy Markdown

textureLoad receives (texture, coords, arrayIndex, level) for 2D-array textures, but the GLSL fallback was destructuring every call as (texture, coords, level). That dropped the real mip level and emitted 2D coordinates for a sampler2DArray.

The generator now detects 2D-array textures, keeps the array index and mip level separate, and emits texelFetch with an ivec3(coords, layer). It also normalizes unsigned coordinate/index inputs to the signed integer types GLSL requires. Non-array and multisampled loads keep their existing path.

Verification:

  • focused GLSL generator suite: 32 passed
  • @typegpu/gl type tests and build
  • changed-file Oxlint and Oxfmt checks
  • fast unit suite on Node 24: 2,809 passed, 2 skipped

Closes #2943.

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

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 GLSL generation, aligns with WGSL signatures, and is covered by targeted generator tests.

Pull request overview

Fixes the WebGL/GLSL fallback codegen for std.textureLoad on 2D-array textures by preserving the (arrayIndex, mipLevel) split and emitting a correct texelFetch call with ivec3(coords, layer).

Changes:

  • Update GlslGenerator’s textureLoad emission to detect texture_2d_array/texture_depth_2d_array and generate texelFetch(texture, ivec3(coords, layer), level).
  • Normalize unsigned coordinate/index inputs to GLSL’s required signed integer types where needed.
  • Add a focused generator snapshot test covering 2D-array textureLoad.
File summaries
File Description
packages/typegpu-gl/src/glslGenerator.ts Fixes textureLoad argument handling for 2D-array textures and ensures GLSL integer type expectations are met for coords/layer.
packages/typegpu-gl/tests/glslGenerator.test.ts Adds an inline-snapshot test asserting correct GLSL output for 2D-array textureLoad.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • 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-gl/tests/glslGenerator.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.

ℹ️ The 2d-array load fix is correct and well-scoped, and the new test genuinely fails without it — three rough edges worth a look.

Reviewed changes

  • Array-aware textureLoad argument handlingGlslGenerator.emitCall now detects 2d-array textures, keeps the array index and mip level separate, and emits texelFetch(texture, ivec3(coords, layer), level).
  • Unsigned input normalizationvec2u coords become ivec2(...) and u32 array indices become int(...), matching GLSL's signed texelFetch signature.
  • Regression test — a sampler2DArray load now pins the ivec3-merged emission; this assertion fails on the pre-fix code path.

Verified locally: focused glslGenerator suite (32 passed) and the full @typegpu/gl suite (65 passed, 2 skipped) both green on this branch.

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

Comment thread packages/typegpu-gl/src/glslGenerator.ts Outdated
Comment thread packages/typegpu-gl/src/glslGenerator.ts
Comment thread packages/typegpu-gl/tests/glslGenerator.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.

ℹ️ The delta cleanly addresses all three findings from the prior review, and the new tests each discriminate the change they cover. One rough edge on the new depth-array snapshot is worth a look.

Reviewed changes

  • Type-discriminant array detectionisTextureArray now keys on textureType === 'texture_2d_array' || 'texture_depth_2d_array' instead of dimension === '2d-array', so read-only texture_storage_2d_array loads keep the non-array path instead of throwing on the four-argument check.
  • Extended unsigned normalization — the vec2u → ivec2 conversion now also covers vec3u → ivec3, so 3D loads with unsigned coordinates no longer emit invalid uvec3 texelFetch coordinates.
  • Expanded test coverage — new snapshots pin the vec2u/u32 normalization, the flip-adapted array emission, depth-2d-array loading, unsigned 3D loading, and a storage-array discrimination regression that genuinely fails on the pre-fix code.

Verified locally on head 56e2e548: focused glslGenerator suite 36 passed; full @typegpu/gl suite 69 passed / 2 skipped; test:types and per-file oxlint/oxfmt clean.

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

Comment thread packages/typegpu-gl/tests/glslGenerator.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.

✅ No new issues found.

Reviewed changes

  • Deferred unsupported depth-array loadsisTextureArray now matches only sampled texture_2d_array (glslGenerator.ts:650), so texture_depth_2d_array loads no longer route through the ivec3(coords, layer) emission. This resolves the concern from the prior review that the depth snapshot was blessing output that cannot compile under GLSL ES 3.00.
  • Removed the depth-2d-array snapshot test — the texelFetch(depthPalette, ivec3(...), 1) assertion is gone, deferring depth loads until the fallback has a real sampler representation for them.

The narrowing is behavior-identical to base for the affected path: depth-2d-array loads (reachable only via rawCodeSnippet synthetic handles — emitVarDeclaration and the WebGL fallback reject them) now emit through the same non-array branch the pre-PR code used, so nothing regresses and the compilability concern disappears. Verified locally on head 859e3e2c: full @typegpu/gl suite 68 passed / 2 skipped, test:types clean, working tree clean.

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.

fix(@typegpu/gl): Suport texture arrays in textureLoad

2 participants