diff --git a/engine/src/flutter/ci/licenses_golden/excluded_files b/engine/src/flutter/ci/licenses_golden/excluded_files index 3a2ce74781..8fe3366da2 100644 --- a/engine/src/flutter/ci/licenses_golden/excluded_files +++ b/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 diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 341ba9d818..de4b1fc032 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/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 diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.cc b/engine/src/flutter/impeller/entity/contents/text_contents.cc index 3ca08c4fe2..a76f87a55b 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.cc +++ b/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 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(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 diff --git a/engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc b/engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc index d1eec4fc2b..54503f135f 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc +++ b/engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc @@ -54,13 +54,15 @@ std::shared_ptr CreateGlyphAtlas( const TypographerContext* typographer_context, HostBuffer& host_buffer, GlyphAtlas::Type type, - Scalar scale, + Rational scale, const std::shared_ptr& atlas_context, const std::shared_ptr& frame, Point offset) { frame->SetPerFrameData( TextFrame::RoundScaledFontSize(scale), /*offset=*/offset, - /*transform=*/Matrix::MakeScale(Vector3{scale, scale, 1}), + /*transform=*/ + Matrix::MakeScale( + Vector3{static_cast(scale), static_cast(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 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 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 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, - /*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}), + data, text_frame, static_cast(font_scale), + /*entity_transform=*/ + Matrix::MakeScale({static_cast(font_scale), + static_cast(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, - /*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}), + data, text_frame, static_cast(font_scale), + /*entity_transform=*/ + Matrix::MakeScale({static_cast(font_scale), + static_cast(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 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 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 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(font_scale), /*entity_transform=*/ Matrix::MakeTranslation(offset) * - Matrix::MakeScale({font_scale, font_scale, 1}), + Matrix::MakeScale({static_cast(font_scale), + static_cast(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 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 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(); diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index afd8e28fd2..66b2270809 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/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.5321111f), 0.53f); - ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f), 2.1f); - ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f), 0.0f); - ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f), 48.0f); + ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f), Rational(43, 100)); + ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f), Rational(53, 100)); + ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f), Rational(21, 10)); + ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f), Rational(0, 1)); + ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f), Rational(48, 1)); } TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) { diff --git a/engine/src/flutter/impeller/geometry/BUILD.gn b/engine/src/flutter/impeller/geometry/BUILD.gn index 4ecde22748..dde58e907e 100644 --- a/engine/src/flutter/impeller/geometry/BUILD.gn +++ b/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", diff --git a/engine/src/flutter/impeller/geometry/rational.cc b/engine/src/flutter/impeller/geometry/rational.cc new file mode 100644 index 0000000000..57dc3705f3 --- /dev/null +++ b/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 +#include +#include + +namespace impeller { +namespace { +uint32_t AbsToUnsigned(int32_t x) { + return static_cast(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(den_), std::abs(num_)); + } +} + +} // namespace impeller diff --git a/engine/src/flutter/impeller/geometry/rational.h b/engine/src/flutter/impeller/geometry/rational.h new file mode 100644 index 0000000000..84f2914c32 --- /dev/null +++ b/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 +#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(num_) / den_; } + + Rational Invert() const; + + private: + int32_t num_; + uint32_t den_; +}; + +} // namespace impeller + +#endif // FLUTTER_IMPELLER_GEOMETRY_RATIONAL_H_ diff --git a/engine/src/flutter/impeller/geometry/rational_unittests.cc b/engine/src/flutter/impeller/geometry/rational_unittests.cc new file mode 100644 index 0000000000..f912965a57 --- /dev/null +++ b/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 diff --git a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc index 19374cac0c..a9d57b3fdf 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/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(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(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(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 = - ComputeGlyphSize(sk_font, subpixel_glyph, scaled_font.scale); + auto glyph_bounds = ComputeGlyphSize( + sk_font, subpixel_glyph, static_cast(scaled_font.scale)); glyph_sizes.push_back(glyph_bounds); auto frame_bounds = FrameBounds{ diff --git a/engine/src/flutter/impeller/typographer/font_glyph_pair.h b/engine/src/flutter/impeller/typographer/font_glyph_pair.h index d5b8906de8..2905c678ac 100644 --- a/engine/src/flutter/impeller/typographer/font_glyph_pair.h +++ b/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 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 properties; SubpixelGlyph(Glyph p_glyph, - Point p_subpixel_offset, + SubpixelPosition p_subpixel_offset, std::optional p_properties) : glyph(p_glyph), subpixel_offset(p_subpixel_offset), @@ -72,14 +102,12 @@ struct SubpixelGlyph { template 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, - sg.subpixel_offset.y); + return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset); } - return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset.x, - sg.subpixel_offset.y, sg.properties->color.ToARGB(), - sg.properties->stroke, sg.properties->stroke_cap, - sg.properties->stroke_join, sg.properties->stroke_miter, - sg.properties->stroke_width); + return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset, + sg.properties->color.ToARGB(), sg.properties->stroke, + sg.properties->stroke_cap, sg.properties->stroke_join, + sg.properties->stroke_miter, sg.properties->stroke_width); } struct Equal { diff --git a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc index 4a484109f4..6575db7a75 100644 --- a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc +++ b/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& frame, - Scalar scale, + Rational scale, Point offset, const Matrix& transform, std::optional properties) { diff --git a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h index 17b8767e27..7c41fbfbab 100644 --- a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h +++ b/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& frame, - Scalar scale, + Rational scale, Point offset, const Matrix& transform, std::optional properties); diff --git a/engine/src/flutter/impeller/typographer/text_frame.cc b/engine/src/flutter/impeller/typographer/text_frame.cc index a679e2c220..cdfd48dca9 100644 --- a/engine/src/flutter/impeller/typographer/text_frame.cc +++ b/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; - Scalar result = std::round(scale * 200) / 200; - return std::clamp(result, 0.0f, kMaximumTextScale); + Rational result = Rational(std::round(scale * kDenominator), kDenominator); + return result < kZero ? kZero : result; } -static constexpr Scalar ComputeFractionalPosition(Scalar value) { +Rational TextFrame::RoundScaledFontSize(Rational scale) { + Rational result = Rational( + std::round((scale.GetNumerator() * static_cast(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(ComputeFractionalPosition(pos.y) + << 2); case AxisAlignment::kAll: - return Point(ComputeFractionalPosition(pos.x), - ComputeFractionalPosition(pos.y)); + return static_cast( + 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 properties) { @@ -99,7 +118,7 @@ void TextFrame::SetPerFrameData(Scalar scale, transform_ = transform; } -Scalar TextFrame::GetScale() const { +Rational TextFrame::GetScale() const { return scale_; } diff --git a/engine/src/flutter/impeller/typographer/text_frame.h b/engine/src/flutter/impeller/typographer/text_frame.h index 0a000109f1..9c84ceb447 100644 --- a/engine/src/flutter/impeller/typographer/text_frame.h +++ b/engine/src/flutter/impeller/typographer/text_frame.h @@ -6,6 +6,7 @@ #define FLUTTER_IMPELLER_TYPOGRAPHER_TEXT_FRAME_H_ #include +#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 properties); @@ -91,7 +93,7 @@ class TextFrame { // processed. std::pair 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 bound_values_; - Scalar scale_ = 0; + Rational scale_ = Rational(0, 1); size_t generation_ = 0; intptr_t atlas_id_ = 0; Point offset_; diff --git a/engine/src/flutter/impeller/typographer/typographer_unittests.cc b/engine/src/flutter/impeller/typographer/typographer_unittests.cc index d12f55e01e..82c8c1aa9b 100644 --- a/engine/src/flutter/impeller/typographer/typographer_unittests.cc +++ b/engine/src/flutter/impeller/typographer/typographer_unittests.cc @@ -34,7 +34,7 @@ static std::shared_ptr CreateGlyphAtlas( const TypographerContext* typographer_context, HostBuffer& host_buffer, GlyphAtlas::Type type, - Scalar scale, + Rational scale, const std::shared_ptr& atlas_context, const std::shared_ptr& frame) { frame->SetPerFrameData(scale, {0, 0}, Matrix(), std::nullopt); @@ -47,7 +47,7 @@ static std::shared_ptr CreateGlyphAtlas( const TypographerContext* typographer_context, HostBuffer& host_buffer, GlyphAtlas::Type type, - Scalar scale, + Rational scale, const std::shared_ptr& atlas_context, const std::vector>& frames, const std::vector>& 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, - MakeTextFrameFromTextBlobSkia(blob)); + GlyphAtlas::Type::kAlphaBitmap, Rational(1), + 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, - MakeTextFrameFromTextBlobSkia(blob)); + GlyphAtlas::Type::kAlphaBitmap, Rational(1), + 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, - MakeTextFrameFromTextBlobSkia(blob)); + GlyphAtlas::Type::kAlphaBitmap, Rational(1), + 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, - MakeTextFrameFromTextBlobSkia(blob)); + GlyphAtlas::Type::kAlphaBitmap, Rational(1), + 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> 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, - MakeTextFrameFromTextBlobSkia(blob)); + GlyphAtlas::Type::kAlphaBitmap, Rational(1), + 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, - MakeTextFrameFromTextBlobSkia(blob2)); + GlyphAtlas::Type::kAlphaBitmap, Rational(1), + 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, - {frame, frame_2}, properties); + GlyphAtlas::Type::kColorBitmap, Rational(1), + 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, - {frame, frame_2}, properties); + GlyphAtlas::Type::kColorBitmap, Rational(1), + 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, - MakeTextFrameFromTextBlobSkia(blob)); + GlyphAtlas::Type::kAlphaBitmap, Rational(1), + 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, - MakeTextFrameFromTextBlobSkia(blob)); + GlyphAtlas::Type::kAlphaBitmap, Rational(50 + i, 1), + 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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*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, - second_atlas_context, frame); + GlyphAtlas::Type::kAlphaBitmap, + /*scale=*/Rational(1), second_atlas_context, frame); EXPECT_TRUE(second_atlas_context->GetGlyphAtlas()->IsValid()); }