diff --git a/src/components/canvas/players/rich-text-player.ts b/src/components/canvas/players/rich-text-player.ts index 52fb437..56a7ee4 100644 --- a/src/components/canvas/players/rich-text-player.ts +++ b/src/components/canvas/players/rich-text-player.ts @@ -40,7 +40,6 @@ export class RichTextPlayer extends Player { private texture: pixi.Texture | null = null; private sprite: pixi.Sprite | null = null; private lastRenderedTime: number = -1; - private cachedFrames = new Map(); private isRendering: boolean = false; private pendingRenderTime: number | null = null; // Stores time requested while rendering (race condition fix) private currentRender: Promise | null = null; // In-flight render promise, so an off-playback capture can await it @@ -267,10 +266,8 @@ export class RichTextPlayer extends Player { try { await this.prepareFontForAsset(richTextAsset, true); - for (const texture of this.cachedFrames.values()) { - texture.destroy(); - } - this.cachedFrames.clear(); + this.texture?.destroy(); + this.texture = null; this.lastRenderedTime = -1; if (this.textEngine) { @@ -362,17 +359,6 @@ export class RichTextPlayer extends Player { private async renderFrame(timeSeconds: number): Promise { if (!this.textEngine || !this.renderer || !this.canvas || !this.validatedAsset) return; - const cacheKey = Math.floor(timeSeconds * RichTextPlayer.PREVIEW_FPS); - - if (this.cachedFrames.has(cacheKey)) { - const cachedTexture = this.cachedFrames.get(cacheKey)!; - if (this.sprite && this.sprite.texture !== cachedTexture) { - this.sprite.texture = cachedTexture; - } - this.lastRenderedTime = timeSeconds; - return; - } - try { // Pass clip duration so animations can cap their length appropriately const clipDuration = this.getLength(); @@ -383,21 +369,17 @@ export class RichTextPlayer extends Player { await this.renderer.render(ops); - const tex = pixi.Texture.from(this.canvas); - - if (!this.sprite) { - this.sprite = new pixi.Sprite(tex); - this.contentContainer.addChild(this.sprite); + if (!this.texture) { + this.texture = pixi.Texture.from(this.canvas); } else { - if (this.texture && !this.cachedFrames.has(cacheKey)) { - this.texture.destroy(); - } - this.sprite.texture = tex; + this.texture.source.update(); } - this.texture = tex; - if (this.cachedFrames.size < 150) { - this.cachedFrames.set(cacheKey, tex); + if (!this.sprite) { + this.sprite = new pixi.Sprite(this.texture); + this.contentContainer.addChild(this.sprite); + } else if (this.sprite.texture !== this.texture) { + this.sprite.texture = this.texture; } this.lastRenderedTime = timeSeconds; @@ -457,13 +439,6 @@ export class RichTextPlayer extends Player { if (this.isRendering) { // Store pending time to render after current render completes (race condition fix) this.pendingRenderTime = timeSeconds; - - // Show nearest cached frame instead of skipping entirely - const cacheKey = Math.floor(timeSeconds * RichTextPlayer.PREVIEW_FPS); - const cachedTexture = this.cachedFrames.get(cacheKey); - if (cachedTexture && this.sprite && this.sprite.texture !== cachedTexture) { - this.sprite.texture = cachedTexture; - } return; } @@ -541,14 +516,7 @@ export class RichTextPlayer extends Player { super.dispose(); this.loadComplete = false; - for (const texture of this.cachedFrames.values()) { - texture.destroy(); - } - this.cachedFrames.clear(); - - if (this.texture && !this.cachedFrames.has(Math.floor(this.lastRenderedTime * RichTextPlayer.PREVIEW_FPS))) { - this.texture.destroy(); - } + this.texture?.destroy(); this.texture = null; if (this.sprite) { @@ -607,10 +575,8 @@ export class RichTextPlayer extends Player { this.canvas.width = width; this.canvas.height = height; - for (const texture of this.cachedFrames.values()) { - texture.destroy(); - } - this.cachedFrames.clear(); + this.texture?.destroy(); + this.texture = null; this.lastRenderedTime = -1; const canvasPayload = this.buildCanvasPayload(richTextAsset); @@ -630,18 +596,12 @@ export class RichTextPlayer extends Player { this.validatedAsset = validated; } - for (const texture of this.cachedFrames.values()) { - texture.destroy(); - } - this.cachedFrames.clear(); + this.texture?.destroy(); + this.texture = null; this.lastRenderedTime = -1; if (this.textEngine && this.renderer) { this.renderFrameSafe(this.getPlaybackTime()); } } - - public getCacheSize(): number { - return this.cachedFrames.size; - } } diff --git a/src/core/edit-session.ts b/src/core/edit-session.ts index 10083f0..6e7bb4e 100644 --- a/src/core/edit-session.ts +++ b/src/core/edit-session.ts @@ -2116,6 +2116,38 @@ export class Edit { return this.canvas?.getZoom() ?? 1; } + /** + * The clip's on-screen rectangle in CSS pixels, relative to the top-left + * of the canvas element. Returns null when the clip is not visible at the + * current playhead (its container isn't drawn then) — seek to the clip's + * start first. Lets an external overlay anchor UI (e.g. a coach-mark) to a + * clip; the caller adds `getCanvasElement().getBoundingClientRect()` to + * convert to viewport coordinates. Clips animate and move with zoom, so + * recompute per animation frame. + * @internal + */ + public getClipViewportRect(trackIndex: number, clipIndex: number): { left: number; top: number; width: number; height: number } | null { + const player = this.getPlayerClip(trackIndex, clipIndex); + if (!player || !player.isActive() || !this.canvas) return null; + const bounds = player.getContainer().getBounds(); + const resolution = this.canvas.application.renderer?.resolution ?? 1; + return { + left: bounds.x / resolution, + top: bounds.y / resolution, + width: bounds.width / resolution, + height: bounds.height / resolution + }; + } + + /** + * The backing `` DOM element, or null before the canvas is + * attached. Pairs with {@link getClipViewportRect} to place an overlay. + * @internal + */ + public getCanvasElement(): HTMLCanvasElement | null { + return this.canvas?.application.canvas ?? null; + } + /** * Get the viewport container for coordinate transforms. * @internal @@ -2175,6 +2207,17 @@ export class Edit { return this.executeCommand(command); } + /** + * Resolve once all in-flight asset loads have finished, including any that + * begin after the initial `load()`. Await before driving playback or + * capturing a frame so the canvas shows fully-loaded content rather than a + * partially-loaded first frame. + * @internal + */ + public whenSettled(): Promise { + return this.playerReconciler.whenSettled(); + } + /** * Capture the rendered output frame as a base64 data URL. * diff --git a/src/core/shotstack-edit.ts b/src/core/shotstack-edit.ts index cc1f57c..d86ce0d 100644 --- a/src/core/shotstack-edit.ts +++ b/src/core/shotstack-edit.ts @@ -260,6 +260,24 @@ export class ShotstackEdit extends Edit { return false; } + /** + * Track/clip index of the first clip bound to the given merge field, or + * null if none. Pairs with `getClipViewportRect` so an overlay can anchor + * UI to the clip a variable drives (e.g. a coach-mark on `{{ADDRESS}}`). + * @internal + */ + public getMergeFieldClipLocation(fieldName: string): { trackIndex: number; clipIndex: number } | null { + const document = this.getDocument(); + if (!document) return null; + for (const clipId of document.getClipIdsWithBindings()) { + const bindings = document.getClipBindings(clipId); + if (bindings && this.clipUsesField(bindings, fieldName)) { + return this.getClipPositionById(clipId); + } + } + return null; + } + /** * Remove a merge field globally from all clips and the registry. */ @@ -482,10 +500,16 @@ export class ShotstackEdit extends Edit { // Also update document clip data (resolver reads clip data, not bindings). // Use numeric-first coercion (matching the resolver strategy) because the // document clip may still hold a string placeholder like "{{ OPACITY }}". + // EXCEPT for text/html content, which must stay a string — a numeric + // value like "3" bedrooms would otherwise become the number 3 and a + // rich-text/text/html asset can't render a non-string, so the clip + // silently stops updating. if (clipLookup) { + const leaf = path.split(".").pop() ?? ""; + const isTextContent = leaf === "text" || leaf === "html"; const trimmed = typeof newResolvedValue === "string" ? newResolvedValue.trim() : ""; const num = trimmed.length > 0 ? Number(newResolvedValue) : NaN; - const typedValue = Number.isFinite(num) ? num : newResolvedValue; + const typedValue = !isTextContent && Number.isFinite(num) ? num : newResolvedValue; setNestedValue(clipLookup.clip as Record, path, typedValue); } } diff --git a/tests/edit-merge-fields.test.ts b/tests/edit-merge-fields.test.ts index 526009a..a2dbf19 100644 --- a/tests/edit-merge-fields.test.ts +++ b/tests/edit-merge-fields.test.ts @@ -405,6 +405,19 @@ describe("Edit Merge Fields", () => { expect(edit.getMergeFieldForProperty(clipId, "asset.text")).toBe("HEADLINE"); }); + it("keeps a numeric text value a string on live update (does not coerce to number)", async () => { + const clip = createTextClip(0, 3, "4"); + await edit.addClip(0, clip); + const clipId = getClipIdOrFail(edit, 0, 0); + edit.applyMergeField(clipId, "asset.text", "BEDROOMS", "4"); + + edit.updateMergeFieldValueLive("BEDROOMS", "3"); + + const { text } = edit.getPlayerClip(0, 0)?.clipConfiguration.asset as { text: unknown }; + expect(text).toBe("3"); + expect(typeof text).toBe("string"); // not the number 3 — rich-text can't render a non-string + }); + it("is undoable - restores previous value", async () => { const originalSrc = "https://example.com/original.jpg"; const clip = createImageClip(0, 3, originalSrc); @@ -1864,10 +1877,11 @@ describe("Edit Merge Fields", () => { expect(typeof (player?.clipConfiguration.asset as { text: string }).text).toBe("string"); }); - it("coerces negative number string to number", async () => { - // Use asset.text to test coercion behavior since numeric clip properties - // have schema constraints (e.g., start >= 0). The coercion logic itself - // doesn't depend on the property — it applies to all merge field values. + it("does NOT coerce a numeric string on a text property (stays string)", async () => { + // Text/html content must stay a string even when it looks numeric — a + // rich-text asset can't render a number, so coercing "-5" to -5 silently + // blanks the clip. Numeric coercion for real numeric properties is covered + // by the opacity test below. const clip = createTextClip(0, 3, "Placeholder"); await edit.addClip(0, clip); @@ -1877,10 +1891,8 @@ describe("Edit Merge Fields", () => { edit.updateMergeFieldValueLive("VAL", "-5"); const player = edit.getPlayerClip(0, 0); - // The coercion converts "-5" to -5 (number) since Number("-5") is finite. - // On a text property this means the value is stored as -5 (number), not "-5" (string). - expect((player?.clipConfiguration.asset as { text: number | string }).text).toBe(-5); - expect(typeof (player?.clipConfiguration.asset as { text: number | string }).text).toBe("number"); + expect((player?.clipConfiguration.asset as { text: string }).text).toBe("-5"); + expect(typeof (player?.clipConfiguration.asset as { text: string }).text).toBe("string"); }); it("coerces float string to number", async () => { @@ -2140,4 +2152,19 @@ describe("Edit Merge Fields", () => { expect(edit.mergeFields.get("SHADOW_X")).toBeUndefined(); }); }); + + describe("getMergeFieldClipLocation()", () => { + it("returns the track/clip index of the clip bound to a field", async () => { + const clip = createTextClip(0, 3); + await edit.addClip(1, clip); + const clipId = getClipIdOrFail(edit, 1, 0); + await edit.applyMergeField(clipId, "asset.text", "HEADLINE", "New headline"); + + expect(edit.getMergeFieldClipLocation("HEADLINE")).toEqual({ trackIndex: 1, clipIndex: 0 }); + }); + + it("returns null for a field no clip references", () => { + expect(edit.getMergeFieldClipLocation("NONEXISTENT")).toBeNull(); + }); + }); }); diff --git a/tests/rich-text-player-font-caching.test.ts b/tests/rich-text-player-font-caching.test.ts index e4237bd..61f31d9 100644 --- a/tests/rich-text-player-font-caching.test.ts +++ b/tests/rich-text-player-font-caching.test.ts @@ -54,7 +54,8 @@ jest.mock("pixi.js", () => { Container: jest.fn().mockImplementation(createMockContainer), Texture: { from: jest.fn().mockImplementation(() => ({ - destroy: jest.fn() + destroy: jest.fn(), + source: { update: jest.fn() } })), WHITE: {} }, @@ -422,4 +423,21 @@ describe("RichTextPlayer font caching", () => { expect(mockRegisterFontFromFile).toHaveBeenCalledTimes(1); expect(mockFetch).toHaveBeenCalledTimes(1); }); + + it("re-uploads the texture source on every repaint so seek/playback shows the current frame", async () => { + const { player } = await createReadyPlayer(); + const renderFrame = (player as unknown as { renderFrame: (t: number) => Promise }).renderFrame.bind(player); + const { texture } = player as unknown as { texture: { source: { update: jest.Mock } } }; + + // load() created the texture on its frame-0 render; repaints must flag its source dirty. + // pixi.Texture.from(canvas) is source-cached, so without this the GPU keeps the first + // frame (blank fade-in start for animated text) forever — the actual bug being guarded. + expect(texture.source.update).toHaveBeenCalledTimes(0); + + await renderFrame(2.3); + await renderFrame(4.0); + + expect(texture.source.update).toHaveBeenCalledTimes(2); + expect((player as unknown as { texture: unknown }).texture).toBe(texture); + }); });