Add `RenderTarget::TextureView` #8042

mrchantey · 2023-03-11T12:25:18Z

Objective

We can currently set camera.target to either an Image or Window. For OpenXR & WebXR we need to be able to render to a TextureView.

This partially addresses #115 as with the addition we can create internal and external xr crates.

Solution

A TextureView item is added to the RenderTarget enum. It holds an id which is looked up by a ManualTextureViews resource, much like how Assets<Image> works.
I believe this approach was first used by @kcking in their xr fork. The only change is that a u32 is used to index the textures as FromReflect does not support uuid and I don't know how to implement that.

Changelog

Added

Render: Added RenderTarget::TextureView as a camera.target option, enabling rendering directly to a TextureView.

Migration Guide

References to the RenderTarget enum will need to handle the additional field, ie in match statements.

Comments

The wgpu work done by @expenses allows us to create framebuffer texture views from wgpu v0.15, bevy 0.10.
I got the WebXR techniques from the xr fork by @dekuraan.
I have tested this with a wip external webxr crate on an Oculus Quest 2.

* Add 0.10.0 changelog * Add missing `

github-actions · 2023-03-11T12:25:29Z

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

github-actions · 2023-03-11T18:28:59Z

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

MalekiRe · 2023-05-03T22:52:26Z

Bumping

IceSentry · 2023-05-24T04:07:24Z

crates/bevy_render/src/camera/camera.rs

@@ -368,6 +370,24 @@ impl CameraRenderGraph {
    }
 }

+/// Stores Texture Views used as render targets.
+#[derive(Default, Clone, Resource, ExtractResource)]
+pub struct ManualTextureViews(HashMap<u32, (TextureView, UVec2)>);


I'm not sure what "Manual" is supposed to mean here.

I think I'd prefer using a wrapper type for the u32. It wasn't immediately obvious to me what the u32 was supposed to be.

I'd also add a note in the comment that the UVec2 is the size. Or maybe just say that it stores the texture view and it's size

Ok great I've updated the comment.

Manual is intended to describe that these are texture views we are managing ourselves outside of Bevy, either by creating them in wgpu or being given them by OpenXR, WebXR etc. I agree its not obvious, any suggestions? Maybe something like Custom or External?

I've added the ManualTextureViewHandle wrapper type, im a little concerned its bloating the camera.rs file, would you like the ManualTextureView struct moved to a seperate file?

I've created manual_texture_view.rs, let me know if you want that reverted.

IceSentry · 2023-05-24T04:07:48Z

crates/bevy_render/src/camera/camera.rs

@@ -502,13 +542,16 @@ impl NormalizedRenderTarget {
 /// [`OrthographicProjection`]: crate::camera::OrthographicProjection
 /// [`PerspectiveProjection`]: crate::camera::PerspectiveProjection
 /// [`Projection`]: crate::camera::Projection
+/// [`CoreSet::PostUpdate`]: bevy_app::CoreSet::PostUpdate


Why was this line added?

Git blame is showing this as from an older pr, i guess it has been removed, i've deleted it from this pr.

IceSentry · 2023-05-24T04:13:55Z

crates/bevy_render/src/camera/camera.rs

@@ -436,13 +465,15 @@ impl NormalizedRenderTarget {
            NormalizedRenderTarget::Image(image_handle) => {
                images.get(image_handle).map(|image| image.texture_format)
            }
+            NormalizedRenderTarget::TextureView(_) => Some(TextureFormat::bevy_default()),


I don't think this line is correct. As far as I know, it's possible for a TextureView to be a different format.

Ok we should check that, my understanding was that the TextureView is opaque and we dont have access to the format, @kcking do you have thoughts on this?

if it is something we want to track, maybe instead of storing a (TextureView,UVec2) tuple, we can store some struct

pub struct ManualTextureView { texture_view: TextureView, size: UVec2, format: TextureFormat, }

Agreed, storing the correct TextureFormat makes the most sense to me.

IceSentry · 2023-06-11T19:44:21Z

So, the implementation seems good to me, but I'm not sure I understand why this is necessary. Isn't the Image render target just a wrapper around targetting a TextureView? As in, couldn't you just use the render to image feature and use the texture view of the image?

kcking · 2023-06-11T19:49:43Z

So, the implementation seems good to me, but I'm not sure I understand why this is necessary. Isn't the Image render target just a wrapper around targetting a TextureView? As in, couldn't you just use the render to image feature and use the texture view of the image?

There's a quick discord conversation about this, essentially OpenXR requires you to use its TextureViews, you can't provide your own. AFAIU bevy's render to image feature creates new TextureViews so it's incompatible with OpenXR

IceSentry · 2023-06-11T19:58:39Z

Alright, makes sense. In that case this PR seems fine.

paul-hansen

Added a suggestion for adding a comment regarding the use case for this.
Looks good otherwise, though I'm not a rendering expert. Has my approval with or without my suggestion.

crates/bevy_render/src/camera/camera.rs

Co-authored-by: Paul Hansen <mail@paul.rs>

paul-hansen

Oof looks like my suggestion somehow ended up with trailing spaces and it broke cargo format. My bad.

crates/bevy_render/src/camera/camera.rs

Co-authored-by: Paul Hansen <mail@paul.rs>

alice-i-cecile · 2023-06-19T13:53:01Z

Straightforward enough, and on the critical path. Let's see what the community can do with this <3

@kcking

# Objective We can currently set `camera.target` to either an `Image` or `Window`. For OpenXR & WebXR we need to be able to render to a `TextureView`. This partially addresses bevyengine#115 as with the addition we can create internal and external xr crates. ## Solution A `TextureView` item is added to the `RenderTarget` enum. It holds an id which is looked up by a `ManualTextureViews` resource, much like how `Assets<Image>` works. I believe this approach was first used by @kcking in their [xr fork](https://github.com/kcking/bevy/blob/eb39afd51bcbab38de6efbeeb0646e01e2ce4766/crates/bevy_render/src/camera/camera.rs#L322). The only change is that a `u32` is used to index the textures as `FromReflect` does not support `uuid` and I don't know how to implement that. --- ## Changelog ### Added Render: Added `RenderTarget::TextureView` as a `camera.target` option, enabling rendering directly to a `TextureView`. ## Migration Guide References to the `RenderTarget` enum will need to handle the additional field, ie in `match` statements. --- ## Comments - The [wgpu work](gfx-rs/wgpu@c039a74) done by @expenses allows us to create framebuffer texture views from `wgpu v0.15, bevy 0.10`. - I got the WebXR techniques from the [xr fork](https://github.com/dekuraan/xr-bevy) by @dekuraan. - I have tested this with a wip [external webxr crate](https://github.com/mrchantey/forky/blob/018e22bb06b7542419db95f5332c7684931c9c95/crates/bevy_webxr/src/bevy_utils/xr_render.rs#L50) on an Oculus Quest 2. ![Screenshot 2023-03-11 230651](https://user-images.githubusercontent.com/25616826/224483696-c176c06f-a806-4abe-a494-b2e096ac96b7.png) --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com> Co-authored-by: Paul Hansen <mail@paul.rs>

cart and others added 6 commits March 6, 2023 02:43

Add 0.10.0 changelog (#7926)

21b2b86

* Add 0.10.0 changelog * Add missing `

Fix publish license path (#7927)

289fd1d

initial commit

fbfd0eb

fix: mistake

840433e

refactor: use ExtractResource

75f2ae2

Merge branch 'main' into feat-rendertarget-textureview

5d259f0

camera system: allow too many arguments

06c1859

james7132 added C-Enhancement A new feature A-Rendering Drawing game state to the screen C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels Mar 11, 2023

james7132 added this to the 0.11 milestone Mar 11, 2023

james7132 mentioned this pull request Mar 11, 2023

Contributor advice on the use of non_exhaustive enums #8044

Open

james7132 added the O-XR Specific to virtual and augmented reality platforms label Mar 16, 2023

james7132 requested a review from superdump March 16, 2023 08:14

mrchantey added 3 commits March 22, 2023 15:50

Merge branch 'main' into feat-rendertarget-textureview

0f5cec2

fix: use ExtractResourcePlugin

1e4cfec

Merge branch 'main' into feat-rendertarget-textureview

ebd7f92

IceSentry reviewed May 24, 2023

View reviewed changes

mrchantey added 5 commits May 24, 2023 14:51

remove comment from a different pr.

be5b705

clarify ManualTextureView documentation

020bb8a

feat: use ManualTextureViewHandle wrapper type

2c7bb88

add documentation for ManualTextureViewHandle

c961d73

feat: ManualTextureView struct includes format

cf890bb

mrchantey requested a review from IceSentry May 30, 2023 00:27

mrchantey changed the title ~~Add RenderTarget::TextureView~~ Add RenderTarget::TextureView May 30, 2023

IceSentry approved these changes Jun 11, 2023

View reviewed changes

paul-hansen approved these changes Jun 13, 2023

View reviewed changes

crates/bevy_render/src/camera/camera.rs Outdated Show resolved Hide resolved

crates/bevy_render/src/camera/camera.rs Outdated Show resolved Hide resolved

alice-i-cecile 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 Jun 13, 2023

mrchantey and others added 2 commits June 14, 2023 09:06

Update crates/bevy_render/src/camera/camera.rs

8a72838

Co-authored-by: Paul Hansen <mail@paul.rs>

Update crates/bevy_render/src/camera/camera.rs

461573b

Co-authored-by: Paul Hansen <mail@paul.rs>

paul-hansen reviewed Jun 13, 2023

View reviewed changes

crates/bevy_render/src/camera/camera.rs Outdated Show resolved Hide resolved

crates/bevy_render/src/camera/camera.rs Outdated Show resolved Hide resolved

mrchantey and others added 2 commits June 14, 2023 14:33

Update crates/bevy_render/src/camera/camera.rs

86cda45

Co-authored-by: Paul Hansen <mail@paul.rs>

Update crates/bevy_render/src/camera/camera.rs

4f9075f

Co-authored-by: Paul Hansen <mail@paul.rs>

alice-i-cecile added this pull request to the merge queue Jun 19, 2023

Merged via the queue into bevyengine:main with commit 6ce4bf5 Jun 19, 2023

This was referenced Jun 28, 2023

News: Release 0.11 (wrong branch ... see #657) bevyengine/bevy-website#656

Closed

News: Release 0.11 bevyengine/bevy-website#657

Merged

cart mentioned this pull request Oct 13, 2023

News: Release 0.12 bevyengine/bevy-website#754

Merged

43 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `RenderTarget::TextureView` #8042

Add `RenderTarget::TextureView` #8042

mrchantey commented Mar 11, 2023 •

edited

Loading

github-actions bot commented Mar 11, 2023

github-actions bot commented Mar 11, 2023

MalekiRe commented May 3, 2023

IceSentry May 24, 2023

IceSentry May 24, 2023

mrchantey May 24, 2023 •

edited

Loading

mrchantey May 24, 2023

mrchantey May 30, 2023

IceSentry May 24, 2023

mrchantey May 24, 2023

IceSentry May 24, 2023

mrchantey May 24, 2023

mrchantey May 24, 2023

kcking May 25, 2023

IceSentry commented Jun 11, 2023

kcking commented Jun 11, 2023 •

edited

Loading

IceSentry commented Jun 11, 2023

paul-hansen left a comment

paul-hansen left a comment

alice-i-cecile commented Jun 19, 2023

Add RenderTarget::TextureView #8042

Add RenderTarget::TextureView #8042

Conversation

mrchantey commented Mar 11, 2023 • edited Loading

Objective

Solution

Changelog

Added

Migration Guide

Comments

github-actions bot commented Mar 11, 2023

github-actions bot commented Mar 11, 2023

MalekiRe commented May 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrchantey May 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IceSentry commented Jun 11, 2023

kcking commented Jun 11, 2023 • edited Loading

IceSentry commented Jun 11, 2023

paul-hansen left a comment

Choose a reason for hiding this comment

paul-hansen left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Jun 19, 2023

Add `RenderTarget::TextureView` #8042

Add `RenderTarget::TextureView` #8042

mrchantey commented Mar 11, 2023 •

edited

Loading

mrchantey May 24, 2023 •

edited

Loading

kcking commented Jun 11, 2023 •

edited

Loading