From 86b1a66af080159770d2c2cd98a809e10896a190 Mon Sep 17 00:00:00 2001
From: Chad Brokaw <cbrokaw@gmail.com>
Date: Mon, 6 Mar 2023 18:29:43 -0500
Subject: [PATCH] Address review feedback

* reorganize GlyphCache::get_or_insert() to use the HashMap entry API
* change DrawGlyphs::hint() parameter name from `yes` to `hint` for clarity
* change Layout::path_data() to return bytes instead of [f32; 2]. Path segments could be encoded as i16, making this incorrect
---
 src/encoding/glyph_cache.rs | 27 +++++++++++++++------------
 src/encoding/resolve.rs     |  2 +-
 src/scene.rs                |  4 ++--
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/encoding/glyph_cache.rs b/src/encoding/glyph_cache.rs
index 6cb0ba8..cbfea17 100644
--- a/src/encoding/glyph_cache.rs
+++ b/src/encoding/glyph_cache.rs
@@ -48,21 +48,24 @@ impl GlyphCache {
         style: &Style,
         scaler: &mut GlyphProvider,
     ) -> Option<CachedRange> {
+        let encoding_cache = &mut self.encoding;
+        let mut encode_glyph = || {
+            let start = encoding_cache.stream_offsets();
+            scaler.encode_glyph(key.glyph_id as u16, style, encoding_cache)?;
+            let end = encoding_cache.stream_offsets();
+            Some(CachedRange { start, end })
+        };
         // For now, only cache non-zero filled glyphs so we don't need to keep style
         // as part of the key.
-        let is_nz_fill = matches!(style, Style::Fill(Fill::NonZero));
-        if is_nz_fill {
-            if let Some(range) = self.glyphs.get(&key) {
-                return Some(*range);
+        let range = if matches!(style, Style::Fill(Fill::NonZero)) {
+            use std::collections::hash_map::Entry;
+            match self.glyphs.entry(key) {
+                Entry::Occupied(entry) => *entry.get(),
+                Entry::Vacant(entry) => *entry.insert(encode_glyph()?),
             }
-        }
-        let start = self.encoding.stream_offsets();
-        scaler.encode_glyph(key.glyph_id as u16, style, &mut self.encoding)?;
-        let end = self.encoding.stream_offsets();
-        let range = CachedRange { start, end };
-        if is_nz_fill {
-            self.glyphs.insert(key, range);
-        }
+        } else {
+            encode_glyph()?
+        };
         Some(range)
     }
 }
diff --git a/src/encoding/resolve.rs b/src/encoding/resolve.rs
index 5c77c49..d36a05b 100644
--- a/src/encoding/resolve.rs
+++ b/src/encoding/resolve.rs
@@ -74,7 +74,7 @@ impl Layout {
     }
 
     /// Returns the path data stream.
-    pub fn path_data<'a>(&self, data: &'a [u8]) -> &'a [[f32; 2]] {
+    pub fn path_data<'a>(&self, data: &'a [u8]) -> &'a [u8] {
         let start = self.path_data_base as usize * 4;
         let end = self.draw_tag_base as usize * 4;
         bytemuck::cast_slice(&data[start..end])
diff --git a/src/scene.rs b/src/scene.rs
index bf73906..29fba23 100644
--- a/src/scene.rs
+++ b/src/scene.rs
@@ -245,8 +245,8 @@ impl<'a> DrawGlyphs<'a> {
     /// Sets whether to enable hinting.
     ///
     /// The default value is `false`.
-    pub fn hint(mut self, yes: bool) -> Self {
-        self.run.hint = yes;
+    pub fn hint(mut self, hint: bool) -> Self {
+        self.run.hint = hint;
         self
     }