Skip to content

Commit 351c7bf

Browse files
fix(shader-transitions): address Copilot round-2 review
Three follow-up fixes from the Copilot review on commit 8cad217: 1. Use strict `t.shader === undefined` instead of `!t.shader` (Copilot c4) in both the WebGL program compile loop and the page-side compositor. An empty-string `shader: ""` from a vanilla-JS caller (the IIFE bundle is hand-loaded via <script> tags in user HTML) should reach the shader registry and surface a loud "unknown shader" error, not silently degrade to a crossfade. 2. Graceful degradation when shader compile fails (Copilot c5). The previous `continue` dropped the transition from `cachedTransitions`, which also dropped its scene-visibility timeline entries and broke scene progression. Now: log a warning and downgrade to the CSS crossfade fallback (prog=null, fallback=true) so the opacity timeline still runs and the composition keeps playing. 3. Preserve index-to-scene-pair correlation when calling the page-side compositor (Copilot c6). The earlier filter `transitions.filter(t => !!t.shader)` shifted indices, so a shader transition at original index 2 (sitting between CSS crossfades) would be paired with scenes[1] and scenes[2] inside `installPageSideCompositor` instead of the correct scenes[2] and scenes[3]. The compositor now accepts the full array, makes `PageCompositeTransitionConfig.shader` optional, and skips CSS-only entries internally while keeping `transitions[i]` aligned with `scenes[i]`/`scenes[i+1]`. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 8cad217 commit 351c7bf

2 files changed

Lines changed: 40 additions & 16 deletions

File tree

packages/shader-transitions/src/engineModePageComposite.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,14 @@ import { isHtmlInCanvasCaptureSupported } from "./capture.js";
4242

4343
interface PageCompositeTransitionConfig {
4444
time: number;
45-
shader: ShaderName;
45+
/**
46+
* Shader id. Undefined entries are CSS crossfades — the page-side
47+
* compositor skips them so the GSAP opacity timeline handles the blend,
48+
* but the entry stays in the array to preserve `transitions[i]` ↔
49+
* `scenes[i]`/`scenes[i+1]` index alignment for the surrounding shader
50+
* entries.
51+
*/
52+
shader?: ShaderName;
4653
duration?: number;
4754
}
4855

@@ -114,6 +121,10 @@ export function installPageSideCompositor(options: PageCompositorInstallOptions)
114121

115122
const programs = new Map<string, WebGLProgram>();
116123
for (const t of transitions) {
124+
// CSS crossfade entries (shader undefined) carry no program. Use a
125+
// strict undefined check so a misconfigured empty string still fails
126+
// loudly through the createProgram path below.
127+
if (t.shader === undefined) continue;
117128
if (programs.has(t.shader)) continue;
118129
try {
119130
programs.set(t.shader, createProgram(gl, getFragSource(t.shader)));
@@ -127,6 +138,10 @@ export function installPageSideCompositor(options: PageCompositorInstallOptions)
127138
for (let i = 0; i < transitions.length; i++) {
128139
const t = transitions[i];
129140
if (!t) continue;
141+
// CSS-only transitions stay on the GSAP opacity timeline; the page-
142+
// side compositor only handles shader entries. Index i is preserved
143+
// so subsequent shader transitions still pair with the right scenes.
144+
if (t.shader === undefined) continue;
130145
const fromSceneId = scenes[i];
131146
const toSceneId = scenes[i + 1];
132147
const prog = programs.get(t.shader);

packages/shader-transitions/src/hyper-shader.ts

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,11 @@ export function init(config: HyperShaderConfig): GsapTimeline {
903903

904904
const programs = new Map<string, WebGLProgram>();
905905
for (const t of transitions) {
906-
if (!t.shader) continue; // CSS-only transitions have no WebGL program
906+
// Strict undefined check — an explicit empty string from a vanilla-JS
907+
// caller (the IIFE bundle is hand-loaded via <script> tags) should NOT
908+
// be silently coerced into a CSS crossfade. The shader registry will
909+
// throw a clear "unknown shader" error for it.
910+
if (t.shader === undefined) continue;
907911
if (!programs.has(t.shader)) {
908912
try {
909913
programs.set(t.shader, createProgram(gl, getFragSource(t.shader)));
@@ -1302,11 +1306,19 @@ export function init(config: HyperShaderConfig): GsapTimeline {
13021306
const toId = scenes[i + 1];
13031307
if (!fromId || !toId) continue;
13041308

1305-
// CSS-only transition when shader is omitted — uses the fallback opacity
1306-
// crossfade path. No WebGL program or texture prewarming needed.
1307-
const isCssFallback = !t.shader;
1308-
const prog = isCssFallback ? null : (programs.get(t.shader!) ?? null);
1309-
if (!isCssFallback && !prog) continue; // shader requested but not compiled
1309+
// shader omitted → CSS crossfade. shader present but program failed to
1310+
// compile (logged above) → degrade gracefully to CSS crossfade so the
1311+
// opacity timeline still runs and scene progression isn't broken. Both
1312+
// paths land in the always-ready prog=null cache.
1313+
const requestedShader = t.shader !== undefined;
1314+
const compiledProg = requestedShader ? (programs.get(t.shader!) ?? null) : null;
1315+
const isCssFallback = !requestedShader || compiledProg === null;
1316+
if (requestedShader && compiledProg === null) {
1317+
console.warn(
1318+
`[HyperShader] Shader "${t.shader}" failed to compile — falling back to CSS crossfade.`,
1319+
);
1320+
}
1321+
const prog = isCssFallback ? null : compiledProg;
13101322

13111323
const dur = t.duration ?? DEFAULT_DURATION;
13121324
const ease = t.ease ?? DEFAULT_EASE;
@@ -1322,7 +1334,7 @@ export function init(config: HyperShaderConfig): GsapTimeline {
13221334
frames: [],
13231335
cacheKey: "",
13241336
dirty: !isCssFallback,
1325-
ready: isCssFallback, // CSS fallback needs no prewarming
1337+
ready: isCssFallback,
13261338
fallback: isCssFallback,
13271339
persisted: isCssFallback,
13281340
textureReady: false,
@@ -2274,16 +2286,13 @@ function initEngineMode(
22742286
const rawH = Number(root?.getAttribute("data-height"));
22752287
const compWidth = Number.isFinite(rawW) && rawW > 0 ? rawW : 1920;
22762288
const compHeight = Number.isFinite(rawH) && rawH > 0 ? rawH : 1080;
2277-
// Page-side compositing only handles WebGL shader transitions. CSS
2278-
// crossfades are driven by GSAP opacity timelines elsewhere, so filter
2279-
// them out — passing them in would break the compositor's required
2280-
// `shader` field and produce a dead transition window with no rendering.
2281-
const shaderTransitions = transitions.filter(
2282-
(t): t is TransitionConfig & { shader: ShaderName } => !!t.shader,
2283-
);
2289+
// Pass the full transitions array so transition[i] still pairs with
2290+
// scenes[i]/scenes[i+1]. The compositor itself skips entries with
2291+
// `shader === undefined` while preserving the index↔scene mapping.
2292+
// (CSS crossfades remain driven by the GSAP opacity timeline.)
22842293
installPageSideCompositor({
22852294
scenes,
2286-
transitions: shaderTransitions,
2295+
transitions,
22872296
bgColor,
22882297
accentColors,
22892298
width: compWidth,

0 commit comments

Comments
 (0)