-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Changes from 12 commits
8c74012
293eea6
f119009
a50b00d
14f555c
3373cdd
7bfb12f
42a2796
1bffb98
de1b468
5bb1fb6
189fe19
6b76e17
79fe5fd
e7e686a
d97219f
34a1d4f
be82618
ebe7a39
0871be0
75d4092
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -349,6 +349,7 @@ struct UiVertex { | |||||||||||||||
pub position: [f32; 3], | ||||||||||||||||
pub uv: [f32; 2], | ||||||||||||||||
pub color: [f32; 4], | ||||||||||||||||
pub mode: u32, | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
#[derive(Resource)] | ||||||||||||||||
|
@@ -379,9 +380,11 @@ const QUAD_INDICES: [usize; 6] = [0, 2, 3, 0, 1, 2]; | |||||||||||||||
pub struct UiBatch { | ||||||||||||||||
pub range: Range<u32>, | ||||||||||||||||
pub image: Handle<Image>, | ||||||||||||||||
pub z: f32, | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
const TEXTURED_QUAD: u32 = 0; | ||||||||||||||||
const UNTEXTURED_QUAD: u32 = 1; | ||||||||||||||||
|
||||||||||||||||
pub fn prepare_uinodes( | ||||||||||||||||
mut commands: Commands, | ||||||||||||||||
render_device: Res<RenderDevice>, | ||||||||||||||||
|
@@ -398,20 +401,27 @@ pub fn prepare_uinodes( | |||||||||||||||
|
||||||||||||||||
let mut start = 0; | ||||||||||||||||
let mut end = 0; | ||||||||||||||||
let mut current_batch_handle = Default::default(); | ||||||||||||||||
let mut last_z = 0.0; | ||||||||||||||||
let mut stored_batch_handle = DEFAULT_IMAGE_HANDLE.typed(); | ||||||||||||||||
|
||||||||||||||||
#[inline] | ||||||||||||||||
fn is_textured(image: &Handle<Image>) -> bool { image.id() != DEFAULT_IMAGE_HANDLE.id() } | ||||||||||||||||
ickshonpe marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
|
||||||||||||||||
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) { | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||||||||||
if is_textured(&stored_batch_handle) { | ||||||||||||||||
if stored_batch_handle.id() != extracted_uinode.image.id() { | ||||||||||||||||
commands.spawn(UiBatch { | ||||||||||||||||
range: start..end, | ||||||||||||||||
image: stored_batch_handle, | ||||||||||||||||
}); | ||||||||||||||||
start = end; | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
current_batch_handle = extracted_uinode.image.clone_weak(); | ||||||||||||||||
} | ||||||||||||||||
stored_batch_handle = extracted_uinode.image.clone_weak(); | ||||||||||||||||
TEXTURED_QUAD | ||||||||||||||||
} else { | ||||||||||||||||
UNTEXTURED_QUAD | ||||||||||||||||
}; | ||||||||||||||||
|
||||||||||||||||
let mut uinode_rect = extracted_uinode.rect; | ||||||||||||||||
|
||||||||||||||||
|
@@ -469,7 +479,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); | ||||||||||||||||
|
@@ -514,19 +524,18 @@ pub fn prepare_uinodes( | |||||||||||||||
position: positions_clipped[i].into(), | ||||||||||||||||
uv: uvs[i].into(), | ||||||||||||||||
color, | ||||||||||||||||
mode, | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||||
}); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
last_z = extracted_uinode.transform.w_axis[2]; | ||||||||||||||||
end += QUAD_INDICES.len() as u32; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// if start != end, there is one last batch to process | ||||||||||||||||
if start != end { | ||||||||||||||||
commands.spawn(UiBatch { | ||||||||||||||||
range: start..end, | ||||||||||||||||
image: current_batch_handle, | ||||||||||||||||
z: last_z, | ||||||||||||||||
image: stored_batch_handle, | ||||||||||||||||
}); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
@@ -604,7 +613,7 @@ pub fn queue_uinodes( | |||||||||||||||
draw_function: draw_ui_function, | ||||||||||||||||
pipeline, | ||||||||||||||||
entity, | ||||||||||||||||
sort_key: FloatOrd(batch.z), | ||||||||||||||||
sort_key: FloatOrd(0.), | ||||||||||||||||
ickshonpe marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
}); | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
There was a problem hiding this comment.
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 twou32
consts for now.There was a problem hiding this comment.
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.