Skip to content
2 changes: 1 addition & 1 deletion packages/dev/core/src/Misc/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ declare const _native: any;
export function nativeOverride<T extends (...params: any[]) => boolean>(
target: any,
propertyKey: string,
descriptor: TypedPropertyDescriptor<(...params: Parameters<T>) => unknown>,
descriptor: TypedPropertyDescriptor<(...params: Parameters<T>) => any>,
predicate?: T
) {
// Cache the original JS function for later.
Expand Down
120 changes: 60 additions & 60 deletions packages/dev/core/src/Misc/dumpTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import { Clamp } from "../Maths/math.scalar.functions";
import type { AbstractEngine } from "../Engines/abstractEngine";
import { EngineStore } from "../Engines/engineStore";
import { Logger } from "./logger";
import { EncodeArrayBufferToBase64 } from "./stringTools";
import { nativeOverride } from "./decorators";

type DumpResources = {
canvas: HTMLCanvasElement | OffscreenCanvas;
dumpEngine?: {
dumpEngine: {
engine: ThinEngine;
renderer: EffectRenderer;
wrapper: EffectWrapper;
Expand All @@ -28,14 +30,11 @@ async function _CreateDumpResourcesAsync(): Promise<DumpResources> {
Logger.Warn("DumpData: OffscreenCanvas will be used for dumping data. This may result in lossy alpha values.");
}

// If WebGL via ThinEngine is not available (e.g. Native), use the BitmapRenderer.
// If WebGL via ThinEngine is not available, we cannot encode the data.
// If https://github.com/whatwg/html/issues/10142 is resolved, we can migrate to just BitmapRenderer and avoid an engine dependency altogether.
const { ThinEngine: thinEngineClass } = await import("../Engines/thinEngine");
if (!thinEngineClass.IsSupported) {
if (!canvas.getContext("bitmaprenderer")) {
throw new Error("DumpData: No WebGL or bitmap rendering context available. Cannot dump data.");
}
return { canvas };
throw new Error("DumpData: No WebGL context available. Cannot dump data.");
}

const options = {
Expand Down Expand Up @@ -85,6 +84,50 @@ async function _GetDumpResourcesAsync() {
return await ResourcesPromise;
}

class EncodingHelper {
/**
* Encodes image data to the given mime type. If the requested MIME type is not supported, an error will be thrown.
* This is put into a helper class so we can apply the nativeOverride decorator to it.
* @internal
*/
@nativeOverride
public static async EncodeImageAsync(pixelData: Uint8Array, width: number, height: number, mimeType: string, invertY: boolean, quality?: number): Promise<ArrayBuffer> {
const resources = await _GetDumpResourcesAsync();

// Keep the async render + read from the shared canvas atomic
return await new Promise<ArrayBuffer>((resolve, reject) => {
const dumpEngine = resources.dumpEngine;
dumpEngine.engine.setSize(width, height, true);

// Create the image
const texture = dumpEngine.engine.createRawTexture(pixelData, width, height, Constants.TEXTUREFORMAT_RGBA, false, !invertY, Constants.TEXTURE_NEAREST_NEAREST);

dumpEngine.renderer.setViewport();
dumpEngine.renderer.applyEffectWrapper(dumpEngine.wrapper);
dumpEngine.wrapper.effect._bindTexture("textureSampler", texture);
dumpEngine.renderer.draw();

texture.dispose();

Tools.ToBlob(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a ToBlobAsync that is promise returning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could. It wouldn't be as simple as replacing with ToBlob with ToBlobAsync, though, as the canvas draw + toBlob must be atomic. Guess we could use an AsyncLock?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean. We can chat, or if you want to just leave it as is that's ok too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synced offline. My previous comment was probably incorrect, and we should be able to directly replace with something like ToBlobAsync.

My question is-- where to put it? ATM, ToBlob exists as a static method in Tools.

Maybe this can be put off for another PR.

resources.canvas,
(blob) => {
if (!blob) {
reject(new Error("DumpData: Failed to convert canvas to blob."));
} else if (blob.type !== mimeType) {
// The MIME type of a canvas.toBlob() output can be different from the one requested if the browser does not support the requested format
reject(new Error(`DumpData: Failed to convert canvas to blob as ${mimeType}. Got ${blob.type} instead.`));
Copy link
Contributor Author

@alexchuber alexchuber Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change, but I think it makes sense to do, considering the API contract is to just return the binary data-- with the expectation that it matches the requested MIME type.

This change would also enable the changes proposed in #17366.

But there's one case that I'm not sure how to handle: when DumpData is used just to download an image, and the user doesn't care about the return type. I think ScreenshotTools is an example of this.
How should this be handled?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change, but I think it makes sense to do, considering the API contract is to just return the binary data-- with the expectation that it matches the requested MIME type.

I would say this is a bug fix so it's okay.

But there's one case that I'm not sure how to handle: when DumpData is used just to download an image, and the user doesn't care about the return type. I think ScreenshotTools is an example of this.
How should this be handled?

Maybe we can have an optional mimeType and we select the best one in the implementation?

Copy link
Contributor Author

@alexchuber alexchuber Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say this is a bug fix so it's okay.

OK, I'll leave this be. Thanks.

Maybe we can have an optional mimeType and we select the best one in the implementation?

By implementation, do you mean the implementation of ScreenshotTools? If so, it already does something like this, defaulting to PNG if no mime type is supplied.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking in this function since that was my interpretation of your question. Maybe we can sync offline quickly.

} else {
resolve(blob.arrayBuffer());
}
},
mimeType,
quality
);
});
}
}

/**
* Dumps the current bound framebuffer
* @param width defines the rendering width
Expand Down Expand Up @@ -168,63 +211,20 @@ export async function DumpDataAsync(
data = data2;
}

const resources = await _GetDumpResourcesAsync();
const buffer = await EncodingHelper.EncodeImageAsync(data as Uint8Array, width, height, mimeType, invertY, quality);

// Keep the async render + read from the shared canvas atomic
// eslint-disable-next-line no-async-promise-executor
return await new Promise<string | ArrayBuffer>(async (resolve) => {
if (resources.dumpEngine) {
const dumpEngine = resources.dumpEngine;
dumpEngine.engine.setSize(width, height, true);

// Create the image
const texture = dumpEngine.engine.createRawTexture(data, width, height, Constants.TEXTUREFORMAT_RGBA, false, !invertY, Constants.TEXTURE_NEAREST_NEAREST);

dumpEngine.renderer.setViewport();
dumpEngine.renderer.applyEffectWrapper(dumpEngine.wrapper);
dumpEngine.wrapper.effect._bindTexture("textureSampler", texture);
dumpEngine.renderer.draw();

texture.dispose();
} else {
const ctx = resources.canvas.getContext("bitmaprenderer") as ImageBitmapRenderingContext;
resources.canvas.width = width;
resources.canvas.height = height;

const imageData = new ImageData(width, height); // ImageData(data, sw, sh) ctor not yet widely implemented
imageData.data.set(data as Uint8ClampedArray);
const imageBitmap = await createImageBitmap(imageData, { premultiplyAlpha: "none", imageOrientation: invertY ? "flipY" : "from-image" });
if (fileName !== undefined) {
// Note: On web, this creates a Blob -> ArrayBuffer -> Blob round-trip.
// This trade-off keeps the native implementation simple while maintaining
// a consistent interface. I'm hoping the overhead is negligible for typical image sizes.
Tools.DownloadBlob(new Blob([buffer], { type: mimeType }), fileName);
}

ctx.transferFromImageBitmap(imageBitmap);
}
if (toArrayBuffer) {
return buffer;
}

Tools.ToBlob(
resources.canvas,
(blob) => {
if (!blob) {
throw new Error("DumpData: Failed to convert canvas to blob.");
}

if (fileName !== undefined) {
Tools.DownloadBlob(blob, fileName);
}

const fileReader = new FileReader();
fileReader.onload = (event: any) => {
const result = event.target!.result as string | ArrayBuffer;
resolve(result);
};

if (toArrayBuffer) {
fileReader.readAsArrayBuffer(blob);
} else {
fileReader.readAsDataURL(blob);
}
},
mimeType,
quality
);
});
return `data:${mimeType};base64,${EncodeArrayBufferToBase64(buffer)}`;
}

/**
Expand Down
Loading