Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] round scale for glyph atlas cache to 2 decimal places. #43752

Merged
auto-submit merged 10 commits into from
Jul 25, 2023
Merged

[Impeller] round scale for glyph atlas cache to 2 decimal places. #43752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
acaa12f
[Impeller] round scale for glyph atlas cache to 1 decimal place.
Jul 18, 2023
db80bc4
++
Jul 18, 2023
15eba97
++
Jul 21, 2023
39bb139
++
Jul 21, 2023
046be21
++
Jul 21, 2023
0ca9505
Merge branch 'main' of github.com:flutter/engine into round_text
Jul 24, 2023
8cc8bc9
Merge branch 'main' of github.com:flutter/engine into round_text
Jul 25, 2023
8670e5a
++
Jul 25, 2023
2545fb5
++
Jul 25, 2023
d4f4231
add some debugging help text
Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion impeller/entity/contents/text_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,12 @@ bool TextContents::Render(const ContentContext& renderer,
size_t vertex_offset = 0;
for (const auto& run : frame_.GetRuns()) {
const Font& font = run.GetFont();
auto rounded_scale = TextFrame::RoundScaledFontSize(
scale_, font.GetMetrics().point_size);

for (const auto& glyph_position : run.GetGlyphPositions()) {
FontGlyphPair font_glyph_pair{font, glyph_position.glyph, scale_};
FontGlyphPair font_glyph_pair{font, glyph_position.glyph,
rounded_scale};
auto maybe_atlas_glyph_bounds =
atlas->FindFontGlyphBounds(font_glyph_pair);
if (!maybe_atlas_glyph_bounds.has_value()) {
Expand Down
7 changes: 7 additions & 0 deletions impeller/entity/entity_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2426,6 +2426,13 @@ TEST_P(EntityTest, ColorFilterContentsWithLargeGeometry) {
ASSERT_TRUE(OpenPlaygroundHere(entity));
}

TEST_P(EntityTest, TextContentsCeilsGlyphScaleToDecimal) {
ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f, 12), 0.43f);
ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f, 12), 0.53f);
ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f, 12), 2.1f);
ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f, 12), 0.0f);
}

} // namespace testing
} // namespace impeller

Expand Down
16 changes: 15 additions & 1 deletion impeller/typographer/text_frame.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,27 @@ bool TextFrame::MaybeHasOverlapping() const {
return false;
}

// static
Scalar TextFrame::RoundScaledFontSize(Scalar scale, Scalar point_size) {
return std::round(scale * 100) / 100;
}
Copy link
Member

@bdero bdero Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this covers all cases, would something like this work?

// The point_size is multiplied into the total scale of the font later,
// and so we need to linearly increase the step precision as the point size increases.
auto precision = 20 * point_size; // The constant here may need to be adjusted for best perf.
return std::pow(kE, std::round(std::log(scale) * precision) / precision);

image

https://www.desmos.com/calculator/sqpcws9bfv

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This curve is going to introduce a lot of churn for values that are already fairly stable.

For the default text scale of 3.5x (the DPR) we can end up converting this to 3.49908 or 3.50127 based on the exact text scale. I don't we're going to be able to use an log based rounding.


void TextFrame::CollectUniqueFontGlyphPairs(FontGlyphPair::Set& set,
Scalar scale) const {
for (const TextRun& run : GetRuns()) {
const Font& font = run.GetFont();
auto rounded_scale =
RoundScaledFontSize(scale, font.GetMetrics().point_size);
for (const TextRun::GlyphPosition& glyph_position :
run.GetGlyphPositions()) {
set.insert({font, glyph_position.glyph, scale});
#if false
// Glyph size error due to RoundScaledFontSize usage above.
if (rounded_scale != scale) {
auto delta = std::abs(rounded_scale - scale);
FML_LOG(ERROR) << glyph_position.glyph.bounds.size * delta;
}
#endif
set.insert({font, glyph_position.glyph, rounded_scale});
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions impeller/typographer/text_frame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class TextFrame {

void CollectUniqueFontGlyphPairs(FontGlyphPair::Set& set, Scalar scale) const;

static Scalar RoundScaledFontSize(Scalar scale, Scalar point_size);

//----------------------------------------------------------------------------
/// @brief The conservative bounding box for this text frame.
///
Expand Down