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

Improved UI render batching #8793

Merged
merged 21 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8c74012
In `prepare_uinodes` set the UVs to a large number if the quad is unt…
ickshonpe Jun 8, 2023
293eea6
cargo fmt --all
ickshonpe Jun 8, 2023
f119009
clippy::collapsible-if
ickshonpe Jun 8, 2023
a50b00d
Improved improved batching.
ickshonpe Jun 8, 2023
14f555c
cargo fmt --all
ickshonpe Jun 8, 2023
3373cdd
Fixed multiple textures bug.
ickshonpe Jun 9, 2023
7bfb12f
Switch to using a flag
ickshonpe Jun 9, 2023
42a2796
replace mode values with consts
ickshonpe Jun 9, 2023
1bffb98
Set
ickshonpe Jun 10, 2023
de1b468
Removed `last_z` from UiBatch` as it is always equal to `0`
ickshonpe Jun 10, 2023
5bb1fb6
Removed the `start != end` check in the main loop as it's redundant …
ickshonpe Jun 10, 2023
189fe19
Replaced `handle.id() != default_id` predicates with a function `is_t…
ickshonpe Jun 10, 2023
6b76e17
renamed `staored_image_handle` to `current_batch_image`
ickshonpe Jun 10, 2023
79fe5fd
Don't need to update `current_batch_image` when it is already equal t…
ickshonpe Jun 10, 2023
e7e686a
Removed the `sort_key` field from `TransparentUi`
ickshonpe Jun 10, 2023
d97219f
fix clippy::unused-unit
ickshonpe Jun 10, 2023
34a1d4f
Reversed the z ordering changes. They aren't important and can be lef…
ickshonpe Jun 11, 2023
be82618
Should be `QUAD_INDICES.len()`, I don't know how this got changed.
ickshonpe Jun 12, 2023
ebe7a39
The `start != end` check is required inside the loop as well to accou…
ickshonpe Jun 19, 2023
0871be0
Merge branch 'main' into improved-ui-batching
ickshonpe Jun 20, 2023
75d4092
`ui.wgsl`
ickshonpe Jun 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 31 additions & 13 deletions crates/bevy_ui/src/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ struct UiVertex {
pub position: [f32; 3],
pub uv: [f32; 2],
pub color: [f32; 4],
pub mode: u32,
}

#[derive(Resource)]
Expand Down Expand Up @@ -587,6 +588,9 @@ pub struct UiBatch {
pub z: f32,
}

const TEXTURED_QUAD: u32 = 0;
const UNTEXTURED_QUAD: u32 = 1;
Comment on lines +591 to +592
Copy link
Contributor

Choose a reason for hiding this comment

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

In the futures, this could be turned into a bitflags, (for example if you want to cram more metadata into the vertex attributes) but I think it should be kept at two u32 consts for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I kept it this basic because the shader will be replaced soon for one that does rounded corners etc.


pub fn prepare_uinodes(
mut commands: Commands,
render_device: Res<RenderDevice>,
Expand All @@ -603,20 +607,33 @@ pub fn prepare_uinodes(

let mut start = 0;
let mut end = 0;
let mut current_batch_handle = Default::default();
let mut current_batch_image = DEFAULT_IMAGE_HANDLE.typed();
let mut last_z = 0.0;

#[inline]
fn is_textured(image: &Handle<Image>) -> bool {
image.id() != DEFAULT_IMAGE_HANDLE.id()
}

for extracted_uinode in &extracted_uinodes.uinodes {
if current_batch_handle != extracted_uinode.image {
if start != end {
commands.spawn(UiBatch {
range: start..end,
image: current_batch_handle,
z: last_z,
});
start = end;
let mode = if is_textured(&extracted_uinode.image) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I still would separate the logic from the value setting, in the imperative style (I say that as someone who has a strong bias in favor of functional programming)

Suggested change
let mode = if is_textured(&extracted_uinode.image) {
let mode = if is_textured(&extracted_uinode.image) {
TEXTURED_QUAD
} else {
UNTEXTURED_QUAD
};
if is_textured(&extracted_uinode.image) {

if current_batch_image.id() != extracted_uinode.image.id() {
if is_textured(&current_batch_image) && start != end {
commands.spawn(UiBatch {
range: start..end,
image: current_batch_image,
z: last_z,
});
start = end;
}
current_batch_image = extracted_uinode.image.clone_weak();
}
current_batch_handle = extracted_uinode.image.clone_weak();
}
TEXTURED_QUAD
} else {
// Untextured `UiBatch`es are never spawned within the loop.
// If all the `extracted_uinodes` are untextured a single untextured UiBatch will be spawned after the loop terminates.
UNTEXTURED_QUAD
};

let mut uinode_rect = extracted_uinode.rect;

Expand Down Expand Up @@ -674,7 +691,7 @@ pub fn prepare_uinodes(
continue;
}
}
let uvs = if current_batch_handle.id() == DEFAULT_IMAGE_HANDLE.id() {
let uvs = if mode == UNTEXTURED_QUAD {
[Vec2::ZERO, Vec2::X, Vec2::ONE, Vec2::Y]
} else {
let atlas_extent = extracted_uinode.atlas_size.unwrap_or(uinode_rect.max);
Expand Down Expand Up @@ -719,6 +736,7 @@ pub fn prepare_uinodes(
position: positions_clipped[i].into(),
uv: uvs[i].into(),
color,
mode,
Copy link
Contributor

Choose a reason for hiding this comment

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

For other reviewers, this is like the most important line of this PR

});
}

Expand All @@ -730,7 +748,7 @@ pub fn prepare_uinodes(
if start != end {
commands.spawn(UiBatch {
range: start..end,
image: current_batch_handle,
image: current_batch_image,
z: last_z,
});
}
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_ui/src/render/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ impl SpecializedRenderPipeline for UiPipeline {
VertexFormat::Float32x2,
// color
VertexFormat::Float32x4,
// mode
VertexFormat::Uint32,
],
);
let shader_defs = Vec::new();
Expand Down
12 changes: 11 additions & 1 deletion crates/bevy_ui/src/render/ui.wgsl
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
#import bevy_render::view

const TEXTURED_QUAD: u32 = 0u;

@group(0) @binding(0)
var<uniform> view: View;

struct VertexOutput {
@location(0) uv: vec2<f32>,
@location(1) color: vec4<f32>,
@location(3) mode: u32,
@builtin(position) position: vec4<f32>,
};

Expand All @@ -14,11 +17,13 @@ fn vertex(
@location(0) vertex_position: vec3<f32>,
@location(1) vertex_uv: vec2<f32>,
@location(2) vertex_color: vec4<f32>,
@location(3) mode: u32,
) -> VertexOutput {
var out: VertexOutput;
out.uv = vertex_uv;
out.position = view.view_proj * vec4<f32>(vertex_position, 1.0);
out.color = vertex_color;
out.mode = mode;
return out;
}

Expand All @@ -29,7 +34,12 @@ var sprite_sampler: sampler;

@fragment
fn fragment(in: VertexOutput) -> @location(0) vec4<f32> {
// textureSample can only be called in unform control flow, not inside an if branch.
var color = textureSample(sprite_texture, sprite_sampler, in.uv);
ickshonpe marked this conversation as resolved.
Show resolved Hide resolved
color = in.color * color;
if in.mode == TEXTURED_QUAD {
color = in.color * color;
} else {
color = in.color;
}
return color;
}