[Merged by Bors] - Add capability to render to a texture #3412

HackerFoo · 2021-12-21T23:22:32Z

Objective

Solution

Use an enum to represent either a WindowId or Handle<Image> in place of Camera::window.

crates/bevy_render/src/camera/camera.rs

alice-i-cecile · 2021-12-22T03:43:04Z

This looks quite nice to me, and should be very useful. It looks like there's just MSAA support left to add and then this should be ready to merge?

HackerFoo · 2021-12-22T06:14:10Z

This looks quite nice to me, and should be very useful. It looks like there's just MSAA support left to add and then this should be ready to merge?

I may leave MSAA for a later PR, because it didn't work on the old renderer and may be significantly more work. I will need MSAA support, so it'll get added eventually.

The missing piece for this PR is a working example in examples/3d/render_to_texture.rs, based on examples/window/multiple_windows.rs. I've been testing functionality with my own app, and I still have some things in the renderer to fix before I can write the example.

crates/bevy_render/src/camera/camera.rs

HackerFoo · 2021-12-23T23:01:45Z

I didn't have any problems getting MSAA working, so that will be in this PR as well:

crates/bevy_core_pipeline/src/lib.rs

crates/bevy_render/src/camera/camera.rs

examples/3d/render_to_texture.rs

examples/README.md

Co-authored-by: Johan Klokkhammer Helsing <johanhelsing@gmail.com>

cart · 2022-02-23T01:57:44Z

This is great and I think I'd like to merge it as-is because it is small, straightforward, and extremely useful as written. But I do think we need to revisit what a "render target" is conceptually. I think users should be able to utilize render targets to draw to textures (or other windows) without touching the render app / graph (or understanding its underlying structure) at all.

A RenderTarget should be "an entity created by a user that correlates to a Window or Texture, which by default runs the 'default render graph logic', but specialized for each target". This means everything runs by default (ui, 2d, 3d, user defined passes, post processing, etc). It follows that Cameras cannot own a RenderTarget because more than one camera is required to render the "default behavior". RenderTargets should drive execution of Render Graphs and clear passes, not Cameras / Views. Cameras / Views should correlate to RenderTargets. This also solves the "ClearColor mapping" problem, because we can add a ClearColor component to RenderTargets. This would also solve the weirdness in the clear_pass.rs impl which clears based on views instead of "targets".

This does create a number of questions:

How will "render target" render ordering be encoded in "main app" worlds? This PR punts that problem to the user in the form of manually defining the render graph. I think we could initially get away with a naive "all non-default targets run first" impl, but ultimately we should be able to support arbitrarily nested targets. (ex: render a scene to a target, which is used in another render target, which is used in the main pass). We could accomplish this in a number of ways, such as using entity hierarchies to define relationships between targets.
Some scenarios will require non-default graph configurations. Can we (and should we) let RenderTargets cover those use cases (ex: by adding a RenderTarget::is_manually_executed flag)?

I think I'm down to merge this right now so we can start ironing out the various kinks unrelated to user-facing apis (such as per-target lights) and unblock people that need the feature asap. The only change I'd request is a switch to a second Res<RenderTargetClearColors> that holds the render target clear color map. That way this PR is a non-breaking change for most users while we sort out the final user-facing ap. I'm going to start experimenting with rephrasing render targets as entities and I'll report back when I have something worth sharing.

HackerFoo · 2022-02-23T08:11:55Z

I'd like to add another question, which I work on a bit in later PRs:

How should users compose "default behaviors" in general? For example, to layer 3D passes by clearing depth, or layering wireframes on top of triangles. It seems like it should be possible without building a new render graph, but run_sub_graph isn't enough. Textures are one way to compose passes, so this is related to the other two questions.

cart · 2022-02-24T00:39:40Z

bors r+

# Objective Will fix #3377 and #3254 ## Solution Use an enum to represent either a `WindowId` or `Handle<Image>` in place of `Camera::window`. Co-authored-by: Carter Anderson <mcanders1@gmail.com>

bors · 2022-02-24T01:19:07Z

Pull request successfully merged into main.

Build succeeded:

superdump · 2022-02-24T08:05:51Z

@cart one more thought on clear colours - if there are multiple cameras contributing to a render target, is it possible / does it make sense to support a different clear colour per camera which would then clear different parts of the screen?

cart · 2022-02-24T08:09:45Z

My current take: I don't think cameras should have clear colors. I think render targets should be decoupled from cameras (and allow multiple cameras mapping to the same render target). RenderTargets should have clear colors. RenderTargets should have a single clear color for the "default render graph". Non-default render target / render graph combos can define their own clear passes (with their own clear color configuation) as it makes sense.

# Objective Will fix bevyengine#3377 and bevyengine#3254 ## Solution Use an enum to represent either a `WindowId` or `Handle<Image>` in place of `Camera::window`. Co-authored-by: Carter Anderson <mcanders1@gmail.com>

# Objective Fixes #3499 ## Solution Uses a `HashMap` from `RenderTarget` to sampled textures when preparing `ViewTarget`s to ensure that two passes with the same render target get sampled to the same texture. This builds on and depends on #3412, so this will be a draft PR until #3412 is merged. All changes for this PR are in the last commit.

# Objective - After #3412, `Camera::world_to_screen` got a little bit uglier to use by needing to provide both `Windows` and `Assets<Image>`, even though only one would be needed https://github.com/bevyengine/bevy/blob/b697e73c3d861c209152ccfb140ae00fbc6e9925/crates/bevy_render/src/camera/camera.rs#L117-L123 - Some time, exact coordinates are not needed but normalized device coordinates is enough ## Solution - Add a function to just get NDC

…4041) # Objective - After bevyengine#3412, `Camera::world_to_screen` got a little bit uglier to use by needing to provide both `Windows` and `Assets<Image>`, even though only one would be needed https://github.com/bevyengine/bevy/blob/b697e73c3d861c209152ccfb140ae00fbc6e9925/crates/bevy_render/src/camera/camera.rs#L117-L123 - Some time, exact coordinates are not needed but normalized device coordinates is enough ## Solution - Add a function to just get NDC

# Objective Fixes bevyengine#3499 ## Solution Uses a `HashMap` from `RenderTarget` to sampled textures when preparing `ViewTarget`s to ensure that two passes with the same render target get sampled to the same texture. This builds on and depends on bevyengine#3412, so this will be a draft PR until bevyengine#3412 is merged. All changes for this PR are in the last commit.

…4041) # Objective - After bevyengine#3412, `Camera::world_to_screen` got a little bit uglier to use by needing to provide both `Windows` and `Assets<Image>`, even though only one would be needed https://github.com/bevyengine/bevy/blob/b697e73c3d861c209152ccfb140ae00fbc6e9925/crates/bevy_render/src/camera/camera.rs#L117-L123 - Some time, exact coordinates are not needed but normalized device coordinates is enough ## Solution - Add a function to just get NDC

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 21, 2021

HackerFoo force-pushed the render_to_texture branch from f46aeec to e814a8a Compare December 21, 2021 23:36

alice-i-cecile added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible P-Regression Functionality that used to work but no longer does. Add a test for this! and removed S-Needs-Triage This issue needs to be labelled labels Dec 22, 2021

alice-i-cecile reviewed Dec 22, 2021

View reviewed changes

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

alice-i-cecile added the S-Needs-Migration-Guide label Dec 22, 2021

jakobhellermann reviewed Dec 22, 2021

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

HackerFoo force-pushed the render_to_texture branch 2 times, most recently from edab9e8 to 4de71ea Compare December 28, 2021 01:07

HackerFoo changed the title ~~[WIP] Add capability to render to a texture~~ Add capability to render to a texture Dec 28, 2021

HackerFoo marked this pull request as ready for review December 28, 2021 01:09

HackerFoo force-pushed the render_to_texture branch 2 times, most recently from 862668f to fbd7c86 Compare December 28, 2021 02:13

HackerFoo requested review from alice-i-cecile and jakobhellermann December 28, 2021 02:30