Skip to content

Conversation

HackerFoo
Copy link
Contributor

Objective

Fixes #3499

Solution

Uses a HashMap from RenderTarget to sampled textures when preparing ViewTargets 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.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 4, 2022
@HackerFoo HackerFoo changed the title Two passes Reuse texture when resolving multiple passes Jan 5, 2022
@HackerFoo HackerFoo marked this pull request as draft January 5, 2022 00:37
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change C-Bug An unexpected or incorrect behavior and removed S-Needs-Triage This issue needs to be labelled C-Code-Quality A section of code that is hard to understand or change labels Jan 5, 2022
@HackerFoo
Copy link
Contributor Author

It looks like the depth buffer is also not being shared by passes.

@superdump
Copy link
Contributor

As #3412 was merged, could you update this on top of main so we can get it reviewed and merged?

@HackerFoo HackerFoo marked this pull request as ready for review March 4, 2022 02:29
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

Just need to update a comment and then this looks good to me.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

A couple of nits, but the example and general code quality look good to me. Cannot comment in depth on the rendering strategy.

@cart
Copy link
Member

cart commented Apr 10, 2022

Sorry for the delay on this. This seems relatively uncontroversial / worth including in 0.7. I'll try to give it a proper review before release.

@alice-i-cecile 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 Apr 11, 2022
@cart
Copy link
Member

cart commented Apr 12, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 12, 2022
# 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.
@bors bors bot changed the title Reuse texture when resolving multiple passes [Merged by Bors] - Reuse texture when resolving multiple passes Apr 12, 2022
@bors bors bot closed this Apr 12, 2022
bors bot pushed a commit that referenced this pull request Apr 13, 2022
Because #3552 got merged only switching the render order is left to fix #3902
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
# 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.
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
Because bevyengine#3552 got merged only switching the render order is left to fix bevyengine#3902
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# 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.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
Because bevyengine#3552 got merged only switching the render order is left to fix bevyengine#3902
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-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Rendering two 3d passes to the same window only shows the last pass when MSAA is enabled

5 participants