-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Keep track of when a texture is first cleared #10325
Conversation
I can also maybe use Relaxed atomic ordering. |
I don’t have time to check right now but I think we need to be careful about removing all the prepass checks for whether to load or clear the depth buffer as they may not mean the same thing. For example, logically if something is the first write then we have to clear it and cannot load it. But if something is not the first write, perhaps then it still needs to be cleared in some cases. I don’t think so but I would also want to check to make sure. |
Oops this was supposed to be for milestone 0.13, fixed. There's no urgency on this PR. |
I also want to add flags for the prepass textures and any other render targets as well. |
TODO: Add support for the prepass outputs and the deferred gbuffer prepass outputs. |
I'm not super familiar with this part of the renderer. Can you say more about why we should / need to use atomics here? |
Keeping track of whether or not to clear the texture is decided in the render nodes, which don't allow mutable access to components, so an atomic is needed. Otherwise it would just be a regular boolean. |
i definitely think this should be done, but i'm not convinced about the way it is done here.
the last two points make me think we're missing an abstraction: it would be nicer i think to have something like a what do you think? |
I'm in favor of some kind of ClearOnceTexture wrapper, agreed.
Yeah it's not the best naming, but I couldn't think of anything better. I suppose moving the info into the type name helps, and then we can call the method just get_load_op() or something. |
Co-authored-by: vero <[email protected]>
…into target_is_first_write
Co-authored-by: Alice Cecile <[email protected]>
This generally looks fine, I understand the motivation, and this is reviewed by folks with expertise. Merging now. |
Head branch was pushed to by a user without write access
Objective
Solution
Changelog
ViewTarget::get_color_attachment()
, removed arguments.ViewTarget::get_unsampled_color_attachment()
, removed arguments.Camera3d::clear_color
.Camera2d::clear_color
.Camera::clear_color
.ExtractedCamera::clear_color
.ColorAttachment
andDepthAttachment
wrappers.ClearColor
andClearColorConfig
frombevy::core_pipeline::clear_color
tobevy::render::camera
.Migration Guide
ViewTarget::get_color_attachment()
andViewTarget::get_unsampled_color_attachment()
.Camera
instead of onCamera3d
andCamera2d
.ClearColor
andClearColorConfig
frombevy::core_pipeline::clear_color
tobevy::render::camera
.ViewDepthTexture
must now be created via thenew()
method