Added Texture Blitting Utility #6852
Conversation
There was a problem hiding this comment.
Thanks for the PR!
Overall structure is good, have a few comments.
We also need a test to make sure things are working correctly. Our tests are in tests/tests and you make a module in the file root.rs. What we need to test is that things pass validation with both kinds of FilterMode. We don't need to actually test the result.
Co-authored-by: Connor Fitzgerald <[email protected]>
Co-authored-by: Connor Fitzgerald <[email protected]>
Co-authored-by: Connor Fitzgerald <[email protected]>
AbdullahElsheshtawy
force-pushed
the
trunk
branch
from
77d3cbd to
7fff9cd
Compare
Should be all good now. |
wgpu/src/util/texture_blitter.rs
Outdated
| compilation_options: PipelineCompilationOptions::default(), | ||
| targets: &[Some(ColorTargetState { | ||
| format, | ||
| blend: None, |
There was a problem hiding this comment.
Very cool. Configuring blending might also be nice
There was a problem hiding this comment.
Yeah this would be nice.
There was a problem hiding this comment.
Implemented and tested.
|
Looks like emscripten does not support wgsl (atleast in ci). Is this intentional? |
|
This actually has nothing to do with emscripten, it seems like that is the only platform that has a compile job that tests without the |
| pub fn new( | ||
| device: &Device, | ||
| format: TextureFormat, | ||
| sample_type: FilterMode, | ||
| blend_state: Option<BlendState>, | ||
| ) -> Self { |
There was a problem hiding this comment.
I feel like this is an easy way to get a million arguments - there are a bunch of reasonable additions we could add to this blitting utility. I would suggest that we build a Builder-Pattern constructor for this. Specifically:
- Have a
TextureBlitter::new(&Device, TextureFormat)which callsTextureBlitterBuilder::new(&Device, TextureFormat).build(). - Then have methods on the builder which allow setting sample_type and blend_state.
Then we can add functionality here without needing to break users.
There was a problem hiding this comment.
Was late because exams. Hope I didn't cause any inconvenience.
Regarding the TextureBlitterBuilder and TextureBlitter can you elaborate on how the user interface would be like? I don't think I quite get what you mean. Do you mean something like this
use wgpu::util::TextureBlitterBuilder;
let blitter = TextureBlitterBuilder::new(&device, format).sample_type(type).blend_state(blend_state).build();or do you want something like this
use wgpu::util::TextureBlitter;
let blitter = TextureBlitter::new(&device, format).blend_state(blend_state).sample_type(type).build(); Documentation! Co-authored-by: Connor Fitzgerald <[email protected]>
Connections
#6819
Checklist
cargo fmt.cargo clippy.CHANGELOG.md. See simple instructions inside file.