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

Reverts "[Impeller] hash even less stuff per frame. (#55092)" #55491

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Reverts "[Impeller] hash even less stuff per frame. (#55092)" #55491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 4 additions & 7 deletions impeller/display_list/dl_dispatcher.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1458,13 +1458,10 @@ void TextFrameDispatcher::drawTextFrame(
}
auto scale =
(matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY();
renderer_.GetLazyGlyphAtlas()->AddTextFrame(
text_frame, //
scale, //
Point(x, y), //
(properties.stroke || text_frame->HasColor()) //
? std::optional<GlyphProperties>(properties) //
: std::nullopt //
renderer_.GetLazyGlyphAtlas()->AddTextFrame(*text_frame, //
scale, //
Point(x, y), //
properties //
);
}

Expand Down
71 changes: 21 additions & 50 deletions impeller/entity/contents/text_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@ bool TextContents::Render(const ContentContext& renderer,
VALIDATION_LOG << "Cannot render glyphs without prepared atlas.";
return false;
}
if (!frame_->IsFrameComplete()) {
VALIDATION_LOG << "Failed to find font glyph bounds.";
return false;
}

// Information shared by all glyph draw calls.
pass.SetCommandLabel("TextFrame");
Expand Down Expand Up @@ -173,12 +169,16 @@ bool TextContents::Render(const ContentContext& renderer,
VS::PerVertexData* vtx_contents =
reinterpret_cast<VS::PerVertexData*>(contents);
size_t i = 0u;
size_t bounds_offset = 0u;
for (const TextRun& run : frame_->GetRuns()) {
const Font& font = run.GetFont();
Scalar rounded_scale = TextFrame::RoundScaledFontSize(
scale_, font.GetMetrics().point_size);
FontGlyphAtlas* font_atlas = nullptr;
const FontGlyphAtlas* font_atlas =
atlas->GetFontGlyphAtlas(font, rounded_scale);
if (!font_atlas) {
VALIDATION_LOG << "Could not find font in the atlas.";
continue;
}

// Adjust glyph position based on the subpixel rounding
// used by the font.
Expand All @@ -201,45 +201,22 @@ bool TextContents::Render(const ContentContext& renderer,
Point screen_offset = (entity_transform * Point(0, 0));
for (const TextRun::GlyphPosition& glyph_position :
run.GetGlyphPositions()) {
const FrameBounds& frame_bounds =
frame_->GetFrameBounds(bounds_offset);
bounds_offset++;
auto atlas_glyph_bounds = frame_bounds.atlas_bounds;
auto glyph_bounds = frame_bounds.glyph_bounds;

// If frame_bounds.is_placeholder is true, this is the first frame
// the glyph has been rendered and so its atlas position was not
// known when the glyph was recorded. Perform a slow lookup into the
// glyph atlas hash table.
if (frame_bounds.is_placeholder) {
// Note: uses unrounded scale for more accurate subpixel position.
if (!font_atlas) {
font_atlas = atlas->GetOrCreateFontGlyphAtlas(
ScaledFont{font, rounded_scale});
}

if (!font_atlas) {
VALIDATION_LOG << "Could not find font in the atlas.";
continue;
}
// Note: uses unrounded scale for more accurate subpixel position.
Point subpixel = TextFrame::ComputeSubpixelPosition(
glyph_position, font.GetAxisAlignment(), offset_, scale_);

std::optional<FrameBounds> maybe_atlas_glyph_bounds =
font_atlas->FindGlyphBounds(SubpixelGlyph{
glyph_position.glyph, //
subpixel, //
GetGlyphProperties() //
});
if (!maybe_atlas_glyph_bounds.has_value()) {
VALIDATION_LOG << "Could not find glyph position in the atlas.";
continue;
}
atlas_glyph_bounds =
maybe_atlas_glyph_bounds.value().atlas_bounds;
// Note: uses unrounded scale for more accurate subpixel position.
Point subpixel = TextFrame::ComputeSubpixelPosition(
glyph_position, font.GetAxisAlignment(), offset_, scale_);
std::optional<std::pair<Rect, Rect>> maybe_atlas_glyph_bounds =
font_atlas->FindGlyphBounds(SubpixelGlyph{
glyph_position.glyph, subpixel,
(properties_.stroke || frame_->HasColor())
? std::optional<GlyphProperties>(properties_)
: std::nullopt});
if (!maybe_atlas_glyph_bounds.has_value()) {
VALIDATION_LOG << "Could not find glyph position in the atlas.";
continue;
}

const Rect& atlas_glyph_bounds =
maybe_atlas_glyph_bounds.value().first;
Rect glyph_bounds = maybe_atlas_glyph_bounds.value().second;
Rect scaled_bounds = glyph_bounds.Scale(1.0 / rounded_scale);
// For each glyph, we compute two rectangles. One for the vertex
// positions and one for the texture coordinates (UVs). The atlas
Expand Down Expand Up @@ -289,10 +266,4 @@ bool TextContents::Render(const ContentContext& renderer,
return pass.Draw().ok();
}

std::optional<GlyphProperties> TextContents::GetGlyphProperties() const {
return (properties_.stroke || frame_->HasColor())
? std::optional<GlyphProperties>(properties_)
: std::nullopt;
}

} // namespace impeller
2 changes: 0 additions & 2 deletions impeller/entity/contents/text_contents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ class TextContents final : public Contents {
RenderPass& pass) const override;

private:
std::optional<GlyphProperties> GetGlyphProperties() const;

std::shared_ptr<TextFrame> frame_;
Scalar scale_ = 1.0;
Scalar inherited_opacity_ = 1.0;
Expand Down
120 changes: 45 additions & 75 deletions impeller/typographer/backends/skia/typographer_context_skia.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,7 @@ static bool BulkUpdateAtlasBitmap(const GlyphAtlas& atlas,
if (!data.has_value()) {
continue;
}
auto [pos, bounds, placeholder] = data.value();
FML_DCHECK(!placeholder);
auto [pos, bounds] = data.value();
Size size = pos.GetSize();
if (size.IsEmpty()) {
continue;
Expand Down Expand Up @@ -326,9 +325,7 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas,
if (!data.has_value()) {
continue;
}
auto [pos, bounds, placeholder] = data.value();
FML_DCHECK(!placeholder);

auto [pos, bounds] = data.value();
Size size = pos.GetSize();
if (size.IsEmpty()) {
continue;
Expand Down Expand Up @@ -405,88 +402,61 @@ static Rect ComputeGlyphSize(const SkFont& font,
scaled_bounds.fBottom);
};

std::pair<std::vector<FontGlyphPair>, std::vector<Rect>>
TypographerContextSkia::CollectNewGlyphs(
const std::shared_ptr<GlyphAtlas>& atlas,
const std::vector<std::shared_ptr<TextFrame>>& text_frames) {
std::vector<FontGlyphPair> new_glyphs;
std::vector<Rect> glyph_sizes;
for (const auto& frame : text_frames) {
// TODO(jonahwilliams): unless we destroy the atlas (which we know about),
// we could probably guarantee that a text frame that is complete does not
// need to be processed unless the scale or properties changed. I'm leaving
// this as a future optimization.
frame->ClearFrameBounds();

for (const auto& run : frame->GetRuns()) {
auto metrics = run.GetFont().GetMetrics();

auto rounded_scale =
TextFrame::RoundScaledFontSize(frame->GetScale(), metrics.point_size);
ScaledFont scaled_font{.font = run.GetFont(), .scale = rounded_scale};

FontGlyphAtlas* font_glyph_atlas =
atlas->GetOrCreateFontGlyphAtlas(scaled_font);
FML_DCHECK(!!font_glyph_atlas);

SkFont sk_font(
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);
sk_font.setEmbolden(metrics.embolden);
// 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.setSubpixel(true);

for (const auto& glyph_position : run.GetGlyphPositions()) {
Point subpixel = TextFrame::ComputeSubpixelPosition(
glyph_position, scaled_font.font.GetAxisAlignment(),
frame->GetOffset(), rounded_scale);
SubpixelGlyph subpixel_glyph(glyph_position.glyph, subpixel,
frame->GetProperties());
const auto& font_glyph_bounds =
font_glyph_atlas->FindGlyphBounds(subpixel_glyph);

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);
glyph_sizes.push_back(glyph_bounds);

auto frame_bounds = FrameBounds{
Rect::MakeLTRB(0, 0, 0, 0), //
glyph_bounds, //
/*placeholder=*/true //
};

frame->AppendFrameBounds(frame_bounds);
font_glyph_atlas->AppendGlyph(subpixel_glyph, frame_bounds);
} else {
frame->AppendFrameBounds(font_glyph_bounds.value());
static void CollectNewGlyphs(const std::shared_ptr<GlyphAtlas>& atlas,
const FontGlyphMap& font_glyph_map,
std::vector<FontGlyphPair>& new_glyphs,
std::vector<Rect>& glyph_sizes) {
for (const auto& font_value : font_glyph_map) {
const ScaledFont& scaled_font = font_value.first;
const FontGlyphAtlas* font_glyph_atlas =
atlas->GetFontGlyphAtlas(scaled_font.font, scaled_font.scale);

auto metrics = scaled_font.font.GetMetrics();

SkFont sk_font(
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);
sk_font.setEmbolden(metrics.embolden);
// 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.setSubpixel(true);

if (font_glyph_atlas) {
for (const SubpixelGlyph& glyph : font_value.second) {
if (!font_glyph_atlas->FindGlyphBounds(glyph)) {
new_glyphs.emplace_back(scaled_font, glyph);
glyph_sizes.push_back(
ComputeGlyphSize(sk_font, glyph, scaled_font.scale));
}
}
} else {
for (const SubpixelGlyph& glyph : font_value.second) {
new_glyphs.emplace_back(scaled_font, glyph);
glyph_sizes.push_back(
ComputeGlyphSize(sk_font, glyph, scaled_font.scale));
}
}
}
return {std::move(new_glyphs), std::move(glyph_sizes)};
}

std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
Context& context,
GlyphAtlas::Type type,
HostBuffer& host_buffer,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
const std::vector<std::shared_ptr<TextFrame>>& text_frames) const {
const FontGlyphMap& font_glyph_map) const {
TRACE_EVENT0("impeller", __FUNCTION__);
if (!IsValid()) {
return nullptr;
}
std::shared_ptr<GlyphAtlas> last_atlas = atlas_context->GetGlyphAtlas();
FML_DCHECK(last_atlas->GetType() == type);

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

Expand All @@ -495,7 +465,9 @@ std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
// with the current atlas and reuse if possible. For each new font and
// glyph pair, compute the glyph size at scale.
// ---------------------------------------------------------------------------
auto [new_glyphs, glyph_sizes] = CollectNewGlyphs(last_atlas, text_frames);
std::vector<FontGlyphPair> new_glyphs;
std::vector<Rect> glyph_sizes;
CollectNewGlyphs(last_atlas, font_glyph_map, new_glyphs, glyph_sizes);
if (new_glyphs.size() == 0) {
return last_atlas;
}
Expand Down Expand Up @@ -564,11 +536,9 @@ std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
blit_old_atlas = false;
new_atlas = std::make_shared<GlyphAtlas>(type);

auto [update_glyphs, update_sizes] =
CollectNewGlyphs(new_atlas, text_frames);
new_glyphs = std::move(update_glyphs);
glyph_sizes = std::move(update_sizes);

new_glyphs.clear();
glyph_sizes.clear();
CollectNewGlyphs(new_atlas, font_glyph_map, new_glyphs, glyph_sizes);
glyph_positions.clear();
glyph_positions.reserve(new_glyphs.size());
first_missing_index = 0;
Expand Down
7 changes: 1 addition & 6 deletions impeller/typographer/backends/skia/typographer_context_skia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,9 @@ class TypographerContextSkia : public TypographerContext {
GlyphAtlas::Type type,
HostBuffer& host_buffer,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
const std::vector<std::shared_ptr<TextFrame>>& text_frames)
const override;
const FontGlyphMap& font_glyph_map) const override;

private:
static std::pair<std::vector<FontGlyphPair>, std::vector<Rect>>
CollectNewGlyphs(const std::shared_ptr<GlyphAtlas>& atlas,
const std::vector<std::shared_ptr<TextFrame>>& text_frames);

TypographerContextSkia(const TypographerContextSkia&) = delete;

TypographerContextSkia& operator=(const TypographerContextSkia&) = delete;
Expand Down
Loading