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

Expose &Texture to users via RenderGraphDataStore #462

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

setzer22
Copy link
Contributor

@setzer22 setzer22 commented Jan 21, 2023

Checklist

  • CI Checked:
    • cargo fmt has been ran
    • cargo clippy reports no issues
    • cargo test succeeds
    • cargo rend3-doc has no warnings
    • cargo deny check issues have been fixed or added to deny.toml
  • Manually Checked:
    • relevant examples/test cases run
    • changes added to changelog
      • Add credit to yourself for each change: Added new functionality @githubname.

Related Issues

None that I know of, but there's a related discussion on Matrix: https://matrix.to/#/!telLUrJoXXaQkubDie:matrix.org/$yWhwEvotAZGH9WMT1l5lRAOLjzNefA1XIzlicG9RSos?via=matrix.org&via=t2bot.io

Description

I added a way to get a &Texture out of the render graph while inside a node. This is required to interact with some wgpu APIs (like copy from texture to buffer) which require a texture, not a view.

This is required to interact with some wgpu APIs (like copy from texture
to buffer) that require a texture, not a view.
base_array_layer: region.layer_start,
array_layer_count: Some(NonZeroU32::new(region.layer_end - region.layer_start).unwrap()),
..TextureViewDescriptor::default()
});
vacant.insert(view);
vacant.insert((view, Arc::clone(texture)));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to make the change as unintrusive as possible, so now the active_views stores both a TextureView and an Arc<Texture> with the texture it was created from as a tuple.

desc: &RenderPassTargets,
encoder: &'rpass mut CommandEncoder,
node_idx: usize,
pass_end_idx: usize,
resource_spans: &'rpass FastHashMap<GraphResource, ResourceSpan>,
active_views: &'rpass FastHashMap<TextureRegion, TextureView>,
active_views: &'rpass FastHashMap<TextureRegion, (TextureView, A)>,
Copy link
Contributor Author

@setzer22 setzer22 Jan 21, 2023

Choose a reason for hiding this comment

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

The Arc<Texture> part of the tuple was unused in this function, so I added a generic parameter with no bounds just to document that the function doesn't really care about this. Hopefully that's not too weird. Let me know if you know a better approach!

Comment on lines +81 to +82
GraphSubResource::ImportedTexture(_) => {
panic!("Getting render target as a texture not supported for imported textures");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if there's a good equivalent for ImportedTextures we can use here. Since the purpose of imported textures is a bit unclear to me, I decided to leave this as unsupported.

Comment on lines -63 to +66
panic!("internal rendergraph error: tried to get a {:?} as a render target", r)
panic!("internal rendergraph error: tried to get a {r:?} as a render target")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clippy will soon complain about this, so I took the chance to adapt the new warnings in the files I touched.

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.

1 participant