Uncouple DynamicTextureAtlasBuilder from assets (#13717)

# Objective

Remove some unnecessary coupling between `DynamicTextureAtlasBuilder`
and `bevy_asset`.

## Solution

Remove the dependency of `DynamicTextureAtlasBuilder::add_texture` to
`bevy_asset`, by directly passing the `Image` of the atlas to mutate,
instead of passing separate `Assets<Image>` and `Handle<Image>` for the
function to do the lookup by itself. The lookup can be done from the
caller, and this allows using the builder in contexts where the `Image`
is not stored inside `Assets`.

Clean-up a bit the font atlas files by introducing a `PlacedGlyph` type
storing the `GlyphId` and its `SubpixelOffset`, which were otherwise
always both passed as function parameters and the pair used as key in
hash maps.

## Testing

There's no change in behavior.

---

## Changelog

- Added a `PlacedGlyph` type aggregating a `GlyphId` and a
`SubpixelOffset`. That type is now used as parameter in a few text atlas
APIs, instead of passing individual values.

## Migration Guide

- Replace the `glyph_id` and `subpixel_offset` of a few text atlas APIs
by a single `place_glyph: PlacedGlyph` parameter trivially combining the
two.

crates/bevy_sprite/src/dynamic_texture_atlas_builder.rs

@@ -1,5 +1,4 @@
 use crate::TextureAtlasLayout;
-use bevy_asset::{Assets, Handle};
 use bevy_math::{URect, UVec2};
 use bevy_render::{
     render_asset::{RenderAsset, RenderAssetUsages},
@@ -38,23 +37,20 @@ impl DynamicTextureAtlasBuilder {
     ///
     /// # Arguments
     ///
-    /// * `altas_layout` - The atlas to add the texture to
+    /// * `altas_layout` - The atlas layout to add the texture to.
-    /// * `textures` - The texture assets container
+    /// * `texture` - The source texture to add to the atlas.
-    /// * `texture` - The new texture to add to the atlas
+    /// * `atlas_texture` - The destination atlas texture to copy the source texture to.
-    /// * `atlas_texture_handle` - The atlas texture to edit
     pub fn add_texture(
         &mut self,
         atlas_layout: &mut TextureAtlasLayout,
-        textures: &mut Assets<Image>,
         texture: &Image,
-        atlas_texture_handle: &Handle<Image>,
+        atlas_texture: &mut Image,
     ) -> Option<usize> {
         let allocation = self.atlas_allocator.allocate(size2(
             (texture.width() + self.padding).try_into().unwrap(),
             (texture.height() + self.padding).try_into().unwrap(),
         ));
         if let Some(allocation) = allocation {
-            let atlas_texture = textures.get_mut(atlas_texture_handle).unwrap();
             assert!(
                 <GpuImage as RenderAsset>::asset_usage(atlas_texture)
                     .contains(RenderAssetUsages::MAIN_WORLD),

crates/bevy_text/src/font_atlas.rs

@@ -40,9 +40,18 @@ impl From<Point> for SubpixelOffset {
     }
 }
 
+/// A font glyph placed at a specific sub-pixel offset.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct PlacedGlyph {
+    /// The font glyph ID.
+    pub glyph_id: GlyphId,
+    /// The sub-pixel offset of the placed glyph.
+    pub subpixel_offset: SubpixelOffset,
+}
+
 pub struct FontAtlas {
     pub dynamic_texture_atlas_builder: DynamicTextureAtlasBuilder,
-    pub glyph_to_atlas_index: HashMap<(GlyphId, SubpixelOffset), usize>,
+    pub glyph_to_atlas_index: HashMap<PlacedGlyph, usize>,
     pub texture_atlas: Handle<TextureAtlasLayout>,
     pub texture: Handle<Image>,
 }
@@ -74,38 +83,44 @@ impl FontAtlas {
         }
     }
 
-    pub fn get_glyph_index(
+    pub fn get_glyph_index(&self, glyph: &PlacedGlyph) -> Option<usize> {
-        &self,
+        self.glyph_to_atlas_index.get(glyph).copied()
-        glyph_id: GlyphId,
-        subpixel_offset: SubpixelOffset,
-    ) -> Option<usize> {
-        self.glyph_to_atlas_index
-            .get(&(glyph_id, subpixel_offset))
-            .copied()
     }
 
-    pub fn has_glyph(&self, glyph_id: GlyphId, subpixel_offset: SubpixelOffset) -> bool {
+    pub fn has_glyph(&self, glyph: &PlacedGlyph) -> bool {
-        self.glyph_to_atlas_index
+        self.glyph_to_atlas_index.contains_key(glyph)
-            .contains_key(&(glyph_id, subpixel_offset))
     }
 
+    /// Add a glyph to the atlas, updating both its texture and layout.
+    ///
+    /// The glyph is represented by `glyph`, and its image content is `glyph_texture`.
+    /// This content is copied into the atlas texture, and the atlas layout is updated
+    /// to store the location of that glyph into the atlas.
+    ///
+    /// # Returns
+    ///
+    /// Returns `true` if the glyph is successfully added, or `false` otherwise.
+    /// In that case, neither the atlas texture nor the atlas layout are
+    /// modified.
     pub fn add_glyph(
         &mut self,
         textures: &mut Assets<Image>,
-        texture_atlases: &mut Assets<TextureAtlasLayout>,
+        atlas_layouts: &mut Assets<TextureAtlasLayout>,
-        glyph_id: GlyphId,
+        glyph: &PlacedGlyph,
-        subpixel_offset: SubpixelOffset,
+        glyph_texture: &Image,
-        texture: &Image,
     ) -> bool {
-        let texture_atlas = texture_atlases.get_mut(&self.texture_atlas).unwrap();
+        let Some(atlas_layout) = atlas_layouts.get_mut(&self.texture_atlas) else {
+            return false;
+        };
+        let Some(atlas_texture) = textures.get_mut(&self.texture) else {
+            return false;
+        };
         if let Some(index) = self.dynamic_texture_atlas_builder.add_texture(
-            texture_atlas,
+            atlas_layout,
-            textures,
+            glyph_texture,
-            texture,
+            atlas_texture,
-            &self.texture,
         ) {
-            self.glyph_to_atlas_index
+            self.glyph_to_atlas_index.insert(*glyph, index);
-                .insert((glyph_id, subpixel_offset), index);
             true
         } else {
             false

crates/bevy_text/src/font_atlas_set.rs

@@ -1,4 +1,4 @@
-use crate::{error::TextError, Font, FontAtlas};
+use crate::{error::TextError, Font, FontAtlas, PlacedGlyph};
 use ab_glyph::{GlyphId, OutlinedGlyph, Point};
 use bevy_asset::{AssetEvent, AssetId};
 use bevy_asset::{Assets, Handle};
@@ -64,9 +64,13 @@ impl FontAtlasSet {
         self.font_atlases
             .get(&FloatOrd(font_size))
             .map_or(false, |font_atlas| {
+                let placed_glyph = PlacedGlyph {
+                    glyph_id,
+                    subpixel_offset: glyph_position.into(),
+                };
                 font_atlas
                     .iter()
-                    .any(|atlas| atlas.has_glyph(glyph_id, glyph_position.into()))
+                    .any(|atlas| atlas.has_glyph(&placed_glyph))
             })
     }
 
@@ -77,8 +81,10 @@ impl FontAtlasSet {
         outlined_glyph: OutlinedGlyph,
     ) -> Result<GlyphAtlasInfo, TextError> {
         let glyph = outlined_glyph.glyph();
-        let glyph_id = glyph.id;
+        let placed_glyph = PlacedGlyph {
-        let glyph_position = glyph.position;
+            glyph_id: glyph.id,
+            subpixel_offset: glyph.position.into(),
+        };
         let font_size = glyph.scale.y;
         let font_atlases = self
             .font_atlases
@@ -87,13 +93,7 @@ impl FontAtlasSet {
 
         let glyph_texture = Font::get_outlined_glyph_texture(outlined_glyph);
         let add_char_to_font_atlas = |atlas: &mut FontAtlas| -> bool {
-            atlas.add_glyph(
+            atlas.add_glyph(textures, texture_atlases, &placed_glyph, &glyph_texture)
-                textures,
-                texture_atlases,
-                glyph_id,
-                glyph_position.into(),
-                &glyph_texture,
-            )
         };
         if !font_atlases.iter_mut().any(add_char_to_font_atlas) {
             // Find the largest dimension of the glyph, either its width or its height
@@ -112,24 +112,20 @@ impl FontAtlasSet {
             if !font_atlases.last_mut().unwrap().add_glyph(
                 textures,
                 texture_atlases,
-                glyph_id,
+                &placed_glyph,
-                glyph_position.into(),
                 &glyph_texture,
             ) {
-                return Err(TextError::FailedToAddGlyph(glyph_id));
+                return Err(TextError::FailedToAddGlyph(placed_glyph.glyph_id));
             }
         }
 
-        Ok(self
+        Ok(self.get_glyph_atlas_info(font_size, &placed_glyph).unwrap())
-            .get_glyph_atlas_info(font_size, glyph_id, glyph_position)
-            .unwrap())
     }
 
     pub fn get_glyph_atlas_info(
         &mut self,
         font_size: f32,
-        glyph_id: GlyphId,
+        placed_glyph: &PlacedGlyph,
-        position: Point,
     ) -> Option<GlyphAtlasInfo> {
         self.font_atlases
             .get(&FloatOrd(font_size))
@@ -137,15 +133,13 @@ impl FontAtlasSet {
                 font_atlases
                     .iter()
                     .find_map(|atlas| {
-                        atlas
+                        atlas.get_glyph_index(placed_glyph).map(|glyph_index| {
-                            .get_glyph_index(glyph_id, position.into())
+                            (
-                            .map(|glyph_index| {
+                                glyph_index,
-                                (
+                                atlas.texture_atlas.clone_weak(),
-                                    glyph_index,
+                                atlas.texture.clone_weak(),
-                                    atlas.texture_atlas.clone_weak(),
+                            )
-                                    atlas.texture.clone_weak(),
+                        })
-                                )
-                            })
                     })
                     .map(|(glyph_index, texture_atlas, texture)| GlyphAtlasInfo {
                         texture_atlas,

crates/bevy_text/src/glyph_brush.rs

@@ -12,7 +12,7 @@ use glyph_brush_layout::{
 
 use crate::{
     error::TextError, BreakLineOn, Font, FontAtlasSet, FontAtlasSets, GlyphAtlasInfo, JustifyText,
-    TextSettings, YAxisOrientation,
+    PlacedGlyph, TextSettings, YAxisOrientation,
 };
 
 pub struct GlyphBrush {
@@ -94,8 +94,10 @@ impl GlyphBrush {
                 mut glyph,
                 font_id: _,
             } = sg;
-            let glyph_id = glyph.id;
+            let placed_glyph = PlacedGlyph {
-            let glyph_position = glyph.position;
+                glyph_id: glyph.id,
+                subpixel_offset: glyph.position.into(),
+            };
             let adjust = GlyphPlacementAdjuster::new(&mut glyph);
             let section_data = sections_data[sg.section_index];
             if let Some(outlined_glyph) = section_data.1.font.outline_glyph(glyph) {
@@ -106,7 +108,7 @@ impl GlyphBrush {
                     .or_insert_with(FontAtlasSet::default);
 
                 let atlas_info = font_atlas_set
-                    .get_glyph_atlas_info(section_data.2, glyph_id, glyph_position)
+                    .get_glyph_atlas_info(section_data.2, &placed_glyph)
                     .map(Ok)
                     .unwrap_or_else(|| {
                         font_atlas_set.add_glyph_to_atlas(texture_atlases, textures, outlined_glyph)