Repository navigation
Conversation
|
pkg.pr.new No packages changed since release |
fdb90b1 to
685a215
Compare
Bundle size comparison (
|
| 🟢 Decreased | ➖ Unchanged | 🔴 Increased | ❔ Unknown |
|---|---|---|---|
| 0 | 356 | 0 | 0 |
No notable changes.
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
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.72, 1.47, 3.30, 5.70, 5.87, 9.57, 19.84, 22.33]
line [0.85, 1.64, 3.66, 5.47, 6.12, 8.68, 20.84, 20.97]
line [0.83, 1.68, 3.50, 5.64, 5.84, 10.47, 21.56, 22.56]
---
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.19, 0.36, 0.55, 0.66, 0.95, 0.97, 1.14, 1.22]
line [0.19, 0.38, 0.52, 0.66, 0.92, 0.96, 1.18, 1.30]
line [0.23, 0.43, 0.58, 0.72, 0.93, 0.91, 1.10, 1.25]
---
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.69, 2.04, 3.04, 6.31, 10.78, 22.64, 48.36, 96.44]
line [0.83, 1.99, 3.43, 6.00, 10.98, 22.02, 48.02, 96.52]
line [0.79, 1.91, 3.27, 6.17, 11.25, 23.93, 47.29, 97.52]
|
1ff47c0 to
b08e885
Compare
b08e885 to
21cab0c
Compare
bac369a to
3659e58
Compare
21cab0c to
b8f8ab5
Compare
3659e58 to
e030ffe
Compare
c413681 to
2ce9aa9
Compare
e030ffe to
d7077d5
Compare
ce8d16f to
baa3a27
Compare
9c6aefc to
edb9a65
Compare
6f5388c to
3c4dffe
Compare
c3f8fac to
cffdf5c
Compare
aed94dc to
c9b87cc
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Quality state races and incorrect device-motion axis mapping cause observable behavior errors.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Adds an advanced “Jelly Knob” WebGPU example featuring ray-marched rendering, spring physics, motion input, and configurable quality.
Changes:
- Implements the knob’s rendering, materials, TAA, camera, and physics.
- Adds interaction, phone motion, quality controls, and resource cleanup.
- Registers the example and adds lifecycle tests.
| File | Description |
|---|---|
apps/typegpu-docs/tests/individual-example-tests/jelly-knob-lifecycle.test.ts |
Tests spring behavior and resource cleanup. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/utils.ts |
Adds rendering and texture helpers. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/taa.ts |
Implements temporal anti-aliasing. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/spring.ts |
Implements scalar and 3D springs. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/sdfs.ts |
Defines scene SDF geometry. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/raymarchers.ts |
Implements ray-marched shading. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/motion.ts |
Handles phone motion input. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/meta.json |
Provides example metadata. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/material.ts |
Defines jelly reflection behavior. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/knob.ts |
Manages knob state and physics. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/index.ts |
Integrates rendering, controls, input, and cleanup. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/index.html |
Adds the example canvas. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/dataTypes.ts |
Defines GPU data structures and layouts. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/constants.ts |
Defines rendering and physics constants. |
apps/typegpu-docs/src/examples/rendering/jelly-knob/camera.ts |
Implements camera projection and jitter. |
apps/typegpu-docs/src/components/ExampleShowcase.tsx |
Registers advanced showcase examples. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
ℹ️ No blocking issues — one minor robustness suggestion inline. Two of the existing Copilot findings appear to be false positives.
Reviewed changes
This run read the full 17-file diff of the new jelly-knob example, ran astro check (0 errors), oxlint --type-aware (clean), and the new lifecycle test (3/3 pass), and validated each open Copilot finding.
- New example —
jelly-knob/ray-marched knob: SDF scene, TAA + blue-noise dither, spring physics, phone-motion input, and controls, registered as an advanced example. - Lifecycle —
handleResizedestroys and recreates render + TAA textures and bind groups;onCleanuptears down the root and removes itswindowlistener. - Tests —
jelly-knob-lifecycle.test.tspins spring convergence/substep parity and texture/listener cleanup; it also exercises shader generation indirectly via import + render. - Showcase list — registers
rendering--jelly-knoband, unrelated to this example,rendering--triplanar-mapping.
ℹ️ Two of the prior review's findings don't hold up
Copilot's review raised three points; two are incorrect and shouldn't drive changes.
- Initial quality vs. rendering scale — the controls runner eagerly invokes each control's handler with its
initialvalue on mount (initializeParaminapps/typegpu-docs/src/utils/examples/exampleRunner.ts:7, called fromexecuteExample). SoonSelectChange('Ultra')runshandleResize()withqualityScale = 1.0right after import, and the displayed selector matches the render resolution. rotationRateaxes — per W3C Device Orientation and Motion §6.3.2, theDeviceMotionEventRotationRategetters arealpha→ x-axis,beta→ y-axis,gamma→ z-axis ("The alpha getter steps are to return the value of this's x axis rotation rate").motion.tsmaps them in exactly that order, and its comment is correct. Copilot'salpha = Zmapping is theDeviceOrientationEventEuler-angle convention, which does not apply torotationRate.
Technical details
# Prior-review findings assessment
## Affected sites
- apps/typegpu-docs/src/utils/examples/exampleRunner.ts:7-26 — `initializeParam` dispatches `onSelectChange`/`onToggleChange`/... with `param.initial`; `executeExample` calls it for every control.
- apps/typegpu-docs/src/examples/rendering/jelly-knob/motion.ts:77-85 — passes `alpha, beta, gamma` as x, y, z.
## Required outcome
- Do not change the `Quality` initial value or the `motionToWorld(alpha, beta, gamma, ...)` ordering on account of these two findings.( ◦°^°◦) 🔍
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commits pushed since the prior Pullfrog review (c9b87ccf → f2a48984): a cancellation guard for the Auto-quality benchmark, touch-cancel handling, a shared camera-position constant, and a far-plane fix.
- Superseded
Autobenchmark is cancelled —Quality.onSelectChangetags each selection with a monotonicqualityRequestIdand adisposedflag; a lateautoSetQualityresult neither writesqualityScalenor callshandleResize(), and the benchmark loop breaks early once cancelled. This closes the prior inline finding. - Touch cancel handled —
touchendandtouchcancelnow sharehandleTouchEnd, so a cancelled gesture releases the knob. - Camera position centralised — a new
CAMERA_POSITIONconstant replaces the duplicated(0, 2.7, 0.8)literal inindex.tsandmotion.ts, which now derives its basis from it (same math). - Projection
faraligned —CameraController.updateProjectiondefault changed100 → 10, matching the constructor andMAX_DIST = 10. - Unrelated showcase entry dropped — the branch no longer adds
rendering--triplanar-mappingtoADVANCED_EXAMPLE_KEYS. - Tests extended — the lifecycle test now covers the superseded-
Autoand cleanup-cancel paths and restores globals viavi.unstubAllGlobals().
( ◦°^°◦) 🏴☠️
Ahoy! I re-reviewed the fresh commits on the Jelly Knob PR: the Auto-quality race be fixed with a request-id guard, touch-cancel be handled proper, and the camera position be shared twixt modules. No new leaks nor wrecks spotted — she be ready to sail, matey!
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
710a427 to
68c91b1
Compare
b85790f to
4958927
Compare
4958927 to
583043e
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commit pushed since the prior Pullfrog review (f2a48984 → 583043e0). The only authored delta is the default Quality selection; the remaining 17 files are byte-identical to the previously reviewed version.
- Default quality is now
Auto—Quality.initialchanged from'Ultra'to'Auto'injelly-knob/index.ts. On mount the controls runner eagerly invokesonSelectChange('Auto'), which starts the adaptive benchmark and applies its scale (or the0.5fallback whentimestamp-queryis unavailable). The existingqualityRequestId/disposedguard already covers this async completion path, so no lifecycle regression.
Verification: jelly-knob-lifecycle.test.ts 3/3 pass; oxlint --type-aware on the changed files is clean.
( ◦°^°◦) 🏴☠️
Ahoy! I re-reviewed the fresh commit on the Jelly Knob PR: the default quality be now Auto, settin' the adaptive benchmark to run on mount. The old request-id guard still be holdin' her steady — no new leaks nor wrecks spotted, so I gave her the green flag. The prior review's one thread already be resolved, matey!
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
583043e to
c6f9d0d
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commits since the prior Pullfrog review (583043e → c6f9d0d). The one authored code change swaps a precomputed blue-noise texture for a GPU-side implementation; the rest of the range is a rebase onto main.
- GPU-side blue noise replaces the precomputed texture — new
jelly-knob/blueNoise.tsportshash12(Fi-hash white noise) andblue12(high-pass over the 3×3 neighborhood, which simplifies tocenter - avg_neighbors);index.tsnow dithers withblue12(std.floor(input.position.xy))instead of sampling a 64×64blueNoise2dtexture. @typegpu/noiseno longer needed for dither — theblueNoise2dimport is gone (the package is still used forperlin3dinraymarchers.ts), so the example no longer depends on the blue-noise work from the base branch.- Rebase onto
main— the example is byte-identical to the prior review (jelly-knob/treedfd3025d, lifecycle-test blob33c230e, and therendering--jelly-knobregistration all unchanged); onlyblueNoise.tsandindex.tsdiffer.
Verification: jelly-knob-lifecycle.test.ts 3/3 pass; oxlint --type-aware on the two changed files is clean; resolving blue12 through tgpu.resolve emits valid WGSL.
ℹ️ docs/jelly-knob no longer descends from its base branch
docs/jelly-knob was rebased onto main, so it no longer contains the PR base feat/typegpu-noise-blue-noise (755f9a082). GitHub now renders this PR as 329 changed files / +20,716 — essentially all of main since the merge-base (743c3b31) plus the example — even though the PR only authors the jelly-knob example. Since the example no longer uses blueNoise2d from the base branch, retargeting the PR to main (or rebasing onto the current base tip) would keep the diff scoped to the example.
Technical details
# Base branch is no longer an ancestor of the head
## Affected sites
- PR metadata: base `feat/typegpu-noise-blue-noise` (`755f9a082`), head `docs/jelly-knob` (`c6f9d0d`).
- `git merge-base 755f9a082 c6f9d0d` → `743c3b31`; `git rev-list c6f9d0d` does not contain `755f9a082`.
- `apps/typegpu-docs/src/examples/rendering/jelly-knob/` tree `dfd3025d` is identical at `583043e` and `9ad34b1c`; only `blueNoise.ts` and `index.ts` differ at head.
## Required outcome
- The PR diff should reflect only the example's changes, not the intervening `main` history.
## Suggested approach (optional)
- If the example is meant to land independently, change the PR base to `main`.
- Otherwise rebase `docs/jelly-knob` onto the current `feat/typegpu-noise-blue-noise` tip.
## Open questions for the human
- Is the base retarget intended, or was the rebase onto `main` incidental?( ◦°^°◦) 🏴☠️
Ahoy! I re-reviewed the fresh commit on the Jelly Knob PR: the blue-noise texture be swapped for a GPU-side hash and she compiles clean with the tests passin' — but the branch be rebased off main, so the PR shows the whole fleet o' unrelated files; check yer base, matey!
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ No critical issues — one minor note inline.
Reviewed changes
This run re-reviewed the commit pushed since the prior Pullfrog review (709105cb → b1edfc3a); the rest of the PR is unchanged.
- Phone-motion button is now mobile-only —
index.tscomputesisMobilefromDeviceMotionEventavailability plusmatchMedia('(pointer: coarse)')and short-circuits the'Enable Phone Motion'control tofalseelsewhere, so desktop control panels no longer show a button that cannot do anything. This answers the reviewer's request to hide it on PC. - Uses the existing short-circuit idiom — the
falsecontrol value is already supported bydefineControlsand skipped byexampleRunner.addParameters, matchinghasSubgroups && {...}andhdr && {...}in other examples.
( ◦°^°◦) 🏴☠️
Ahoy! I re-reviewed the newest commit on the Jelly Knob PR and spied that the phone-motion button now be hidden on PC as the matey asked — but she still shows on Android where the button does naught, so I noted that one small thing.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
|
I found one place that results in an error, however it seems like there are multiple problematic spots, and it's starting to take too much time. I'll just give up on it working on my phone, and try to report the repro somewhere |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commit pushed since the prior Pullfrog review (b1edfc3a → 35baec24); the rest of the PR is unchanged.
- Phone-motion button now gated on the permission API —
'Enable Phone Motion'uses the newPhoneMotion.needsPermissiongetter (no active listener and arequestPermissionAPI present) alongside the coarse-pointer check, so the control renders only on iOS where a tap is actually required. On Android and desktop the constructor has already started listening, soneedsPermissionisfalseand the button stays hidden. - Prior Android no-op-button thread retired — the inline note from the previous review is addressed by the same change and the thread was resolved.
( ◦°^°◦) 🏴☠️
Ahoy! I re-reviewed the newest commit on the Jelly Knob PR and the phone-motion button now only shows where the waters be askin' for a tap — no dead buttons on Android, matey!
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
35baec2 to
8c49645
Compare
8c49645 to
a3464af
Compare



Screen.Recording.2026-09-06.at.17.31.51.mov