Switched the font atlas to discrete math for hash keys (#164822)

issue: https://github.com/flutter/flutter/issues/164606

This is a refactor of how we store font atlas data. Previously we were
using floating point values with custom hash functions as keys. This was
very hard to debug. This change instead makes all keys used in the font
atlas discrete.

The hope was that this might fix
https://github.com/flutter/flutter/issues/164606, or at least make it
easier to debug. It didn't fix the issue.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

engine/src/flutter/ci/licenses_golden/excluded_files

@@ -174,6 +174,7 @@
 ../../../flutter/impeller/geometry/geometry_unittests.cc
 ../../../flutter/impeller/geometry/matrix_unittests.cc
 ../../../flutter/impeller/geometry/path_unittests.cc
+../../../flutter/impeller/geometry/rational_unittests.cc
 ../../../flutter/impeller/geometry/rect_unittests.cc
 ../../../flutter/impeller/geometry/round_rect_unittests.cc
 ../../../flutter/impeller/geometry/round_superellipse_unittests.cc

engine/src/flutter/ci/licenses_golden/licenses_flutter

@@ -41860,6 +41860,8 @@ ORIGIN: ../../../flutter/impeller/geometry/point.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/geometry/point.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/geometry/quaternion.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/geometry/quaternion.h + ../../../flutter/LICENSE
+ORIGIN: ../../../flutter/impeller/geometry/rational.cc + ../../../flutter/LICENSE
+ORIGIN: ../../../flutter/impeller/geometry/rational.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/geometry/rect.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/geometry/rect.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/geometry/round_rect.cc + ../../../flutter/LICENSE
@@ -44821,6 +44823,8 @@ FILE: ../../../flutter/impeller/geometry/point.cc
 FILE: ../../../flutter/impeller/geometry/point.h
 FILE: ../../../flutter/impeller/geometry/quaternion.cc
 FILE: ../../../flutter/impeller/geometry/quaternion.h
+FILE: ../../../flutter/impeller/geometry/rational.cc
+FILE: ../../../flutter/impeller/geometry/rational.h
 FILE: ../../../flutter/impeller/geometry/rect.cc
 FILE: ../../../flutter/impeller/geometry/rect.h
 FILE: ../../../flutter/impeller/geometry/round_rect.cc

engine/src/flutter/impeller/entity/contents/text_contents.cc

@@ -105,7 +105,7 @@ void TextContents::ComputeVertexData(
   size_t bounds_offset = 0u;
   for (const TextRun& run : frame->GetRuns()) {
     const Font& font = run.GetFont();
-    Scalar rounded_scale = frame->GetScale();
+    Rational rounded_scale = frame->GetScale();
     const Matrix transform = frame->GetOffsetTransform();
     FontGlyphAtlas* font_atlas = nullptr;
 
@@ -149,7 +149,7 @@ void TextContents::ComputeVertexData(
           VALIDATION_LOG << "Could not find font in the atlas.";
           continue;
         }
-        Point subpixel = TextFrame::ComputeSubpixelPosition(
+        SubpixelPosition subpixel = TextFrame::ComputeSubpixelPosition(
             glyph_position, font.GetAxisAlignment(), transform);
 
         std::optional<FrameBounds> maybe_atlas_glyph_bounds =
@@ -165,7 +165,8 @@ void TextContents::ComputeVertexData(
         atlas_glyph_bounds = maybe_atlas_glyph_bounds.value().atlas_bounds;
       }
 
-      Rect scaled_bounds = glyph_bounds.Scale(1.0 / rounded_scale);
+      Rect scaled_bounds =
+          glyph_bounds.Scale(static_cast<Scalar>(rounded_scale.Invert()));
       // For each glyph, we compute two rectangles. One for the vertex
       // positions and one for the texture coordinates (UVs). The atlas
       // glyph bounds are used to compute UVs in cases where the

engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc

@@ -54,13 +54,15 @@ std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
     const TypographerContext* typographer_context,
     HostBuffer& host_buffer,
     GlyphAtlas::Type type,
-    Scalar scale,
+    Rational scale,
     const std::shared_ptr<GlyphAtlasContext>& atlas_context,
     const std::shared_ptr<TextFrame>& frame,
     Point offset) {
   frame->SetPerFrameData(
       TextFrame::RoundScaledFontSize(scale), /*offset=*/offset,
-      /*transform=*/Matrix::MakeScale(Vector3{scale, scale, 1}),
+      /*transform=*/
+      Matrix::MakeScale(
+          Vector3{static_cast<Scalar>(scale), static_cast<Scalar>(scale), 1}),
       /*properties=*/std::nullopt);
   return typographer_context->CreateGlyphAtlas(context, type, host_buffer,
                                                atlas_context, {frame});
@@ -122,7 +124,7 @@ TEST_P(TextContentsTest, SimpleComputeVertexData) {
   ASSERT_TRUE(context && context->IsValid());
   std::shared_ptr<GlyphAtlas> atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/Rational(1, 1),
                        atlas_context, text_frame, /*offset=*/{0, 0});
 
   ISize texture_size = atlas->GetTexture()->GetSize();
@@ -156,7 +158,7 @@ TEST_P(TextContentsTest, SimpleComputeVertexData2x) {
   std::shared_ptr<HostBuffer> host_buffer = HostBuffer::Create(
       GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter());
   ASSERT_TRUE(context && context->IsValid());
-  Scalar font_scale = 2.f;
+  Rational font_scale(2, 1);
   std::shared_ptr<GlyphAtlas> atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
                        GlyphAtlas::Type::kAlphaBitmap, font_scale,
@@ -164,8 +166,10 @@ TEST_P(TextContentsTest, SimpleComputeVertexData2x) {
 
   ISize texture_size = atlas->GetTexture()->GetSize();
   TextContents::ComputeVertexData(
-      data, text_frame, font_scale,
+      data, text_frame, static_cast<Scalar>(font_scale),
-      /*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}),
+      /*entity_transform=*/
+      Matrix::MakeScale({static_cast<Scalar>(font_scale),
+                         static_cast<Scalar>(font_scale), 1}),
       /*offset=*/Vector2(0, 0),
       /*glyph_properties=*/std::nullopt, atlas);
 
@@ -187,7 +191,7 @@ TEST_P(TextContentsTest, MaintainsShape) {
   ASSERT_TRUE(context && context->IsValid());
 
   for (int i = 0; i <= 1000; ++i) {
-    Scalar font_scale = 0.440 + (i / 1000.0);
+    Rational font_scale(440 + i, 1000.0);
     Rect position_rect[2];
     Rect uv_rect[2];
 
@@ -200,8 +204,10 @@ TEST_P(TextContentsTest, MaintainsShape) {
       ISize texture_size = atlas->GetTexture()->GetSize();
 
       TextContents::ComputeVertexData(
-          data, text_frame, font_scale,
+          data, text_frame, static_cast<Scalar>(font_scale),
-          /*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}),
+          /*entity_transform=*/
+          Matrix::MakeScale({static_cast<Scalar>(font_scale),
+                             static_cast<Scalar>(font_scale), 1}),
           /*offset=*/Vector2(0, 0),
           /*glyph_properties=*/std::nullopt, atlas);
       position_rect[0] = PerVertexDataPositionToRect(data);
@@ -234,7 +240,7 @@ TEST_P(TextContentsTest, SimpleSubpixel) {
   Point offset = Point(0.5, 0);
   std::shared_ptr<GlyphAtlas> atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/Rational(1),
                        atlas_context, text_frame, offset);
 
   ISize texture_size = atlas->GetTexture()->GetSize();
@@ -268,7 +274,7 @@ TEST_P(TextContentsTest, SimpleSubpixel3x) {
   std::shared_ptr<HostBuffer> host_buffer = HostBuffer::Create(
       GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter());
   ASSERT_TRUE(context && context->IsValid());
-  Scalar font_scale = 3.f;
+  Rational font_scale(3, 1);
   Point offset = {0.16667, 0};
   std::shared_ptr<GlyphAtlas> atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
@@ -277,10 +283,11 @@ TEST_P(TextContentsTest, SimpleSubpixel3x) {
 
   ISize texture_size = atlas->GetTexture()->GetSize();
   TextContents::ComputeVertexData(
-      data, text_frame, font_scale,
+      data, text_frame, static_cast<Scalar>(font_scale),
       /*entity_transform=*/
       Matrix::MakeTranslation(offset) *
-          Matrix::MakeScale({font_scale, font_scale, 1}),
+          Matrix::MakeScale({static_cast<Scalar>(font_scale),
+                             static_cast<Scalar>(font_scale), 1}),
       offset,
       /*glyph_properties=*/std::nullopt, atlas);
 
@@ -314,7 +321,7 @@ TEST_P(TextContentsTest, SimpleSubpixel26) {
   Point offset = Point(0.26, 0);
   std::shared_ptr<GlyphAtlas> atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/Rational(1),
                        atlas_context, text_frame, offset);
 
   ISize texture_size = atlas->GetTexture()->GetSize();
@@ -351,7 +358,7 @@ TEST_P(TextContentsTest, SimpleSubpixel80) {
   Point offset = Point(0.80, 0);
   std::shared_ptr<GlyphAtlas> atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                       GlyphAtlas::Type::kAlphaBitmap, /*scale=*/Rational(1),
                        atlas_context, text_frame, offset);
 
   ISize texture_size = atlas->GetTexture()->GetSize();

engine/src/flutter/impeller/entity/entity_unittests.cc

@@ -2110,11 +2110,11 @@ TEST_P(EntityTest, ColorFilterContentsWithLargeGeometry) {
 }
 
 TEST_P(EntityTest, TextContentsCeilsGlyphScaleToDecimal) {
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f), 0.43f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f), Rational(43, 100));
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f), 0.53f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f), Rational(53, 100));
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f), 2.1f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f), Rational(21, 10));
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f), 0.0f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f), Rational(0, 1));
-  ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f), 48.0f);
+  ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f), Rational(48, 1));
 }
 
 TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) {

engine/src/flutter/impeller/geometry/BUILD.gn

@@ -27,6 +27,8 @@ impeller_component("geometry") {
     "point.h",
     "quaternion.cc",
     "quaternion.h",
+    "rational.cc",
+    "rational.h",
     "rect.cc",
     "rect.h",
     "round_rect.cc",
@@ -80,6 +82,7 @@ impeller_component("geometry_unittests") {
     "geometry_unittests.cc",
     "matrix_unittests.cc",
     "path_unittests.cc",
+    "rational_unittests.cc",
     "rect_unittests.cc",
     "round_rect_unittests.cc",
     "round_superellipse_unittests.cc",

engine/src/flutter/impeller/geometry/rational.cc

@@ -0,0 +1,58 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/impeller/geometry/rational.h"
+
+#include <cmath>
+#include <cstdlib>
+#include <numeric>
+
+namespace impeller {
+namespace {
+uint32_t AbsToUnsigned(int32_t x) {
+  return static_cast<uint32_t>(std::abs(x));
+}
+}  // namespace
+
+bool Rational::operator==(const Rational& that) const {
+  if (den_ == that.den_) {
+    return num_ == that.num_;
+  } else if ((num_ >= 0) != (that.num_ >= 0)) {
+    return false;
+  } else {
+    return AbsToUnsigned(num_) * that.den_ == AbsToUnsigned(that.num_) * den_;
+  }
+}
+
+bool Rational::operator!=(const Rational& that) const {
+  return !(*this == that);
+}
+
+bool Rational::operator<(const Rational& that) const {
+  if (den_ == that.den_) {
+    return num_ < that.num_;
+  } else if ((num_ >= 0) != (that.num_ >= 0)) {
+    return num_ < that.num_;
+  } else {
+    return AbsToUnsigned(num_) * that.den_ < AbsToUnsigned(that.num_) * den_;
+  }
+}
+
+uint64_t Rational::GetHash() const {
+  if (num_ == 0) {
+    return 0;
+  }
+  uint64_t gcd = std::gcd(num_, den_);
+  return ((num_ / gcd) << 32) | (den_ / gcd);
+}
+
+Rational Rational::Invert() const {
+  if (num_ >= 0) {
+    return Rational(den_, num_);
+  } else {
+    return Rational(-1 * static_cast<int32_t>(den_), std::abs(num_));
+  }
+}
+
+}  // namespace impeller

engine/src/flutter/impeller/geometry/rational.h

@@ -0,0 +1,42 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_IMPELLER_GEOMETRY_RATIONAL_H_
+#define FLUTTER_IMPELLER_GEOMETRY_RATIONAL_H_
+
+#include <cstdint>
+#include "impeller/geometry/scalar.h"
+
+namespace impeller {
+
+class Rational {
+ public:
+  constexpr explicit Rational(int32_t num) : num_(num), den_(1) {}
+
+  constexpr Rational(int32_t num, uint32_t den) : num_(num), den_(den) {}
+
+  int32_t GetNumerator() const { return num_; }
+
+  uint32_t GetDenominator() const { return den_; }
+
+  bool operator==(const Rational& that) const;
+
+  bool operator!=(const Rational& that) const;
+
+  bool operator<(const Rational& that) const;
+
+  uint64_t GetHash() const;
+
+  explicit operator Scalar() const { return static_cast<float>(num_) / den_; }
+
+  Rational Invert() const;
+
+ private:
+  int32_t num_;
+  uint32_t den_;
+};
+
+}  // namespace impeller
+
+#endif  // FLUTTER_IMPELLER_GEOMETRY_RATIONAL_H_

engine/src/flutter/impeller/geometry/rational_unittests.cc

@@ -0,0 +1,57 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/impeller/geometry/rational.h"
+
+#include "gtest/gtest.h"
+
+namespace impeller {
+
+TEST(RationalTest, Make) {
+  Rational value(1, 2);
+  EXPECT_EQ(value.GetNumerator(), 1);
+  EXPECT_EQ(value.GetDenominator(), 2u);
+}
+
+TEST(RationalTest, EqualsSameDen) {
+  EXPECT_TRUE(Rational(1, 2) == Rational(1, 2));
+}
+
+TEST(RationalTest, NotEqualsSameDen) {
+  EXPECT_FALSE(Rational(3, 2) == Rational(1, 2));
+}
+
+TEST(RationalTest, EqualsDifferentDen) {
+  EXPECT_TRUE(Rational(1, 2) == Rational(2, 4));
+}
+
+TEST(RationalTest, NegationNotEquals) {
+  EXPECT_FALSE(Rational(1, 2) == Rational(-1, 2));
+}
+
+TEST(RationalTest, LessThanSameDen) {
+  EXPECT_TRUE(Rational(1, 2) < Rational(2, 2));
+}
+
+TEST(RationalTest, LessThanNegation) {
+  EXPECT_TRUE(Rational(-1, 2) < Rational(2, 23));
+}
+
+TEST(RationalTest, LessThanDifferentDen) {
+  EXPECT_TRUE(Rational(1, 2) < Rational(25, 23));
+}
+
+TEST(RationalTest, NotLessThanDifferentDen) {
+  EXPECT_FALSE(Rational(25, 23) < Rational(1, 2));
+}
+
+TEST(RationalTest, SameHashes) {
+  EXPECT_EQ(Rational(1, 2).GetHash(), Rational(2, 4).GetHash());
+}
+
+TEST(RationalTest, DifferentHashes) {
+  EXPECT_NE(Rational(2, 2).GetHash(), Rational(2, 4).GetHash());
+}
+
+}  // namespace impeller

engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc

@@ -205,6 +205,10 @@ static ISize ComputeNextAtlasSize(
   return {};
 }
 
+static Point SubpixelPositionToPoint(SubpixelPosition pos) {
+  return Point((pos & 0xff) / 4.f, (pos >> 2 & 0xff) / 4.f);
+}
+
 static void DrawGlyph(SkCanvas* canvas,
                       const SkPoint position,
                       const ScaledFont& scaled_font,
@@ -222,7 +226,7 @@ static void DrawGlyph(SkCanvas* canvas,
   sk_font.setHinting(SkFontHinting::kSlight);
   sk_font.setEmbolden(metrics.embolden);
   sk_font.setSubpixel(true);
-  sk_font.setSize(sk_font.getSize() * scaled_font.scale);
+  sk_font.setSize(sk_font.getSize() * static_cast<Scalar>(scaled_font.scale));
 
   auto glyph_color = prop.has_value() ? prop->color.ToARGB() : SK_ColorBLACK;
 
@@ -231,13 +235,15 @@ static void DrawGlyph(SkCanvas* canvas,
   glyph_paint.setBlendMode(SkBlendMode::kSrc);
   if (prop.has_value() && prop->stroke) {
     glyph_paint.setStroke(true);
-    glyph_paint.setStrokeWidth(prop->stroke_width * scaled_font.scale);
+    glyph_paint.setStrokeWidth(prop->stroke_width *
+                               static_cast<Scalar>(scaled_font.scale));
     glyph_paint.setStrokeCap(ToSkiaCap(prop->stroke_cap));
     glyph_paint.setStrokeJoin(ToSkiaJoin(prop->stroke_join));
     glyph_paint.setStrokeMiter(prop->stroke_miter);
   }
   canvas->save();
-  canvas->translate(glyph.subpixel_offset.x, glyph.subpixel_offset.y);
+  Point subpixel_offset = SubpixelPositionToPoint(glyph.subpixel_offset);
+  canvas->translate(subpixel_offset.x, subpixel_offset.y);
   canvas->drawGlyphs(1u,         // count
                      &glyph_id,  // glyphs
                      &position,  // positions
@@ -400,7 +406,7 @@ static Rect ComputeGlyphSize(const SkFont& font,
 
   // Expand the bounds of glyphs at subpixel offsets by 2 in the x direction.
   Scalar adjustment = 0.0;
-  if (glyph.subpixel_offset != Point(0, 0)) {
+  if (glyph.subpixel_offset != SubpixelPosition::kSubpixel00) {
     adjustment = 1.0;
   }
   return Rect::MakeLTRB(scaled_bounds.fLeft - adjustment, scaled_bounds.fTop,
@@ -453,11 +459,12 @@ TypographerContextSkia::CollectNewGlyphs(
       // Rather than computing the bounds at the requested point size and
       // scaling up the bounds, we scale up the font size and request the
       // bounds. This seems to give more accurate bounds information.
-      sk_font.setSize(sk_font.getSize() * scaled_font.scale);
+      sk_font.setSize(sk_font.getSize() *
+                      static_cast<Scalar>(scaled_font.scale));
       sk_font.setSubpixel(true);
 
       for (const auto& glyph_position : run.GetGlyphPositions()) {
-        Point subpixel = TextFrame::ComputeSubpixelPosition(
+        SubpixelPosition subpixel = TextFrame::ComputeSubpixelPosition(
             glyph_position, scaled_font.font.GetAxisAlignment(),
             frame->GetOffsetTransform());
         SubpixelGlyph subpixel_glyph(glyph_position.glyph, subpixel,
@@ -467,8 +474,8 @@ TypographerContextSkia::CollectNewGlyphs(
 
         if (!font_glyph_bounds.has_value()) {
           new_glyphs.push_back(FontGlyphPair{scaled_font, subpixel_glyph});
-          auto glyph_bounds =
+          auto glyph_bounds = ComputeGlyphSize(
-              ComputeGlyphSize(sk_font, subpixel_glyph, scaled_font.scale);
+              sk_font, subpixel_glyph, static_cast<Scalar>(scaled_font.scale));
           glyph_sizes.push_back(glyph_bounds);
 
           auto frame_bounds = FrameBounds{

engine/src/flutter/impeller/typographer/font_glyph_pair.h

@@ -8,6 +8,7 @@
 #include "impeller/geometry/color.h"
 #include "impeller/geometry/path.h"
 #include "impeller/geometry/point.h"
+#include "impeller/geometry/rational.h"
 #include "impeller/typographer/font.h"
 #include "impeller/typographer/glyph.h"
 
@@ -39,11 +40,11 @@ struct GlyphProperties {
 ///
 struct ScaledFont {
   Font font;
-  Scalar scale;
+  Rational scale;
 
   template <typename H>
   friend H AbslHashValue(H h, const ScaledFont& sf) {
-    return H::combine(std::move(h), sf.font.GetHash(), sf.scale);
+    return H::combine(std::move(h), sf.font.GetHash(), sf.scale.GetHash());
   }
 
   struct Equal {
@@ -54,16 +55,45 @@ struct ScaledFont {
   };
 };
 
+/// All possible positions for a subpixel alignment.
+/// The name is in the format kSubpixelXY where X and Y are numerators to 1/4
+/// fractions in their respective directions.
+enum SubpixelPosition : uint8_t {
+  // Subpixel at {0, 0}.
+  kSubpixel00 = 0x0,
+  // Subpixel at {0.25, 0}.
+  kSubpixel10 = 0x1,
+  // Subpixel at {0.5, 0}.
+  kSubpixel20 = 0x2,
+  // Subpixel at {0.75, 0}.
+  kSubpixel30 = 0x3,
+  // Subpixel at {0, 0.25}.
+  kSubpixel01 = kSubpixel10 << 2,
+  // Subpixel at {0, 0.5}.
+  kSubpixel02 = kSubpixel20 << 2,
+  // Subpixel at {0, 0.75}.
+  kSubpixel03 = kSubpixel30 << 2,
+  kSubpixel11 = kSubpixel10 | kSubpixel01,
+  kSubpixel12 = kSubpixel10 | kSubpixel02,
+  kSubpixel13 = kSubpixel10 | kSubpixel03,
+  kSubpixel21 = kSubpixel20 | kSubpixel01,
+  kSubpixel22 = kSubpixel20 | kSubpixel02,
+  kSubpixel23 = kSubpixel20 | kSubpixel03,
+  kSubpixel31 = kSubpixel30 | kSubpixel01,
+  kSubpixel32 = kSubpixel30 | kSubpixel02,
+  kSubpixel33 = kSubpixel30 | kSubpixel03,
+};
+
 //------------------------------------------------------------------------------
 /// @brief      A glyph and its subpixel position.
 ///
 struct SubpixelGlyph {
   Glyph glyph;
-  Point subpixel_offset;
+  SubpixelPosition subpixel_offset;
   std::optional<GlyphProperties> properties;
 
   SubpixelGlyph(Glyph p_glyph,
-                Point p_subpixel_offset,
+                SubpixelPosition p_subpixel_offset,
                 std::optional<GlyphProperties> p_properties)
       : glyph(p_glyph),
         subpixel_offset(p_subpixel_offset),
@@ -72,14 +102,12 @@ struct SubpixelGlyph {
   template <typename H>
   friend H AbslHashValue(H h, const SubpixelGlyph& sg) {
     if (!sg.properties.has_value()) {
-      return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset.x,
+      return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset);
-                        sg.subpixel_offset.y);
     }
-    return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset.x,
+    return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset,
-                      sg.subpixel_offset.y, sg.properties->color.ToARGB(),
+                      sg.properties->color.ToARGB(), sg.properties->stroke,
-                      sg.properties->stroke, sg.properties->stroke_cap,
+                      sg.properties->stroke_cap, sg.properties->stroke_join,
-                      sg.properties->stroke_join, sg.properties->stroke_miter,
+                      sg.properties->stroke_miter, sg.properties->stroke_width);
-                      sg.properties->stroke_width);
   }
 
   struct Equal {

engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc

@@ -32,7 +32,7 @@ LazyGlyphAtlas::LazyGlyphAtlas(
 LazyGlyphAtlas::~LazyGlyphAtlas() = default;
 
 void LazyGlyphAtlas::AddTextFrame(const std::shared_ptr<TextFrame>& frame,
-                                  Scalar scale,
+                                  Rational scale,
                                   Point offset,
                                   const Matrix& transform,
                                   std::optional<GlyphProperties> properties) {

engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h

@@ -5,6 +5,7 @@
 #ifndef FLUTTER_IMPELLER_TYPOGRAPHER_LAZY_GLYPH_ATLAS_H_
 #define FLUTTER_IMPELLER_TYPOGRAPHER_LAZY_GLYPH_ATLAS_H_
 
+#include "impeller/geometry/rational.h"
 #include "impeller/renderer/context.h"
 #include "impeller/typographer/glyph_atlas.h"
 #include "impeller/typographer/text_frame.h"
@@ -20,7 +21,7 @@ class LazyGlyphAtlas {
   ~LazyGlyphAtlas();
 
   void AddTextFrame(const std::shared_ptr<TextFrame>& frame,
-                    Scalar scale,
+                    Rational scale,
                     Point offset,
                     const Matrix& transform,
                     std::optional<GlyphProperties> properties);

engine/src/flutter/impeller/typographer/text_frame.cc

@@ -37,50 +37,69 @@ bool TextFrame::HasColor() const {
   return has_color_;
 }
 
+namespace {
+constexpr uint32_t kDenominator = 200;
+constexpr int32_t kMaximumTextScale = 48;
+constexpr Rational kZero(0, kDenominator);
+}  // namespace
+
 // static
-Scalar TextFrame::RoundScaledFontSize(Scalar scale) {
+Rational TextFrame::RoundScaledFontSize(Scalar scale) {
+  if (scale > kMaximumTextScale) {
+    return Rational(kMaximumTextScale * kDenominator, kDenominator);
+  }
   // An arbitrarily chosen maximum text scale to ensure that regardless of the
   // CTM, a glyph will fit in the atlas. If we clamp significantly, this may
   // reduce fidelity but is preferable to the alternative of failing to render.
-  constexpr Scalar kMaximumTextScale = 48;
+  Rational result = Rational(std::round(scale * kDenominator), kDenominator);
-  Scalar result = std::round(scale * 200) / 200;
+  return result < kZero ? kZero : result;
-  return std::clamp(result, 0.0f, kMaximumTextScale);
 }
 
-static constexpr Scalar ComputeFractionalPosition(Scalar value) {
+Rational TextFrame::RoundScaledFontSize(Rational scale) {
+  Rational result = Rational(
+      std::round((scale.GetNumerator() * static_cast<Scalar>(kDenominator))) /
+          scale.GetDenominator(),
+      kDenominator);
+  return std::clamp(result, Rational(0, kDenominator),
+                    Rational(kMaximumTextScale * kDenominator, kDenominator));
+}
+
+static constexpr SubpixelPosition ComputeFractionalPosition(Scalar value) {
   value += 0.125;
   value = (value - floorf(value));
   if (value < 0.25) {
-    return 0;
+    return SubpixelPosition::kSubpixel00;
   }
   if (value < 0.5) {
-    return 0.25;
+    return SubpixelPosition::kSubpixel10;
   }
   if (value < 0.75) {
-    return 0.5;
+    return SubpixelPosition::kSubpixel20;
   }
-  return 0.75;
+  return SubpixelPosition::kSubpixel30;
 }
 
 // Compute subpixel position for glyphs based on X position and provided
 // max basis length (scale).
 // This logic is based on the SkPackedGlyphID logic in SkGlyph.h
 // static
-Point TextFrame::ComputeSubpixelPosition(
+SubpixelPosition TextFrame::ComputeSubpixelPosition(
     const TextRun::GlyphPosition& glyph_position,
     AxisAlignment alignment,
     const Matrix& transform) {
   Point pos = transform * glyph_position.position;
   switch (alignment) {
     case AxisAlignment::kNone:
-      return Point(0, 0);
+      return SubpixelPosition::kSubpixel00;
     case AxisAlignment::kX:
-      return Point(ComputeFractionalPosition(pos.x), 0);
+      return ComputeFractionalPosition(pos.x);
     case AxisAlignment::kY:
-      return Point(0, ComputeFractionalPosition(pos.y));
+      return static_cast<SubpixelPosition>(ComputeFractionalPosition(pos.y)
+                                           << 2);
     case AxisAlignment::kAll:
-      return Point(ComputeFractionalPosition(pos.x),
+      return static_cast<SubpixelPosition>(
-                   ComputeFractionalPosition(pos.y));
+          ComputeFractionalPosition(pos.x) |
+          (ComputeFractionalPosition(pos.y) << 2));
   }
 }
 
@@ -88,7 +107,7 @@ Matrix TextFrame::GetOffsetTransform() const {
   return transform_ * Matrix::MakeTranslation(offset_);
 }
 
-void TextFrame::SetPerFrameData(Scalar scale,
+void TextFrame::SetPerFrameData(Rational scale,
                                 Point offset,
                                 const Matrix& transform,
                                 std::optional<GlyphProperties> properties) {
@@ -99,7 +118,7 @@ void TextFrame::SetPerFrameData(Scalar scale,
   transform_ = transform;
 }
 
-Scalar TextFrame::GetScale() const {
+Rational TextFrame::GetScale() const {
   return scale_;
 }
 

engine/src/flutter/impeller/typographer/text_frame.h

@@ -6,6 +6,7 @@
 #define FLUTTER_IMPELLER_TYPOGRAPHER_TEXT_FRAME_H_
 
 #include <cstdint>
+#include "impeller/geometry/rational.h"
 #include "impeller/typographer/glyph_atlas.h"
 #include "impeller/typographer/text_run.h"
 
@@ -27,12 +28,13 @@ class TextFrame {
 
   ~TextFrame();
 
-  static Point ComputeSubpixelPosition(
+  static SubpixelPosition ComputeSubpixelPosition(
       const TextRun::GlyphPosition& glyph_position,
       AxisAlignment alignment,
       const Matrix& transform);
 
-  static Scalar RoundScaledFontSize(Scalar scale);
+  static Rational RoundScaledFontSize(Scalar scale);
+  static Rational RoundScaledFontSize(Rational scale);
 
   //----------------------------------------------------------------------------
   /// @brief      The conservative bounding box for this text frame.
@@ -80,7 +82,7 @@ class TextFrame {
 
   /// @brief Store text frame scale, offset, and properties for hashing in th
   /// glyph atlas.
-  void SetPerFrameData(Scalar scale,
+  void SetPerFrameData(Rational scale,
                        Point offset,
                        const Matrix& transform,
                        std::optional<GlyphProperties> properties);
@@ -91,7 +93,7 @@ class TextFrame {
   // processed.
   std::pair<size_t, intptr_t> GetAtlasGenerationAndID() const;
 
-  Scalar GetScale() const;
+  Rational GetScale() const;
 
   TextFrame& operator=(TextFrame&& other) = default;
 
@@ -122,7 +124,7 @@ class TextFrame {
   // Data that is cached when rendering the text frame and is only
   // valid for the current atlas generation.
   std::vector<FrameBounds> bound_values_;
-  Scalar scale_ = 0;
+  Rational scale_ = Rational(0, 1);
   size_t generation_ = 0;
   intptr_t atlas_id_ = 0;
   Point offset_;

engine/src/flutter/impeller/typographer/typographer_unittests.cc

@@ -34,7 +34,7 @@ static std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
     const TypographerContext* typographer_context,
     HostBuffer& host_buffer,
     GlyphAtlas::Type type,
-    Scalar scale,
+    Rational scale,
     const std::shared_ptr<GlyphAtlasContext>& atlas_context,
     const std::shared_ptr<TextFrame>& frame) {
   frame->SetPerFrameData(scale, {0, 0}, Matrix(), std::nullopt);
@@ -47,7 +47,7 @@ static std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
     const TypographerContext* typographer_context,
     HostBuffer& host_buffer,
     GlyphAtlas::Type type,
-    Scalar scale,
+    Rational scale,
     const std::shared_ptr<GlyphAtlasContext>& atlas_context,
     const std::vector<std::shared_ptr<TextFrame>>& frames,
     const std::vector<std::optional<GlyphProperties>>& properties) {
@@ -89,8 +89,8 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) {
   ASSERT_TRUE(blob);
   auto atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kAlphaBitmap, Rational(1),
-                       MakeTextFrameFromTextBlobSkia(blob));
+                       atlas_context, MakeTextFrameFromTextBlobSkia(blob));
 
   ASSERT_NE(atlas, nullptr);
   ASSERT_NE(atlas->GetTexture(), nullptr);
@@ -137,14 +137,14 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) {
 
   LazyGlyphAtlas lazy_atlas(TypographerContextSkia::Make());
 
-  lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, Matrix(), {});
+  lazy_atlas.AddTextFrame(frame, Rational(1), {0, 0}, Matrix(), {});
 
   frame = MakeTextFrameFromTextBlobSkia(
       SkTextBlob::MakeFromString("😀 ", emoji_font));
 
   ASSERT_TRUE(frame->GetAtlasType() == GlyphAtlas::Type::kColorBitmap);
 
-  lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, Matrix(), {});
+  lazy_atlas.AddTextFrame(frame, Rational(1), {0, 0}, Matrix(), {});
 
   // Creates different atlases for color and red bitmap.
   auto color_atlas = lazy_atlas.CreateOrGetGlyphAtlas(
@@ -168,8 +168,8 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) {
   ASSERT_TRUE(blob);
   auto atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kAlphaBitmap, Rational(1),
-                       MakeTextFrameFromTextBlobSkia(blob));
+                       atlas_context, MakeTextFrameFromTextBlobSkia(blob));
   ASSERT_NE(atlas, nullptr);
   ASSERT_NE(atlas->GetTexture(), nullptr);
 
@@ -189,8 +189,8 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) {
   ASSERT_TRUE(blob);
   auto atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kAlphaBitmap, Rational(1),
-                       MakeTextFrameFromTextBlobSkia(blob));
+                       atlas_context, MakeTextFrameFromTextBlobSkia(blob));
   ASSERT_NE(atlas, nullptr);
   ASSERT_NE(atlas->GetTexture(), nullptr);
   ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas());
@@ -199,8 +199,8 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) {
 
   auto next_atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kAlphaBitmap, Rational(1),
-                       MakeTextFrameFromTextBlobSkia(blob));
+                       atlas_context, MakeTextFrameFromTextBlobSkia(blob));
   ASSERT_EQ(atlas, next_atlas);
   ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas);
 }
@@ -227,7 +227,8 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) {
   std::vector<std::shared_ptr<TextFrame>> frames;
   for (size_t index = 0; index < size_count; index += 1) {
     frames.push_back(MakeTextFrameFromTextBlobSkia(blob));
-    frames.back()->SetPerFrameData(0.6 * index, {0, 0}, Matrix(), {});
+    frames.back()->SetPerFrameData(Rational(6 * index, 10), {0, 0}, Matrix(),
+                                   {});
   };
   auto atlas =
       context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kAlphaBitmap,
@@ -265,8 +266,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) {
   ASSERT_TRUE(blob);
   auto atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kAlphaBitmap, Rational(1),
-                       MakeTextFrameFromTextBlobSkia(blob));
+                       atlas_context, MakeTextFrameFromTextBlobSkia(blob));
   auto old_packer = atlas_context->GetRectPacker();
 
   ASSERT_NE(atlas, nullptr);
@@ -280,8 +281,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) {
   auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font);
   auto next_atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kAlphaBitmap, Rational(1),
-                       MakeTextFrameFromTextBlobSkia(blob2));
+                       atlas_context, MakeTextFrameFromTextBlobSkia(blob2));
   ASSERT_EQ(atlas, next_atlas);
   auto* second_texture = next_atlas->GetTexture().get();
 
@@ -320,8 +321,8 @@ TEST_P(TypographerTest, GlyphColorIsPartOfCacheKey) {
 
   auto next_atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kColorBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kColorBitmap, Rational(1),
-                       {frame, frame_2}, properties);
+                       atlas_context, {frame, frame_2}, properties);
 
   EXPECT_EQ(next_atlas->GetGlyphCount(), 2u);
 }
@@ -351,8 +352,8 @@ TEST_P(TypographerTest, GlyphColorIsIgnoredForNonEmojiFonts) {
 
   auto next_atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kColorBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kColorBitmap, Rational(1),
-                       {frame, frame_2}, properties);
+                       atlas_context, {frame, frame_2}, properties);
 
   EXPECT_EQ(next_atlas->GetGlyphCount(), 1u);
 }
@@ -438,8 +439,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) {
   ASSERT_TRUE(blob);
   auto atlas =
       CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                       GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context,
+                       GlyphAtlas::Type::kAlphaBitmap, Rational(1),
-                       MakeTextFrameFromTextBlobSkia(blob));
+                       atlas_context, MakeTextFrameFromTextBlobSkia(blob));
   // Continually append new glyphs until the glyph size grows to the maximum.
   // Note that the sizes here are more or less experimentally determined, but
   // the important expectation is that the atlas size will shrink again after
@@ -479,8 +480,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) {
 
     atlas =
         CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                         GlyphAtlas::Type::kAlphaBitmap, 50 + i, atlas_context,
+                         GlyphAtlas::Type::kAlphaBitmap, Rational(50 + i, 1),
-                         MakeTextFrameFromTextBlobSkia(blob));
+                         atlas_context, MakeTextFrameFromTextBlobSkia(blob));
     ASSERT_TRUE(!!atlas);
     EXPECT_EQ(atlas->GetTexture()->GetTextureDescriptor().size,
               expected_sizes[i]);
@@ -508,8 +509,8 @@ TEST_P(TypographerTest, TextFrameInitialBoundsArePlaceholder) {
                                         GetContext()->GetIdleWaiter());
 
   auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                                GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                                GlyphAtlas::Type::kAlphaBitmap,
-                                atlas_context, frame);
+                                /*scale=*/Rational(1), atlas_context, frame);
 
   // The glyph position in the atlas was not known when this value
   // was recorded. It is marked as a placeholder.
@@ -517,8 +518,8 @@ TEST_P(TypographerTest, TextFrameInitialBoundsArePlaceholder) {
   EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder);
 
   atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                           GlyphAtlas::Type::kAlphaBitmap,
-                           atlas_context, frame);
+                           /*scale=*/Rational(1), atlas_context, frame);
 
   // The second time the glyph is rendered, the bounds are correcly known.
   EXPECT_TRUE(frame->IsFrameComplete());
@@ -541,8 +542,8 @@ TEST_P(TypographerTest, TextFrameInvalidationWithScale) {
                                         GetContext()->GetIdleWaiter());
 
   auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                                GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                                GlyphAtlas::Type::kAlphaBitmap,
-                                atlas_context, frame);
+                                /*scale=*/Rational(1), atlas_context, frame);
 
   // The glyph position in the atlas was not known when this value
   // was recorded. It is marked as a placeholder.
@@ -552,8 +553,8 @@ TEST_P(TypographerTest, TextFrameInvalidationWithScale) {
   // Change the scale and the glyph data will still be a placeholder, as the
   // old data is no longer valid.
   atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/2.0f,
+                           GlyphAtlas::Type::kAlphaBitmap,
-                           atlas_context, frame);
+                           /*scale=*/Rational(2), atlas_context, frame);
 
   // The second time the glyph is rendered, the bounds are correcly known.
   EXPECT_TRUE(frame->IsFrameComplete());
@@ -576,8 +577,8 @@ TEST_P(TypographerTest, TextFrameAtlasGenerationTracksState) {
                                         GetContext()->GetIdleWaiter());
 
   auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                                GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                                GlyphAtlas::Type::kAlphaBitmap,
-                                atlas_context, frame);
+                                /*scale=*/Rational(1), atlas_context, frame);
 
   // The glyph position in the atlas was not known when this value
   // was recorded. It is marked as a placeholder.
@@ -591,8 +592,8 @@ TEST_P(TypographerTest, TextFrameAtlasGenerationTracksState) {
   }
 
   atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                           GlyphAtlas::Type::kAlphaBitmap,
-                           atlas_context, frame);
+                           /*scale=*/Rational(1), atlas_context, frame);
 
   // The second time the glyph is rendered, the bounds are correcly known.
   EXPECT_TRUE(frame->IsFrameComplete());
@@ -606,8 +607,8 @@ TEST_P(TypographerTest, TextFrameAtlasGenerationTracksState) {
   // Force increase the generation.
   atlas_context->GetGlyphAtlas()->SetAtlasGeneration(2u);
   atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                           GlyphAtlas::Type::kAlphaBitmap,
-                           atlas_context, frame);
+                           /*scale=*/Rational(1), atlas_context, frame);
 
   EXPECT_EQ(frame->GetAtlasGenerationAndID().first, 2u);
 }
@@ -628,8 +629,8 @@ TEST_P(TypographerTest, InvalidAtlasForcesRepopulation) {
                                         GetContext()->GetIdleWaiter());
 
   auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
-                                GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                                GlyphAtlas::Type::kAlphaBitmap,
-                                atlas_context, frame);
+                                /*scale=*/Rational(1), atlas_context, frame);
 
   // The glyph position in the atlas was not known when this value
   // was recorded. It is marked as a placeholder.
@@ -649,8 +650,8 @@ TEST_P(TypographerTest, InvalidAtlasForcesRepopulation) {
   EXPECT_FALSE(second_atlas_context->GetGlyphAtlas()->IsValid());
 
   atlas = CreateGlyphAtlas(*GetContext(), second_context.get(), *host_buffer,
-                           GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f,
+                           GlyphAtlas::Type::kAlphaBitmap,
-                           second_atlas_context, frame);
+                           /*scale=*/Rational(1), second_atlas_context, frame);
 
   EXPECT_TRUE(second_atlas_context->GetGlyphAtlas()->IsValid());
 }