Skip to content

Commit

Permalink
[Impeller] Reorganize the glyph atlas to improve efficiency when look…
Browse files Browse the repository at this point in the history 
…ing up runs of glyphs in the same font

The atlas will now store a separate map of glyph positions for each
font/scale pair.  This avoids the cost of repeated hashing and copying
of font objects for each glyph in a text run.

See flutter/flutter#133201
  • Loading branch information
@jason-simmons
jason-simmons committed Aug 29, 2023
1 parent 57162a5 commit 19a3660
Show file tree
Hide file tree
Showing 15 changed files with 266 additions and 166 deletions.
10 changes: 7 additions & 3 deletions impeller/entity/contents/text_contents.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,17 @@ bool TextContents::Render(const ContentContext& renderer,
const Font& font = run.GetFont();
Scalar rounded_scale = TextFrame::RoundScaledFontSize(
scale_, font.GetMetrics().point_size);
const FontGlyphAtlas* font_atlas =
atlas->GetFontGlyphAtlas(font, rounded_scale);
if (!font_atlas) {
VALIDATION_LOG << "Could not find font in the atlas.";
continue;
}

for (const TextRun::GlyphPosition& glyph_position :
run.GetGlyphPositions()) {
FontGlyphPair font_glyph_pair{font, glyph_position.glyph,
rounded_scale};
std::optional<Rect> maybe_atlas_glyph_bounds =
atlas->FindFontGlyphBounds(font_glyph_pair);
font_atlas->FindGlyphBounds(glyph_position.glyph);
if (!maybe_atlas_glyph_bounds.has_value()) {
VALIDATION_LOG << "Could not find glyph position in the atlas.";
continue;
Expand Down
88 changes: 55 additions & 33 deletions impeller/typographer/backends/skia/typographer_context_skia.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "impeller/typographer/backends/skia/typographer_context_skia.h"

#include <numeric>
#include <utility>

#include "flutter/fml/logging.h"
Expand All @@ -21,9 +22,6 @@

namespace impeller {

using FontGlyphPairRefVector =
std::vector<std::reference_wrapper<const FontGlyphPair>>;

// TODO(bdero): We might be able to remove this per-glyph padding if we fix
// the underlying causes of the overlap.
// https://github.com/flutter/flutter/issues/114563
Expand All @@ -43,7 +41,7 @@ TypographerContextSkia::CreateGlyphAtlasContext() const {
}

static size_t PairsFitInAtlasOfSize(
const FontGlyphPair::Set& pairs,
const std::vector<FontGlyphPair>& pairs,
const ISize& atlas_size,
std::vector<Rect>& glyph_positions,
const std::shared_ptr<RectanglePacker>& rect_packer) {
Expand All @@ -58,7 +56,8 @@ static size_t PairsFitInAtlasOfSize(
for (auto it = pairs.begin(); it != pairs.end(); ++i, ++it) {
const auto& pair = *it;

const auto glyph_size = ISize::Ceil(pair.glyph.bounds.size * pair.scale);
const auto glyph_size =
ISize::Ceil(pair.glyph.bounds.size * pair.scaled_font.scale);
IPoint16 location_in_atlas;
if (!rect_packer->addRect(glyph_size.width + kPadding, //
glyph_size.height + kPadding, //
Expand All @@ -78,7 +77,7 @@ static size_t PairsFitInAtlasOfSize(

static bool CanAppendToExistingAtlas(
const std::shared_ptr<GlyphAtlas>& atlas,
const FontGlyphPairRefVector& extra_pairs,
const std::vector<FontGlyphPair>& extra_pairs,
std::vector<Rect>& glyph_positions,
ISize atlas_size,
const std::shared_ptr<RectanglePacker>& rect_packer) {
Expand All @@ -95,7 +94,8 @@ static bool CanAppendToExistingAtlas(
for (size_t i = 0; i < extra_pairs.size(); i++) {
const FontGlyphPair& pair = extra_pairs[i];

const auto glyph_size = ISize::Ceil(pair.glyph.bounds.size * pair.scale);
const auto glyph_size =
ISize::Ceil(pair.glyph.bounds.size * pair.scaled_font.scale);
IPoint16 location_in_atlas;
if (!rect_packer->addRect(glyph_size.width + kPadding, //
glyph_size.height + kPadding, //
Expand All @@ -114,7 +114,7 @@ static bool CanAppendToExistingAtlas(
}

static ISize OptimumAtlasSizeForFontGlyphPairs(
const FontGlyphPair::Set& pairs,
const std::vector<FontGlyphPair>& pairs,
std::vector<Rect>& glyph_positions,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
GlyphAtlas::Type type) {
Expand Down Expand Up @@ -153,16 +153,17 @@ static ISize OptimumAtlasSizeForFontGlyphPairs(
}

static void DrawGlyph(SkCanvas* canvas,
const FontGlyphPair& font_glyph,
const ScaledFont& scaled_font,
const Glyph& glyph,
const Rect& location,
bool has_color) {
const auto& metrics = font_glyph.font.GetMetrics();
const auto position = SkPoint::Make(location.origin.x / font_glyph.scale,
location.origin.y / font_glyph.scale);
SkGlyphID glyph_id = font_glyph.glyph.index;
const auto& metrics = scaled_font.font.GetMetrics();
const auto position = SkPoint::Make(location.origin.x / scaled_font.scale,
location.origin.y / scaled_font.scale);
SkGlyphID glyph_id = glyph.index;

SkFont sk_font(
TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(),
TypefaceSkia::Cast(*scaled_font.font.GetTypeface()).GetSkiaTypeface(),
metrics.point_size, metrics.scaleX, metrics.skewX);
sk_font.setEdging(SkFont::Edging::kAntiAlias);
sk_font.setHinting(SkFontHinting::kSlight);
Expand All @@ -173,21 +174,20 @@ static void DrawGlyph(SkCanvas* canvas,
SkPaint glyph_paint;
glyph_paint.setColor(glyph_color);
canvas->resetMatrix();
canvas->scale(font_glyph.scale, font_glyph.scale);
canvas->drawGlyphs(
1u, // count
&glyph_id, // glyphs
&position, // positions
SkPoint::Make(-font_glyph.glyph.bounds.GetLeft(),
-font_glyph.glyph.bounds.GetTop()), // origin
sk_font, // font
glyph_paint // paint
canvas->scale(scaled_font.scale, scaled_font.scale);
canvas->drawGlyphs(1u, // count
&glyph_id, // glyphs
&position, // positions
SkPoint::Make(-glyph.bounds.GetLeft(),
-glyph.bounds.GetTop()), // origin
sk_font, // font
glyph_paint // paint
);
}

static bool UpdateAtlasBitmap(const GlyphAtlas& atlas,
const std::shared_ptr<SkBitmap>& bitmap,
const FontGlyphPairRefVector& new_pairs) {
const std::vector<FontGlyphPair>& new_pairs) {
TRACE_EVENT0("impeller", __FUNCTION__);
FML_DCHECK(bitmap != nullptr);

Expand All @@ -207,7 +207,7 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas,
if (!pos.has_value()) {
continue;
}
DrawGlyph(canvas, pair, pos.value(), has_color);
DrawGlyph(canvas, pair.scaled_font, pair.glyph, pos.value(), has_color);
}
return true;
}
Expand Down Expand Up @@ -243,9 +243,10 @@ static std::shared_ptr<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,

bool has_color = atlas.GetType() == GlyphAtlas::Type::kColorBitmap;

atlas.IterateGlyphs([canvas, has_color](const FontGlyphPair& font_glyph,
atlas.IterateGlyphs([canvas, has_color](const ScaledFont& scaled_font,
const Glyph& glyph,
const Rect& location) -> bool {
DrawGlyph(canvas, font_glyph, location, has_color);
DrawGlyph(canvas, scaled_font, glyph, location, has_color);
return true;
});

Expand Down Expand Up @@ -313,26 +314,37 @@ std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
Context& context,
GlyphAtlas::Type type,
std::shared_ptr<GlyphAtlasContext> atlas_context,
const FontGlyphPair::Set& font_glyph_pairs) const {
const FontGlyphMap& font_glyph_map) const {
TRACE_EVENT0("impeller", __FUNCTION__);
if (!IsValid()) {
return nullptr;
}
auto& atlas_context_skia = GlyphAtlasContextSkia::Cast(*atlas_context);
std::shared_ptr<GlyphAtlas> last_atlas = atlas_context->GetGlyphAtlas();

if (font_glyph_pairs.empty()) {
if (font_glyph_map.empty()) {
return last_atlas;
}

// ---------------------------------------------------------------------------
// Step 1: Determine if the atlas type and font glyph pairs are compatible
// with the current atlas and reuse if possible.
// ---------------------------------------------------------------------------
FontGlyphPairRefVector new_glyphs;
for (const FontGlyphPair& pair : font_glyph_pairs) {
if (!last_atlas->FindFontGlyphBounds(pair).has_value()) {
new_glyphs.push_back(pair);
std::vector<FontGlyphPair> new_glyphs;
for (const auto& font_value : font_glyph_map) {
const ScaledFont& scaled_font = font_value.first;
const FontGlyphAtlas* font_glyph_atlas =
last_atlas->GetFontGlyphAtlas(scaled_font.font, scaled_font.scale);
if (font_glyph_atlas) {
for (const Glyph& glyph : font_value.second) {
if (!font_glyph_atlas->FindGlyphBounds(glyph)) {
new_glyphs.emplace_back(scaled_font, glyph);
}
}
} else {
for (const Glyph& glyph : font_value.second) {
new_glyphs.emplace_back(scaled_font, glyph);
}
}
}
if (last_atlas->GetType() == type && new_glyphs.size() == 0) {
Expand Down Expand Up @@ -381,6 +393,16 @@ std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
// ---------------------------------------------------------------------------
// Step 3b: Get the optimum size of the texture atlas.
// ---------------------------------------------------------------------------
std::vector<FontGlyphPair> font_glyph_pairs;
font_glyph_pairs.reserve(std::accumulate(
font_glyph_map.begin(), font_glyph_map.end(), 0,
[](const int a, const auto& b) { return a + b.second.size(); }));
for (const auto& font_value : font_glyph_map) {
const ScaledFont& scaled_font = font_value.first;
for (const Glyph& glyph : font_value.second) {
font_glyph_pairs.push_back({scaled_font, glyph});
}
}
auto glyph_atlas = std::make_shared<GlyphAtlas>(type);
auto atlas_size = OptimumAtlasSizeForFontGlyphPairs(
font_glyph_pairs, glyph_positions, atlas_context, type);
Expand Down
2 changes: 1 addition & 1 deletion impeller/typographer/backends/skia/typographer_context_skia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class TypographerContextSkia : public TypographerContext {
Context& context,
GlyphAtlas::Type type,
std::shared_ptr<GlyphAtlasContext> atlas_context,
const FontGlyphPair::Set& font_glyph_pairs) const override;
const FontGlyphMap& font_glyph_map) const override;

private:
FML_DISALLOW_COPY_AND_ASSIGN(TypographerContextSkia);
Expand Down
Loading

0 comments on commit 19a3660

Please sign in to comment.