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

Allow custom depth texture usage #6815

Merged
merged 7 commits into from
May 8, 2023

Conversation

Neo-Zhixing
Copy link
Contributor

Objective

Sometimes we might want to read from the depth texture in some custom rendering features. We must then add STORAGE_BINDING or TEXTURE_BINDING to the texture usage flags when creating them.

Solution

This PR allows one to customize the usage flags in the Camera3d component.

}

#[derive(Clone, Copy, Reflect)]
pub struct Camera3dDepthTextureUsage(u32);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating a custom class to wrap the TextureUsage so that it implements Reflect

@james7132 james7132 added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Dec 2, 2022
Copy link
Contributor

@kurtkuehnert kurtkuehnert left a comment

Choose a reason for hiding this comment

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

LGTM.
This only works for 3D cameras not for 2D ones (although 2D is probably less important).
Maybe we can generalize the camera/view target abstraction for both the 2D and 3D cases in the future.

@Neo-Zhixing
Copy link
Contributor Author

@superdump Can you merge this PR?

@bonsairobo
Copy link
Contributor

Bump

@JulianRuiseco
Copy link

Bump :)

@IceSentry IceSentry added this to the 0.11 milestone Mar 21, 2023
#[extract_component_filter(With<Camera>)]
#[reflect(Component)]
pub struct Camera3d {
/// The clear color operation to perform for the main 3d pass.
pub clear_color: ClearColorConfig,
/// The depth clear operation to perform for the main 3d pass.
pub depth_load_op: Camera3dDepthLoadOp,
/// The texture usages for the depth texture created for the main 3d pass.
pub depth_texture_usages: Camera3dDepthTextureUsage,
Copy link
Member

Choose a reason for hiding this comment

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

what happens if I set a value without TextureUsages::RENDER_ATTACHMENT here? Should we consider it's present even if the user didn't set it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

imo the user should be allowed to set a value without TextureUsages::RENDER_ATTACHMENT. If they explicitly opted out of the RENDER_ATTACHMENT usage, there's probably a reason for it.

@IceSentry IceSentry added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 4, 2023
@cart cart added this pull request to the merge queue May 8, 2023
Merged via the queue into bevyengine:main with commit 44a365d May 8, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jan 18, 2024
# Objective

- Some users want to change the default texture usage of the main camera
but they are currently hardcoded

## Solution

- Add a component that is used to configure the main texture usage field

---

## Changelog

Added `CameraMainTextureUsage`
Added `CameraMainTextureUsage` to `Camera3dBundle` and `Camera2dBundle`

## Migration Guide

Add `main_texture_usages: Default::default()` to your camera bundle.

# Notes

Inspired by: #6815
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants