From d70b4a317074e96554a7678caf3254e1520ae6c6 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Fri, 3 Nov 2023 01:14:43 +0000 Subject: [PATCH] UI batching Fix (#9610) # Objective Reimplement #8793 on top of the recent rendering changes. ## Solution The batch creation logic is quite convoluted, but I tested it on enough examples to convince myself that it works. The initial value of `batch_image_handle` is changed from `HandleId::Id(Uuid::nil(), u64::MAX)` to `DEFAULT_IMAGE_HANDLE.id()`, which allowed me to make the if-block simpler I think. The default image from `DEFAULT_IMAGE_HANDLE` is always inserted into `UiImageBindGroups` even if it's not used. I tried to add a check so that it would be only inserted when there is only one batch using the default image but this crashed. --- ## Changelog `prepare_uinodes` * Changed the initial value of `batch_image_handle` to `DEFAULT_IMAGE_HANDLE.id()`. * The default image is added to the UI image bind groups before assembling the batches. * A new `UiBatch` isn't created when the next `ExtractedUiNode`s image is set to `DEFAULT_IMAGE_HANDLE` (unless it is the first item in the UI phase items list). --- crates/bevy_ui/src/render/mod.rs | 45 +++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 40985ad237f0f..a0a332bb61747 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -732,6 +732,7 @@ pub fn queue_uinodes( transparent_phase .items .reserve(extracted_uinodes.uinodes.len()); + for (entity, extracted_uinode) in extracted_uinodes.uinodes.iter() { transparent_phase.add(TransparentUi { draw_function, @@ -781,11 +782,6 @@ pub fn prepare_uinodes( }; } - #[inline] - fn is_textured(image: AssetId) -> bool { - image != AssetId::default() - } - if let Some(view_binding) = view_uniforms.uniforms.binding() { let mut batches: Vec<(Entity, UiBatch)> = Vec::with_capacity(*previous_len); @@ -798,7 +794,6 @@ pub fn prepare_uinodes( // Vertex buffer index let mut index = 0; - for mut ui_phase in &mut phases { let mut batch_item_index = 0; let mut batch_image_handle = AssetId::invalid(); @@ -806,11 +801,14 @@ pub fn prepare_uinodes( for item_index in 0..ui_phase.items.len() { let item = &mut ui_phase.items[item_index]; if let Some(extracted_uinode) = extracted_uinodes.uinodes.get(&item.entity) { - let mut existing_batch = batches - .last_mut() - .filter(|_| batch_image_handle == extracted_uinode.image); - - if existing_batch.is_none() { + let mut existing_batch = batches.last_mut(); + + if batch_image_handle == AssetId::invalid() + || existing_batch.is_none() + || (batch_image_handle != AssetId::default() + && extracted_uinode.image != AssetId::default() + && batch_image_handle != extracted_uinode.image) + { if let Some(gpu_image) = gpu_images.get(extracted_uinode.image) { batch_item_index = item_index; batch_image_handle = extracted_uinode.image; @@ -840,9 +838,32 @@ pub fn prepare_uinodes( } else { continue; } + } else if batch_image_handle == AssetId::default() + && extracted_uinode.image != AssetId::default() + { + if let Some(gpu_image) = gpu_images.get(extracted_uinode.image) { + batch_image_handle = extracted_uinode.image; + existing_batch.as_mut().unwrap().1.image = extracted_uinode.image; + + image_bind_groups + .values + .entry(batch_image_handle) + .or_insert_with(|| { + render_device.create_bind_group( + "ui_material_bind_group", + &ui_pipeline.image_layout, + &BindGroupEntries::sequential(( + &gpu_image.texture_view, + &gpu_image.sampler, + )), + ) + }); + } else { + continue; + } } - let mode = if is_textured(extracted_uinode.image) { + let mode = if extracted_uinode.image != AssetId::default() { TEXTURED_QUAD } else { UNTEXTURED_QUAD