feat: Route binary operations through the shader generator - #2822
Open
iwoplaza wants to merge 1 commit into
Open
feat: Route binary operations through the shader generator#2822iwoplaza wants to merge 1 commit into
iwoplaza wants to merge 1 commit into
Conversation
|
pkg.pr.new packages benchmark commit |
iwoplaza
force-pushed
the
feat/route-bin-ops-through-generator
branch
from
August 10, 2026 10:06
b5d746f to
077d022
Compare
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.80, 1.68, 3.66, 4.94, 5.93, 8.99, 18.18, 22.01]
line [0.77, 1.54, 3.41, 4.75, 5.86, 9.70, 17.73, 21.12]
line [0.81, 1.74, 3.46, 5.67, 5.88, 10.12, 18.79, 19.11]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.24, 0.48, 0.59, 0.73, 0.95, 1.03, 1.21, 1.30]
line [0.22, 0.40, 0.56, 0.66, 0.92, 0.97, 1.18, 1.24]
line [0.29, 0.50, 0.63, 0.81, 1.04, 1.05, 1.21, 1.38]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.77, 1.72, 3.91, 6.10, 10.19, 21.15, 45.54, 91.48]
line [0.88, 1.71, 3.43, 5.31, 9.83, 20.73, 45.94, 90.59]
line [0.67, 1.74, 4.11, 6.57, 10.84, 21.93, 47.39, 96.65]
|
Bundle size comparison (
|
| 🟢 Decreased (max -0.25%) | ➖ Unchanged | 🔴 Increased (max 2.79%) | ❔ Unknown |
|---|---|---|---|
| 6 | 129 | 187 | 1 |
import * as ... in PR vs import * as ... in target (did bundle size increase?):
| Test | tsdown |
|---|---|
| std_isBeingTranspiled.ts | 15.71 kB ( |
| std_getTargetShaderLanguage.ts | 15.77 kB ( |
| STATIC_std.ts | 110.08 kB ( |
| std_getShaderStage.ts | 15.76 kB |
import { ... } in PR vs import * as ... in PR (is the library tree-Shakeable?):
| Test | tsdown |
|---|---|
| tgpu_init.ts | 268.30 kB ( |
| tgpu_initFromDevice.ts | 267.77 kB ( |
| tgpu_resolve.ts | 169.05 kB ( |
| tgpu_resolveWithContext.ts | 168.98 kB ( |
| tgpu_bindGroupLayout.ts | 73.91 kB ( |
| tgpu_mutableAccessor.ts | 68.64 kB ( |
| tgpu_accessor.ts | 68.63 kB ( |
| tgpu_privateVar.ts | 67.33 kB ( |
| tgpu_workgroupVar.ts | 67.32 kB ( |
| tgpu_const.ts | 66.74 kB ( |
| tgpu_lazy.ts | 66.54 kB ( |
| tgpu_fragmentFn.ts | 38.92 kB ( |
| tgpu_fn.ts | 38.86 kB ( |
| tgpu_vertexFn.ts | 38.73 kB ( |
| tgpu_computeFn.ts | 38.44 kB ( |
| tgpu_vertexLayout.ts | 27.57 kB ( |
| tgpu_comptime.ts | 15.18 kB ( |
| tgpu_unroll.ts | 1.75 kB ( |
| tgpu_slot.ts | 1.70 kB ( |
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ℹ️ Minor suggestions only — one small typo in an error message.
Reviewed changes
emitBinaryOproute — AddedemitBinaryOpplus theBinaryOperatorunion to theShaderGeneratorinterface andWgslGenerator, and rewired+ - * / % << >>instd/operators.tsand the binary-expression emitter through it (replacing inlinestitchand the old parenthesization block).- Method renames — Renamed the unstable
ShaderGeneratormethodscall→emitCallandtypeAnnotation→emitTypeAnnotationacrossWgslGenerator,GlslGenerator,internal.ts,resolutionCtx.ts, and every std codegen call site. All callers were updated. _callShelllessextraction — Factored the shellless-call lowering out of_expressioninto a reusable protectedWgslGenerator._callShellless, and reused it fromGlslGenerator.emitBinaryOpto translate floating-point%into aremaindershell helper (GLSL ES has no float%), leaving non-float%on thesuperpath.
The parenthesization and operator-mapping behavior is preserved for WGSL, so existing WGSL snapshots should be unaffected.
ℹ️ Nitpicks
packages/typegpu-gl/src/glslGenerator.ts:430— error message has a doubled(type:prefix on the RHS operand.- The
%→remainderpath in the new test only covers scalarf32;vec2f/3f/4foperands hit the same branch but aren't exercised.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
iwoplaza
force-pushed
the
feat/route-bin-ops-through-generator
branch
from
August 10, 2026 10:17
077d022 to
728fda6
Compare
iwoplaza
force-pushed
the
feat/route-bin-ops-through-generator
branch
from
August 10, 2026 10:21
728fda6 to
74bc6ff
Compare
There was a problem hiding this comment.
Pull request overview
This PR centralizes emission of binary operations (and related helpers like function calls/type annotations) behind the ShaderGenerator interface so language backends (notably GLSL) can customize operator lowering while reusing the same TGSL resolution pipeline.
Changes:
- Add
emitBinaryOptoShaderGeneratorand route TGSL binary/assignment/logical operator emission through it. - Rename
typeAnnotation/calltoemitTypeAnnotation/emitCalland update stdlib + resolution code to use the new API. - Extend the GLSL generator to lower floating-point
%to a generatedremainderhelper, with a new regression test.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/typegpu/src/tgsl/wgslGenerator.ts | Adds _callShellless, routes binary ops via emitBinaryOp, renames generator emission helpers. |
| packages/typegpu/src/tgsl/shellless.ts | Makes ShelllessRepository.get accept readonly snippet arrays. |
| packages/typegpu/src/tgsl/shaderGenerator.ts | Extends generator interface with emitBinaryOp and renames emission methods. |
| packages/typegpu/src/std/operators.ts | Routes arithmetic/shift operator codegen via ctx.gen.emitBinaryOp (and generator-driven instantiation). |
| packages/typegpu/src/std/numeric.ts | Updates built-in call emission to emitCall. |
| packages/typegpu/src/std/boolean.ts | Updates built-in call emission to emitCall. |
| packages/typegpu/src/std/bitcast.ts | Updates built-in call emission to emitCall. |
| packages/typegpu/src/std/array.ts | Updates built-in call emission to emitCall. |
| packages/typegpu/src/resolutionCtx.ts | Updates schema-to-type annotation emission to emitTypeAnnotation. |
| packages/typegpu/src/internal.ts | Re-exports BinaryOperator from internal API surface. |
| packages/typegpu-gl/tests/glslGenerator.test.ts | Adds test ensuring float % becomes a remainder() call in GLSL output. |
| packages/typegpu-gl/src/glslGenerator.ts | Implements GLSL-specific % lowering via a shellless remainder helper and updates renamed methods. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+46
to
+63
| export type BinaryOperator = | ||
| | '=' | ||
| | '^' | ||
| | '|' | ||
| | '&' | ||
| | '*' | ||
| | '/' | ||
| | '%' | ||
| | '+' | ||
| | '-' | ||
| | '<<' | ||
| | '>>' | ||
| | '<' | ||
| | '>' | ||
| | '<=' | ||
| | '>=' | ||
| | '==' | ||
| | '!='; |
cieplypolar
approved these changes
Aug 10, 2026
| schema: d.BaseData | UnknownData, | ||
| ): schema is d.F32 | d.Vec2f | d.Vec3f | d.Vec4f { | ||
| return ( | ||
| typeof schema !== 'symbol' && |
Collaborator
There was a problem hiding this comment.
Suggested change
| typeof schema !== 'symbol' && | |
| schema !== UnknownData && |
It is obvious for us that this symbol is UnknownData, but I think an explicit check is more readable.
| new (): T; | ||
| } | ||
|
|
||
| export type BinaryOperator = |
Collaborator
There was a problem hiding this comment.
Could you create an issue to update BinaryOperator type and link it in the TODO comment?
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.
No description provided.