-
Notifications
You must be signed in to change notification settings - Fork 344
Description
The current spec says
When an
HTMLCanvasElementorOffscreenCanvas|canvas| with a
GPUCanvasContext|context| has itswidthorheightproperties modified,
update the canvas size:
- Replace the drawing buffer of context.
- ...
This seems less than ideal. The new drawing buffer will not be used until
the next call to context.getCurrentTexture() so I'd like to suggest the
spec should change to something to the effect of
1. Record the new drawing buffer size
...
And then at getCurrentTexture(), a texture of the recorded size should be created
In effect, changing the size of a webgpu canvas should only record the size, not actually cause any other operations to happen (maybe at compositing time something happens?)
The spec shows this code currently
const canvas = document.createElement('canvas');
const context = canvas.getContext('webgpu');
const resizeObserver = new ResizeObserver(entries => {
for (const entry of entries) {
if (entry.target != canvas) { continue; }
canvas.width = entry.devicePixelContentBoxSize[0].inlineSize;
canvas.height = entry.devicePixelContentBoxSize[0].blockSize;
}
});
resizeObserver.observe(canvas);The code above provides a bad experience, at least on Chrome. The reason seems to be because of the requirement that setting the width and height of the canvas do something immediately (step 1, replace the drawing buffer).
Chrome might be able to fix this issue without changing the spec but it seems like it would be nice if the spec tried to make it more explicit that the code above is best practice. As it is, it's not best practice.
To see the issue, imagine you have a rAF loop. If you follow the code above, when the user resizes the page, you'll get this behavior
Screen.Recording.2023-11-06.at.15.19.00.mov
The workaround is to only record what you want the size to be but don't actually change the size until just before you call getCurrentTexture AND, only if the size has changed. This turns out to be a little tedious
const canvas = document.createElement('canvas');
const context = canvas.getContext('webgpu');
const desiredCanvasSize = {
width: canvas.width,
height: canvas.height,
};
function render() {
// Only change the width and height of canvas if they are different
// because some browsers this is a heavy operation. Since the spec doesn't
// prevent this from being a heavy operation we have to be pro-active and
// assume it is
if (canvas.width !== desiredCanvasSize.width ||
canvas.height !== desiredCanvasSize.height) {
canvas.width = desiredCanvasSize.width;
canvas.height = desiredCanvasSize.height;
}
const texture = context.getCurrentTexture();
requestAnimationFrame(render);
}
requestAnimationFrame(render);
const resizeObserver = new ResizeObserver(entries => {
for (const entry of entries) {
if (entry.target != canvas) { continue; }
// Only record the new size here because setting the
// canvas size itself here is problematic.
desiredCanvasSize.width = entry.devicePixelContentBoxSize[0].inlineSize;
desiredCanvasSize.height = entry.devicePixelContentBoxSize[0].blockSize;
}
});
resizeObserver.observe(canvas);Now the behavior when the user resizes is like this
Screen.Recording.2023-11-06.at.15.19.26.mov
I'm curious if we could change the spec so setting the canvas width
and height only record the new size and then nothing else happens until
some operation that requires the new width and height. Examples would be
- calling
getCurrentTexture - calling
toDataURL,toBlob - passing the canvas to
drawImage,texImage2D,copyExternalImageToTexture
I'm also curious if it would be bad if these functions,
(toDataURL, toBlob, drawImage, texImage2D, copyExternalImageToTexture)
all used the previous texture and that the only thing that actually changed
textures was getCurrentTexture
Conversely, we could change the example in the spec to explicitly call out that it provides a bad experience. It seems like it would be nice to not have to make the user jump through hoops and guess what they'll need to do (1) record the size (2) don't set it if it's the same and instead, just update the spec so the code that's in their currently is, as much as possible, specified to be best practice.