Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(web): ImageBitmap #21898

Merged
merged 26 commits into from
Jan 22, 2024
Merged

feat(web): ImageBitmap #21898

merged 26 commits into from
Jan 22, 2024

Conversation

crowlKats
Copy link
Member

@crowlKats crowlKats commented Jan 11, 2024

implements the ImageBitmap class and createImageBitmap function from https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html.

Prerequisite for #5701

This API by itself is useless without OffscreenCanvas and/or integration into WebPGU, since accessing the data of an ImageBitmap directly is not possible.

  • enable some WPTs no WPTs can be enabled due to all relevant WPTs requiring OffscreenCanvas
  • typings
  • lazy load

ext/canvas/01_image.js Outdated Show resolved Hide resolved
Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

Woah, nice!

ImageBitmap can integrate with WebGPU's copyExternalImageToTexture - very useful for texture rendering.

@crowlKats
Copy link
Member Author

@littledivy yes, however this new "canvas" extension will depend on the webgpu extension, so we will need to be careful when adding support for such, and will be tricky especially since webgpu implementation also lives in wgpu.
definitively a usecase to consider, however I'd say lets focus first on the direct usecase of OffscreenCanvas and investigate this later on.

# Conflicts:
#	Cargo.toml
#	ext/web/16_image_data.js
#	runtime/build.rs
#	runtime/js/98_global_scope_shared.js
@bartlomieju bartlomieju added this to the 1.40 milestone Jan 11, 2024
Comment on lines +38 to +94
const { op_lazy_load_esm } = core.ensureFastOps(true);
let image;

function ImageNonEnumerable(getter) {
let valueIsSet = false;
let value;

return {
get() {
loadImage();

if (valueIsSet) {
return value;
} else {
return getter();
}
},
set(v) {
loadImage();

valueIsSet = true;
value = v;
},
enumerable: false,
configurable: true,
};
}
function ImageWritable(getter) {
let valueIsSet = false;
let value;

return {
get() {
loadImage();

if (valueIsSet) {
return value;
} else {
return getter();
}
},
set(v) {
loadImage();

valueIsSet = true;
value = v;
},
enumerable: true,
configurable: true,
};
}
function loadImage() {
if (!image) {
image = op_lazy_load_esm("ext:deno_canvas/01_image.js");
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

quite a bit duplicate with webgpu. we should look into having a better solution that requires less setup for each lazy loaded file

Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

Are there WPT we can enable or unit tests that can be added?

@crowlKats
Copy link
Member Author

crowlKats commented Jan 19, 2024

@littledivy from the PR description:

no WPTs can be enabled due to all relevant WPTs requiring OffscreenCanvas

I'll some unit tests though

Comment on lines +47 to +48
let view =
RgbaImage::from_vec(args.width, args.height, buf.to_vec()).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

The full allocation here will hurt performance. I think you can create a view into the JS slice:

 let view: ImageBuffer<Rgba<u8>, &[u8]> = ImageBuffer::from_raw(args.witdh, args.height, buf).unwrap();

Copy link
Member Author

Choose a reason for hiding this comment

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

sadly I attempted this already, but it wont accept a slice no matter what (even your code, oddly enough, errors out with it wanting a vec), which is weird given that the docs for from_raw state that a slice can be passed.

ext/canvas/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM, nice work

Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants