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

UI only shows if in last pass #5721

Closed
HackerFoo opened this issue Aug 17, 2022 · 3 comments
Closed

UI only shows if in last pass #5721

HackerFoo opened this issue Aug 17, 2022 · 3 comments
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior

Comments

@HackerFoo
Copy link
Contributor

Bevy version

f1be89d

Relevant system information

Tested on MacOS and iOS, with 4x MSAA

What you did

I used UiCameraConfig to enable show_ui on only one pass that was not the last pass (using camera priority.)

What went wrong

The UI did not show, but I could see it was being rendered using the Metal debugger.

Additional information

I suspected that the wrong color attachment was being used, so that the rendered UI was accidentally discarded.

So I made the following changes:

diff --git a/crates/bevy_ui/src/render/pipeline.rs b/crates/bevy_ui/src/render/pipeline.rs
index e199cef52..7914d3b8f 100644
--- a/crates/bevy_ui/src/render/pipeline.rs
+++ b/crates/bevy_ui/src/render/pipeline.rs
@@ -105,7 +105,7 @@ impl SpecializedRenderPipeline for UiPipeline {
             },
             depth_stencil: None,
             multisample: MultisampleState {
-                count: 1,
+                count: 4,
                 mask: !0,
                 alpha_to_coverage_enabled: false,
             },
diff --git a/crates/bevy_ui/src/render/render_pass.rs b/crates/bevy_ui/src/render/render_pass.rs
index 66445c92a..9308b7a26 100644
--- a/crates/bevy_ui/src/render/render_pass.rs
+++ b/crates/bevy_ui/src/render/render_pass.rs
@@ -81,14 +81,10 @@ impl Node for UiPassNode {
         };
         let pass_descriptor = RenderPassDescriptor {
             label: Some("ui_pass"),
-            color_attachments: &[Some(RenderPassColorAttachment {
-                view: &target.view,
-                resolve_target: None,
-                ops: Operations {
-                    load: LoadOp::Load,
-                    store: true,
-                },
-            })],
+            color_attachments: &[Some(target.get_color_attachment(Operations {
+                load: LoadOp::Load,
+                store: true,
+            }))],
             depth_stencil_attachment: None,
         };

and it works as expected now, but it's probably not necessary to use MSAA for the UI, so I'm not sure what the correct way to do this is. Obviously hardcoding the multisample count is a bad idea; it would have to be passed through the key if this is actually the correct solution.

@HackerFoo HackerFoo added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets and removed S-Needs-Triage This issue needs to be labelled labels Aug 17, 2022
@HackerFoo
Copy link
Contributor Author

The reason I needed to render a pass after the UI pass was to draw on top of the UI, so I can circle UI elements in an instructional mode.

HackerFoo added a commit to HackerFoo/bevy that referenced this issue Aug 19, 2022
@rotoclone
Copy link

I also ran into this issue; I made a repo containing minimal code to reproduce it if it's helpful: https://github.com/rotoclone/bevy-ui-bug/blob/master/src/main.rs

HackerFoo added a commit to HackerFoo/bevy that referenced this issue Sep 6, 2022
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Sep 8, 2022
oceantume added a commit to oceantume/bevy that referenced this issue Sep 14, 2022
This is taken from bevyengine#5721 as a temporary fix. Without it, only the UI
from the highest priority camera will actually appear.
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Dec 21, 2022
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Dec 21, 2022
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Dec 21, 2022
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Jan 6, 2023
@nicopap
Copy link
Contributor

nicopap commented Jan 16, 2023

Note that the workaround patch is now

diff --git a/crates/bevy_ui/src/render/pipeline.rs b/crates/bevy_ui/src/render/pipeline.rs
index 1a86d577ab41..685856d14044 100644
--- a/crates/bevy_ui/src/render/pipeline.rs
+++ b/crates/bevy_ui/src/render/pipeline.rs
@@ -114,7 +114,7 @@ impl SpecializedRenderPipeline for UiPipeline {
             },
             depth_stencil: None,
             multisample: MultisampleState {
-                count: 1,
+                count: 4,
                 mask: !0,
                 alpha_to_coverage_enabled: false,
             },
diff --git a/crates/bevy_ui/src/render/render_pass.rs b/crates/bevy_ui/src/render/render_pass.rs
index ee05cf6f8a21..26cd4bfc427e 100644
--- a/crates/bevy_ui/src/render/render_pass.rs
+++ b/crates/bevy_ui/src/render/render_pass.rs
@@ -78,7 +78,7 @@ impl Node for UiPassNode {
         };
         let pass_descriptor = RenderPassDescriptor {
             label: Some("ui_pass"),
-            color_attachments: &[Some(target.get_unsampled_color_attachment(Operations {
+            color_attachments: &[Some(target.get_color_attachment(Operations {
                 load: LoadOp::Load,
                 store: true,
             }))],

HackerFoo added a commit to HackerFoo/bevy that referenced this issue Jan 22, 2023
@bors bors bot closed this as completed in abcb066 Mar 1, 2023
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Mar 4, 2023
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Mar 5, 2023
Shfty pushed a commit to shfty-rust/bevy that referenced this issue Mar 19, 2023
# Objective

Alternative to bevyengine#7490. I wrote all of the code in this PR, but I have added @robtfm as co-author on commits that build on ideas from bevyengine#7490. I would not have been able to solve these problems on my own without much more time investment and I'm largely just rephrasing the ideas from that PR.

Fixes bevyengine#7435
Fixes bevyengine#7361
Fixes bevyengine#5721

## Solution

This implements the solution I [outlined here](bevyengine#7490 (comment)).


 * Adds "msaa writeback" as an explicit "msaa camera feature" and default to msaa_writeback: true for each camera. If this is true, a camera has MSAA enabled, and it isn't the first camera for the target, add a writeback before the main pass for that camera.
 * Adds a CameraOutputMode, which can be used to configure if (and how) the results of a camera's rendering will be written to the final RenderTarget output texture (via the upscaling node). The `blend_state` and `color_attachment_load_op` are now configurable, giving much more control over how a camera will write to the output texture.
 * Made cameras with the same target share the same main_texture tracker by using `Arc<AtomicUsize>`, which ensures continuity across cameras. This was previously broken / could produce weird results in some cases. `ViewTarget::main_texture()` is now correct in every context.
 * Added a new generic / specializable BlitPipeline, which the new MsaaWritebackNode uses internally. The UpscalingPipelineNode now uses BlitPipeline instead of its own pipeline. We might ultimately need to fork this back out if we choose to add more configurability to the upscaling, but for now this will save on binary size by not embedding the same shader twice.
 * Moved the "camera sorting" logic from the camera driver node to its own system. The results are now stored in the `SortedCameras` resource, which can be used anywhere in the renderer. MSAA writeback makes use of this.

---

## Changelog

- Added `Camera::msaa_writeback` which can enable and disable msaa writeback.
- Added specializable `BlitPipeline` and ported the upscaling node to use this.
- Added SortedCameras, exposing information that was previously internal to the camera driver node.
- Made cameras with the same target share the same main_texture tracker, which ensures continuity across cameras.
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Apr 10, 2023
HackerFoo added a commit to HackerFoo/bevy that referenced this issue Apr 26, 2023
This reverts commit f6ef378.
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 A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior
Projects
None yet
3 participants