Bug in the glyph caching system

[KnorrFG]

Hey, running this program

use macroquad::{
    prelude::*,
    ui::{root_ui, widgets::Button, Skin},
};

#[macroquad::main("a bug")]
async fn main() {
    let font = load_ttf_font("OpenSans-Regular.ttf").await.unwrap();

    let button_style = root_ui()
        .style_builder()
        .with_font(&font)
        .unwrap()
        // .text_color(Color::from_rgba(180, 180, 100, 255))
        .font_size(23)
        .build();

    let button_skin = Skin {
        button_style,
        ..root_ui().default_skin()
    };

    root_ui().push_skin(&button_skin);
    measure_text("I'd love to.", Some(&font), 23, 1.0);
    measure_text("Sorry, I already drunk too much", Some(&font), 23, 1.0);

    loop {
        Button::new("I'd love to.")
            .position(vec2(0., 0.))
            .ui(&mut root_ui());
        next_frame().await;
    }
}

will crash with the following error:

thread 'main' panicked at /home/felix/.cargo/registry/src/index.crates.io-6f17d22bba15001f/macroquad-0.4.13/src/text.rs:171:53:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I tried to find out what the problem is, and it seems that the entry for the character disappears from the font-atlas for some reason, but not from the characters map. Sadly, I have no idea why. I'll attach the font file.

OpenSans-Regular.zip

It seems the cache is corrupted when called from the buttons ui method, but still ok at the end of the 2nd measure_text call in my code. To me, it looked like the only point in time where something can disappear from the cache, is during a texture update, when the texture needs to be resized, however, as far as I could tell, this didn't happen. Is there any other path to modifying the atlas? I'd be happy to commit a fix for that, but the input from someone more familiar with this code would be useful.

[InnocentusLime]

I think I might have a hypothesis about what is going wrong (aka definitely a bug), will try to patch it late evening.

Basically what happens. A font is a bunch of smart pointers:

#[derive(Clone)]
pub struct Font {
    font: Arc<fontdue::Font>,
    atlas: Arc<Mutex<Atlas>>,
    characters: Arc<Mutex<HashMap<(char, u16), CharacterInfo>>>,
}

The issue at hand happens with the atlas field. Style::with_font makes the clone_font point to a new atlas while font keeps pointing to a different atlas.

When you call .with_font(), the following happens:

pub fn with_font(self, font: &Font) -> Result<StyleBuilder, Error> {
  let mut clone_font = font.clone(); //renamed to `clone_font` for clarity
  font.set_atlas(self.atlas.clone());
  Ok(StyleBuilder {
    font: Arc::new(Mutex::new(clone_font)),
    ..self
  })
}

However, both clone_font and font still share the same characters field. That means if font was to cache some glyphs -- this would be immediately reflected in clone_font, but since it technically has its own atlas copy -- it doesn't get updated.

So when you call measure_text, a stunning desync happens, where clone_font is in a partially correct state. As Button tries to render it :)

[InnocentusLime]

I have PR'd the fix. Let me know if it doesn't work!