From 1f476cbac5688b78ae9b40ede8fc88b72711f414 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 28 Aug 2023 16:07:47 +0100 Subject: [PATCH 1/5] Fixed UI batching so new batches aren't created for untextured nodes. Thee 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 and only add it if there is only one batch but this crashed. --- crates/bevy_ui/src/render/mod.rs | 130 +++++++++++++++++-------------- 1 file changed, 73 insertions(+), 57 deletions(-) diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 1305093c05047..67742fbab4891 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -38,8 +38,9 @@ use bevy_sprite::TextureAtlas; use bevy_text::{PositionedGlyph, Text, TextLayoutInfo}; use bevy_transform::components::GlobalTransform; use bevy_utils::HashMap; -use bevy_utils::{FloatOrd, Uuid}; +use bevy_utils::FloatOrd; use bytemuck::{Pod, Zeroable}; +use std::mem::replace; use std::ops::Range; pub mod node { @@ -663,6 +664,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, @@ -706,11 +708,6 @@ pub fn prepare_uinodes( }; } - #[inline] - fn is_textured(image: &Handle) -> bool { - image.id() != DEFAULT_IMAGE_HANDLE.id() - } - if let Some(view_binding) = view_uniforms.uniforms.binding() { let mut batches: Vec<(Entity, UiBatch)> = Vec::with_capacity(*previous_len); @@ -726,64 +723,85 @@ 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 = HandleId::Id(Uuid::nil(), u64::MAX); + let mut batch_image_handle = DEFAULT_IMAGE_HANDLE.id(); + + if let Some(gpu_image) = gpu_images.get(&DEFAULT_IMAGE_HANDLE.typed()) { + image_bind_groups + .values + .entry(Handle::weak(DEFAULT_IMAGE_HANDLE.id())) + .or_insert_with(|| { + render_device.create_bind_group(&BindGroupDescriptor { + entries: &[ + BindGroupEntry { + binding: 0, + resource: BindingResource::TextureView(&gpu_image.texture_view), + }, + BindGroupEntry { + binding: 1, + resource: BindingResource::Sampler(&gpu_image.sampler), + }, + ], + label: Some("ui_material_bind_group"), + layout: &ui_pipeline.image_layout, + }) + }); + } 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.id()); - - if existing_batch.is_none() { - if let Some(gpu_image) = gpu_images.get(&extracted_uinode.image) { - batch_item_index = item_index; - batch_image_handle = extracted_uinode.image.id(); - - let new_batch = UiBatch { - range: index..index, - image_handle_id: extracted_uinode.image.id(), - }; - - batches.push((item.entity, new_batch)); - - image_bind_groups - .values - .entry(Handle::weak(batch_image_handle)) - .or_insert_with(|| { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView( - &gpu_image.texture_view, - ), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler( - &gpu_image.sampler, - ), - }, - ], - label: Some("ui_material_bind_group"), - layout: &ui_pipeline.image_layout, - }) - }); - - existing_batch = batches.last_mut(); + let existing_batch = if extracted_uinode.image.id() == DEFAULT_IMAGE_HANDLE.id() + || extracted_uinode.image.id() == batch_image_handle + { + batches.last_mut() + } else if let Some(gpu_image) = gpu_images.get(&extracted_uinode.image) { + image_bind_groups + .values + .entry(Handle::weak(extracted_uinode.image.id())) + .or_insert_with(|| { + render_device.create_bind_group(&BindGroupDescriptor { + entries: &[ + BindGroupEntry { + binding: 0, + resource: BindingResource::TextureView( + &gpu_image.texture_view, + ), + }, + BindGroupEntry { + binding: 1, + resource: BindingResource::Sampler(&gpu_image.sampler), + }, + ], + label: Some("ui_material_bind_group"), + layout: &ui_pipeline.image_layout, + }) + }); + if replace(&mut batch_image_handle, extracted_uinode.image.id()) == DEFAULT_IMAGE_HANDLE.id() { + let existing_batch = batches.last_mut().unwrap(); + existing_batch.1.image_handle_id = extracted_uinode.image.id(); + Some(existing_batch) } else { - continue; + None } + } else { + continue; + }; + if existing_batch.is_none() { + batch_item_index = item_index; + let new_batch = UiBatch { + range: index..index, + image_handle_id: extracted_uinode.image.id(), + }; + + batches.push((item.entity, new_batch)); } - let mode = if is_textured(&extracted_uinode.image) { - TEXTURED_QUAD - } else { + let mode = if extracted_uinode.image.id() == DEFAULT_IMAGE_HANDLE.id() { UNTEXTURED_QUAD + } else { + TEXTURED_QUAD }; let mut uinode_rect = extracted_uinode.rect; @@ -893,11 +911,9 @@ pub fn prepare_uinodes( }); } index += QUAD_INDICES.len() as u32; - existing_batch.unwrap().1.range.end = index; + batches.last_mut().unwrap().1.range.end = index; ui_phase.items[batch_item_index].batch_size += 1; - } else { - batch_image_handle = HandleId::Id(Uuid::nil(), u64::MAX); - } + } } } ui_meta.vertices.write_buffer(&render_device, &render_queue); From 6891a81f6bae4b445fea55c1da90d598cb5fd5b2 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 28 Aug 2023 16:27:01 +0100 Subject: [PATCH 2/5] cargo fmt --all --- crates/bevy_ui/src/render/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 67742fbab4891..151d360d0e4a5 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -37,8 +37,8 @@ use bevy_sprite::TextureAtlas; #[cfg(feature = "bevy_text")] use bevy_text::{PositionedGlyph, Text, TextLayoutInfo}; use bevy_transform::components::GlobalTransform; -use bevy_utils::HashMap; use bevy_utils::FloatOrd; +use bevy_utils::HashMap; use bytemuck::{Pod, Zeroable}; use std::mem::replace; use std::ops::Range; @@ -778,7 +778,9 @@ pub fn prepare_uinodes( layout: &ui_pipeline.image_layout, }) }); - if replace(&mut batch_image_handle, extracted_uinode.image.id()) == DEFAULT_IMAGE_HANDLE.id() { + if replace(&mut batch_image_handle, extracted_uinode.image.id()) + == DEFAULT_IMAGE_HANDLE.id() + { let existing_batch = batches.last_mut().unwrap(); existing_batch.1.image_handle_id = extracted_uinode.image.id(); Some(existing_batch) @@ -913,7 +915,7 @@ pub fn prepare_uinodes( index += QUAD_INDICES.len() as u32; batches.last_mut().unwrap().1.range.end = index; ui_phase.items[batch_item_index].batch_size += 1; - } + } } } ui_meta.vertices.write_buffer(&render_device, &render_queue); From 4196697c940757b0f6d4f6ddb4187bff6bac1629 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 28 Aug 2023 16:54:15 +0100 Subject: [PATCH 3/5] Fixed panic, can't just unwrap the last element from batches to change the texture handle as we might not have created any batches yet. --- crates/bevy_ui/src/render/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 151d360d0e4a5..ac8aae5ac4413 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -781,9 +781,10 @@ pub fn prepare_uinodes( if replace(&mut batch_image_handle, extracted_uinode.image.id()) == DEFAULT_IMAGE_HANDLE.id() { - let existing_batch = batches.last_mut().unwrap(); - existing_batch.1.image_handle_id = extracted_uinode.image.id(); - Some(existing_batch) + batches.last_mut().map(|existing_batch| { + existing_batch.1.image_handle_id = batch_image_handle; + existing_batch + }) } else { None } From 0f9f483590e4e14cd77c862178f35772b284602b Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Tue, 31 Oct 2023 15:00:57 +0000 Subject: [PATCH 4/5] Fixed batching to use minimal draw calls again. --- crates/bevy_ui/src/render/mod.rs | 44 ++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index a7a52c628319e..6c0b5046e9e65 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -3,6 +3,7 @@ mod render_pass; use bevy_core_pipeline::{core_2d::Camera2d, core_3d::Camera3d}; use bevy_hierarchy::Parent; +use bevy_render::extract_instances::ExtractInstance; use bevy_render::render_phase::PhaseItem; use bevy_render::view::ViewVisibility; use bevy_render::{render_resource::BindGroupEntries, ExtractSchedule, Render}; @@ -803,11 +804,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); @@ -828,11 +824,15 @@ 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; @@ -862,9 +862,30 @@ 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 @@ -986,6 +1007,7 @@ pub fn prepare_uinodes( } ui_meta.vertices.write_buffer(&render_device, &render_queue); *previous_len = batches.len(); + println!("{}", batches.len()); commands.insert_or_spawn_batch(batches); } extracted_uinodes.uinodes.clear(); From 2b5dac4c495be008bf506d9c50f9c32cecaa4a6a Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Tue, 31 Oct 2023 15:21:31 +0000 Subject: [PATCH 5/5] cargo fmt --all --- crates/bevy_ui/src/render/mod.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 36a3aff778adc..511be30d4fdcd 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -824,14 +824,13 @@ pub fn prepare_uinodes( 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(); - + 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 - ) { + || 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; @@ -861,7 +860,9 @@ pub fn prepare_uinodes( } else { continue; } - } else if batch_image_handle == AssetId::default() && extracted_uinode.image != AssetId::default() { + } 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;