diff --git a/.changeset/spryte-target-feedback.md b/.changeset/spryte-target-feedback.md new file mode 100644 index 00000000..d9e005ba --- /dev/null +++ b/.changeset/spryte-target-feedback.md @@ -0,0 +1,5 @@ +--- +"spryte": patch +--- + +A render target refilled with `draw.into()` after it was drawn stays filled. Its texture could still be bound to a texture unit from the earlier draw (or from its own creation), so the next draw into it was a feedback loop: WebGL rejected it and logged `GL_INVALID_OPERATION ... Feedback loop formed between Framebuffer and active Texture`, and the target stayed blank. Switching to a target now rebinds the 8 sprite units first. The same loop hit frames of shapes only under `blendSpace: "linear"`, whose sRGB framebuffer texture stayed bound after `end()` presented it; it no longer does. Reported in #118. diff --git a/docs/decisions.md b/docs/decisions.md index db18b689..55a8e843 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -155,3 +155,7 @@ Papercut #26: a full-screen vignette drawn as a rect inside `withShader` seemed ## An audio stream's stop() is final From 0.31.1 `audio.stream()` tracks one state (playing, paused or stopped) instead of a `wanted` flag. `stop()` unloads the element, so `resume()` after it does nothing; call `stream()` again to play the track afresh. Before, `resume()` during a `stop(fade)` restarted playback that the pending unload then cut off, and after the unload it armed a retry that could never succeed. Considered: letting `resume()` cancel a pending fade-out (needs the gain restored and the unload timer cancelled, and gives stop two meanings). A blocked `play()` now waits for the context unlocking or the next pointer or key gesture, with one wait armed at most: media elements have their own autoplay rules, so the context can already be running while the element is still blocked. + +## spryte rebinds all 8 sprite units when drawing into a target + +#118: a target refilled with `draw.into()` every frame and also drawn every frame stayed blank, with a feedback-loop error per draw. The earlier draw left the target's texture bound to a texture unit, and every spryte program samples all 8 sprite units, so the next draw into the target read the texture it wrote. From 0.16 `setTarget` binds the white texture to all 8 units after binding a target's framebuffer, and the linear-light framebuffer's texture no longer stays on unit 0 after it is created or presented. Considered: the issue's proposal, rebinding only the units that `units` records as holding the target. `units` records what sprite batches bound, but uploads, `copy()`, `copies()`, static layers and a target's creation bind units without recording it, and those can run mid-batch, where updating the record would break the pending batch. A full mirror of every unit's binding would touch all of those paths. Rebinding all 8 costs 16 GL calls per `into()`, none on the per-sprite path. diff --git a/packages/spryte/browser-test/checks.ts b/packages/spryte/browser-test/checks.ts index 56f6b8e0..9afd732e 100644 --- a/packages/spryte/browser-test/checks.ts +++ b/packages/spryte/browser-test/checks.ts @@ -76,6 +76,14 @@ async function render(scene: (draw: Draw) => void, backend: "webgl2" | "2d" = "w return out; } +/** readPixels rows are bottom first: flip them to compare with `render`. */ +function flipRows(out: Uint8Array): Uint8Array { + const row = W * 4; + const flipped = new Uint8Array(out.length); + for (let y = 0; y < H; y++) flipped.set(out.subarray((H - 1 - y) * row, (H - y) * row), y * row); + return flipped; +} + function diff(a: Uint8Array, b: Uint8Array): number { let n = 0; for (let i = 0; i < a.length; i += 4) if (a[i] !== b[i] || a[i + 1] !== b[i + 1] || a[i + 2] !== b[i + 2] || a[i + 3] !== b[i + 3]) n++; @@ -1794,4 +1802,86 @@ function hostCanvas(): WebGL2RenderingContext { } } +// A target refilled with into() every frame and drawn every frame: the previous frame's draw left the target's +// texture on a texture unit, and drawing into it while bound is a feedback loop that WebGL rejects (#118). +{ + type Target = ReturnType; + const pos1 = new Float32Array([30, 15]); + const fillRect = (draw: Draw) => draw.rect(0, 0, 16, 16, "#cffb2d"); + const fillPolygon = (draw: Draw) => polygon(draw, [0, 0, 16, 0, 16, 16, 0, 16], "#cffb2d"); + // The image first puts the target on unit 1, which a batch of one texture doesn't rebind. + const afterImage = (draw: Draw, t: Target) => { + draw.sprite(img, 4, 4); + draw.sprite(t, 30, 15, { origin: [0, 0] }); + }; + const alone = (draw: Draw) => draw.rect(30, 15, 16, 16, "#cffb2d"); + const withImage = (draw: Draw) => { + draw.sprite(img, 4, 4); + alone(draw); + }; + for (const [how, fill, show, direct] of [ + ["sprite after an image, refilled by a rect", fillRect, afterImage, withImage], + ["sprite after an image, refilled by a polygon", fillPolygon, afterImage, withImage], + ["sprite, refilled by a polygon", fillPolygon, (draw: Draw, t: Target) => draw.sprite(t, 30, 15, { origin: [0, 0] }), alone], + ["sprites, refilled by a polygon", fillPolygon, (draw: Draw, t: Target) => draw.sprites(t, pos1, 1, { origin: [0, 0] }), alone], + ] as const) { + const frames = async (n: number): Promise<{ pixels: Uint8Array; error: number }> => { + const canvas = document.createElement("canvas"); + canvas.width = W; + canvas.height = H; + const draw = createRenderer(canvas, { backend: "webgl2", smooth: false }); + await draw.ready; + draw.resize(W, H); + const t = draw.createTarget(16, 16); + const raw = draw.raw; + if (raw.kind !== "webgl2") throw new Error("expected webgl2"); + let error = 0; + for (let i = 0; i < n; i++) { + draw.begin("#102030", W / 2, H / 2, 2); + draw.into(t, () => fill(draw)); + show(draw, t); + draw.end(); + error ||= raw.gl.getError(); + } + const pixels = new Uint8Array(W * H * 4); + raw.gl.readPixels(0, 0, W, H, raw.gl.RGBA, raw.gl.UNSIGNED_BYTE, pixels); + draw.dispose(); + raw.gl.getExtension("WEBGL_lose_context")?.loseContext(); + return { pixels, error }; + }; + const thrice = await frames(3); + const expected = await render(direct); + check(`into(t) every frame, drawing t with ${how}: no GL error`, thrice.error === 0, `0x${thrice.error.toString(16)}`); + record(`into(t) every frame, drawing t with ${how} == drawn directly`, flipRows(thrice.pixels), expected); + } +} + +// The same loop under blendSpace "linear": each frame is drawn into a framebuffer whose texture end() samples to +// present it. A frame of shapes only (no batch unit rebound) drew into it while it was still bound. +{ + const canvas = document.createElement("canvas"); + canvas.width = W; + canvas.height = H; + const draw = createRenderer(canvas, { backend: "webgl2", smooth: false, blendSpace: "linear" }); + await draw.ready; + draw.resize(W, H); + const raw = draw.raw; + let error = 0; + const pixel = new Uint8Array(4); + if (raw.kind === "webgl2") { + for (let i = 0; i < 3; i++) { + draw.begin("#102030", W / 2, H / 2, 2); + polygon(draw, [0, 0, 16, 0, 16, 16, 0, 16], "#cffb2d"); + draw.end(); + error ||= raw.gl.getError(); + } + // Inside the polygon: 8, 8 in the view is 16, 16 on the canvas, read from the bottom row up. + raw.gl.readPixels(16, H - 17, 1, 1, raw.gl.RGBA, raw.gl.UNSIGNED_BYTE, pixel); + } + draw.dispose(); + if (raw.kind === "webgl2") raw.gl.getExtension("WEBGL_lose_context")?.loseContext(); + check("frames of shapes only under blendSpace linear: no GL error", error === 0, `0x${error.toString(16)}`); + check("frames of shapes only under blendSpace linear: the shape is drawn", pixel[1] > 240, `${pixel.join(",")}`); +} + (window as unknown as { __checks: unknown }).__checks = results; diff --git a/packages/spryte/src/webgl2.ts b/packages/spryte/src/webgl2.ts index 86634e34..f42e30c6 100644 --- a/packages/spryte/src/webgl2.ts +++ b/packages/spryte/src/webgl2.ts @@ -778,7 +778,9 @@ export class WebGL2Backend implements Backend { gl.bindFramebuffer(gl.FRAMEBUFFER, fbo); gl.framebufferTexture2D(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.TEXTURE_2D, tex, 0); this.out = { fbo, tex, w, h }; - // Unit 0 now holds this texture; drop the batch's record of what it held. + // Not left on unit 0: the frame draws into this texture, and sampling it meanwhile is a feedback loop (#118). + gl.bindTexture(gl.TEXTURE_2D, this.white.tex); + // Unit 0 no longer holds what the batch bound there; drop the batch's record of it. this.units[0] = null; this.lastSrc = null; return fbo; @@ -807,6 +809,8 @@ export class WebGL2Backend implements Backend { gl.bindTexture(gl.TEXTURE_2D, out.tex); gl.bindSampler(0, this.nearest); gl.drawArrays(gl.TRIANGLE_STRIP, 0, 4); + // The next frame draws into out.tex again: it can't stay bound (#118). + gl.bindTexture(gl.TEXTURE_2D, this.white.tex); gl.enable(gl.BLEND); gl.useProgram(this.active.program); gl.bindVertexArray(this.vao); @@ -1533,6 +1537,14 @@ export class WebGL2Backend implements Backend { gl.bindFramebuffer(gl.FRAMEBUFFER, this.targetStore(target).fbo); gl.viewport(0, 0, target.width, target.height); this.outH = target.height; + // The target's texture may still be bound to a unit: by the batch that last drew it, a copy, a layer or its + // own creation. Every program samples all the units, so the next draw would read the texture it writes, a + // feedback loop WebGL rejects (#118). Only some of those bindings are tracked: rebind them all. + const white = this.white.tex; + for (let i = UNITS - 1; i >= 0; i--) { + gl.activeTexture(gl.TEXTURE0 + i); + gl.bindTexture(gl.TEXTURE_2D, white); + } } else this.bindOutput(); this.target = target; this.viewW = width; diff --git a/packages/spryte/test/webgl2.test.ts b/packages/spryte/test/webgl2.test.ts index 3206dd45..26e211e9 100644 --- a/packages/spryte/test/webgl2.test.ts +++ b/packages/spryte/test/webgl2.test.ts @@ -67,6 +67,30 @@ function fakeGL(active: { name: string; type: string; size?: number }[] = []) { const M: Matrix = [16, 0, 0, 16, 0, 0]; const image = (w = 16, h = 16) => ({ width: w, height: h }) as unknown as TextureSource; +/** + * Replay the recorded calls and count the draws made while one of the 8 sprite units (which every program samples) + * held the texture of the framebuffer being drawn into: a feedback loop, which WebGL rejects. + */ +function feedbackDraws(calls: { name: string; args: unknown[] }[]): number { + const units = new Map(); + const attached = new Map(); + let unit: unknown = "TEXTURE00"; + let framebuffer: unknown = null; + let loops = 0; + for (const { name, args } of calls) { + // The fake's constants are strings: unit i is "TEXTURE0" + i, and plain TEXTURE0 is unit 0 too. + if (name === "activeTexture") unit = args[0] === "TEXTURE0" ? "TEXTURE00" : args[0]; + else if (name === "bindTexture") units.set(unit, args[1]); + else if (name === "bindFramebuffer") framebuffer = args[1]; + else if (name === "framebufferTexture2D") attached.set(framebuffer, args[3]); + else if (/^draw(Arrays|Elements)/.test(name)) { + const into = attached.get(framebuffer); + for (let i = 0; i < 8; i++) if (into !== undefined && units.get(`TEXTURE0${i}`) === into) loops++; + } + } + return loops; +} + describe("WebGL2Backend", () => { it("uses highp texture coordinates so large atlases sample the right texel on mobile GPUs", () => { const { gl, shaderSources } = fakeGL(); @@ -792,6 +816,30 @@ describe("WebGL2Backend", () => { expect([info[0], info[1], info[2]]).toEqual([0.1, -0.1, 1].map(Math.fround)); }); + it("never draws into a target while a sprite unit still holds its texture (#118)", () => { + const { gl, calls } = fakeGL(); + const draw = new Draw(new WebGL2Backend(gl), false); + const t = draw.createTarget(16, 16); + const hero = image(); + const at = new Float32Array([30, 15]); + for (let frame = 0; frame < 3; frame++) { + draw.begin("#000", 100, 50, 1); + // Shapes only, so the batch rebinds no unit, then a rect, which binds unit 0 only. + draw.into(t, () => roundRect(draw, 0, 0, 16, 16, 2, "#fff")); + draw.into(t, () => draw.rect(0, 0, 16, 16, "#fff")); + // The image first puts the target on unit 1; sprites() binds it to unit 0 on its own. + draw.sprite(hero, 0, 0); + draw.sprite(t, 30, 15); + draw.end(); + draw.begin("#000", 100, 50, 1); + draw.into(t, () => roundRect(draw, 0, 0, 16, 16, 2, "#fff")); + draw.sprites(t, at, 1); + draw.end(); + } + expect(calls.some((c) => c.name === "framebufferTexture2D")).toBe(true); + expect(feedbackDraws(calls)).toBe(0); + }); + it("refuses to draw a target into itself, or dispose it while drawing into it", () => { const { gl } = fakeGL(); const draw = new Draw(new WebGL2Backend(gl), false); @@ -888,6 +936,18 @@ describe("blendSpace linear", () => { return f; }; + it("never leaves the frame's own texture on a unit while drawing into it (#118)", () => { + const { gl, calls } = linearGL(); + const draw = new Draw(new WebGL2Backend(gl), false); + for (let frame = 0; frame < 3; frame++) { + draw.begin("#000", 100, 50, 1); + roundRect(draw, 0, 0, 16, 16, 2, "#fff"); + draw.end(); + } + expect(calls.some((c) => c.name === "framebufferTexture2D")).toBe(true); + expect(feedbackDraws(calls)).toBe(0); + }); + it("defines SPRYTE_LINEAR only on linear contexts, and gives every shader spryte_toLinear", () => { const plain = fakeGL(); new WebGL2Backend(plain.gl);