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

Implement copyExternalImageToTexture #2781

Merged

Conversation

ybiletskyi
Copy link
Contributor

Connections
A few requests in matrix chat.

Description
This method allow to improve performance of web page, because creating an ImageBitmap causes the image data to be decoded into a GPU-friendly format, and it does so off the main thread. link

Testing
I created an example locally to test the new method.
Let me know if I should push it.

@jimblandy
Copy link
Member

Yes, let's include the example, if it's not too large.

@maxammann
Copy link
Contributor

Is it possible to test this using wasm? I'm not sure how to run the examples for Web.

@ybiletskyi
Copy link
Contributor Author

Is it possible to test this using wasm? I'm not sure how to run the examples for Web.

Yes. You should to run cargo run-wasm --example hello-copy-source

@maxammann
Copy link
Contributor

I kind of failed at testing this. Firefox Nightly does not yet support this. The dev branch of google chrome currently fails to execute any WebGPU demo for me.

I think this feature should also be supported with WebGL. The following command fails for me:

$ cargo run-wasm --example hello-copy-source --features webgl
   Compiling wgpu v0.12.0 (/home/max/projects/wgpu/wgpu)
error[E0599]: no method named `copy_external_image_to_texture` found for struct `Queue` in the current scope
   --> wgpu/examples/hello-copy-source/main.rs:189:19
    |
189 |             queue.copy_external_image_to_texture(&image, texture.as_image_copy(), texture_size);
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `Queue`

For more information about this error, try `rustc --explain E0599`.

@ybiletskyi
Copy link
Contributor Author

ybiletskyi commented Jun 28, 2022

I kind of failed at testing this. Firefox Nightly does not yet support this. The dev branch of google chrome currently fails to execute any WebGPU demo for me.

I think this feature should also be supported with WebGL. The following command fails for me:

$ cargo run-wasm --example hello-copy-source --features webgl
   Compiling wgpu v0.12.0 (/home/max/projects/wgpu/wgpu)
error[E0599]: no method named `copy_external_image_to_texture` found for struct `Queue` in the current scope
   --> wgpu/examples/hello-copy-source/main.rs:189:19
    |
189 |             queue.copy_external_image_to_texture(&image, texture.as_image_copy(), texture_size);
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `Queue`

For more information about this error, try `rustc --explain E0599`.

I tested this changes on the Chrome Canary version. I probably add WebGL support, but I prefer to do it in separate pull request to minimize changes for review.

wgpu/examples/hello-copy-source/main.rs Outdated Show resolved Hide resolved
wgpu/examples/hello-copy-source/main.rs Outdated Show resolved Hide resolved
@anlumo
Copy link
Contributor

anlumo commented Jul 3, 2022

#1888

@maxammann
Copy link
Contributor

Whats the status of this? :)

@ybiletskyi
Copy link
Contributor Author

@maxammann I have resolved conflicts and changes according PR comments. So probably need to await reviewers approve.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

I still have maintainability concerns about this getting it's own example, but we can tackle that later, I want to get this into the release

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) October 5, 2022 20:57
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

I have multiple concerns about the example - I don't want to merge the example right now - but I do want to get the core functionality in for this release. I have pushed a change to remove the example from this PR - but I want to continue iterating on the example in another PR!

@cwfitzgerald cwfitzgerald merged commit 50e2a5b into gfx-rs:master Oct 5, 2022
@ybiletskyi ybiletskyi deleted the support-copy-external-image-to-texture branch October 7, 2022 09:59
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.

None yet

7 participants