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

[Impeller] remove shared_ptr copies during text contents rendering. #49837

Merged
merged 3 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions impeller/entity/contents/content_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -731,11 +731,11 @@ class ContentContext {
const RenderTarget& subpass_target,
const SubpassCallback& subpass_callback) const;

std::shared_ptr<LazyGlyphAtlas> GetLazyGlyphAtlas() const {
const std::shared_ptr<LazyGlyphAtlas>& GetLazyGlyphAtlas() const {
return lazy_glyph_atlas_;
}

std::shared_ptr<RenderTargetAllocator> GetRenderTargetCache() const {
const std::shared_ptr<RenderTargetAllocator>& GetRenderTargetCache() const {
return render_target_cache_;
}

Expand Down
17 changes: 3 additions & 14 deletions impeller/entity/contents/text_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,6 @@ void TextContents::SetTextFrame(const std::shared_ptr<TextFrame>& frame) {
frame_ = frame;
}

std::shared_ptr<GlyphAtlas> TextContents::ResolveAtlas(
Context& context,
GlyphAtlas::Type type,
const std::shared_ptr<LazyGlyphAtlas>& lazy_atlas) const {
FML_DCHECK(lazy_atlas);
if (lazy_atlas) {
return lazy_atlas->CreateOrGetGlyphAtlas(context, type);
}

return nullptr;
}

void TextContents::SetColor(Color color) {
color_ = color;
}
Expand Down Expand Up @@ -82,8 +70,9 @@ bool TextContents::Render(const ContentContext& renderer,
}

auto type = frame_->GetAtlasType();
auto atlas =
ResolveAtlas(*renderer.GetContext(), type, renderer.GetLazyGlyphAtlas());
const std::shared_ptr<GlyphAtlas>& atlas =
renderer.GetLazyGlyphAtlas()->CreateOrGetGlyphAtlas(
*renderer.GetContext(), type);

if (!atlas || !atlas->IsValid()) {
VALIDATION_LOG << "Cannot render glyphs without prepared atlas.";
Expand Down
9 changes: 0 additions & 9 deletions impeller/entity/contents/text_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,8 @@
#ifndef FLUTTER_IMPELLER_ENTITY_CONTENTS_TEXT_CONTENTS_H_
#define FLUTTER_IMPELLER_ENTITY_CONTENTS_TEXT_CONTENTS_H_

#include <functional>
#include <memory>
#include <variant>
#include <vector>

#include "flutter/fml/macros.h"
#include "impeller/entity/contents/contents.h"
#include "impeller/geometry/color.h"
#include "impeller/typographer/glyph_atlas.h"
Expand Down Expand Up @@ -70,11 +66,6 @@ class TextContents final : public Contents {
Vector2 offset_;
bool force_text_color_ = false;

std::shared_ptr<GlyphAtlas> ResolveAtlas(
Context& context,
GlyphAtlas::Type type,
const std::shared_ptr<LazyGlyphAtlas>& lazy_atlas) const;

TextContents(const TextContents&) = delete;

TextContents& operator=(const TextContents&) = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ static std::shared_ptr<Texture> UploadGlyphTextureAtlas(
std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
Context& context,
GlyphAtlas::Type type,
std::shared_ptr<GlyphAtlasContext> atlas_context,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
const FontGlyphMap& font_glyph_map) const {
TRACE_EVENT0("impeller", __FUNCTION__);
if (!IsValid()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef FLUTTER_IMPELLER_TYPOGRAPHER_BACKENDS_SKIA_TYPOGRAPHER_CONTEXT_SKIA_H_
#define FLUTTER_IMPELLER_TYPOGRAPHER_BACKENDS_SKIA_TYPOGRAPHER_CONTEXT_SKIA_H_

#include "flutter/fml/macros.h"
#include "impeller/typographer/typographer_context.h"

namespace impeller {
Expand All @@ -25,7 +24,7 @@ class TypographerContextSkia : public TypographerContext {
std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
Context& context,
GlyphAtlas::Type type,
std::shared_ptr<GlyphAtlasContext> atlas_context,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
const FontGlyphMap& font_glyph_map) const override;

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ static std::shared_ptr<Texture> UploadGlyphTextureAtlas(
std::shared_ptr<GlyphAtlas> TypographerContextSTB::CreateGlyphAtlas(
Context& context,
GlyphAtlas::Type type,
std::shared_ptr<GlyphAtlasContext> atlas_context,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
const FontGlyphMap& font_glyph_map) const {
TRACE_EVENT0("impeller", __FUNCTION__);
if (!IsValid()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class TypographerContextSTB : public TypographerContext {
std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
Context& context,
GlyphAtlas::Type type,
std::shared_ptr<GlyphAtlasContext> atlas_context,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
const FontGlyphMap& font_glyph_map) const override;

private:
Expand Down
42 changes: 28 additions & 14 deletions impeller/typographer/lazy_glyph_atlas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@

#include "impeller/typographer/lazy_glyph_atlas.h"

#include "fml/logging.h"
#include "impeller/base/validation.h"
#include "impeller/typographer/glyph_atlas.h"
#include "impeller/typographer/typographer_context.h"

#include <utility>

namespace impeller {

static const std::shared_ptr<GlyphAtlas> kNullGlyphAtlas = nullptr;

LazyGlyphAtlas::LazyGlyphAtlas(
std::shared_ptr<TypographerContext> typographer_context)
: typographer_context_(std::move(typographer_context)),
Expand All @@ -24,7 +28,7 @@ LazyGlyphAtlas::LazyGlyphAtlas(
LazyGlyphAtlas::~LazyGlyphAtlas() = default;

void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale) {
FML_DCHECK(atlas_map_.empty());
FML_DCHECK(alpha_atlas_ == nullptr && color_atlas_ == nullptr);
if (frame.GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) {
frame.CollectUniqueFontGlyphPairs(alpha_glyph_map_, scale);
} else {
Expand All @@ -35,42 +39,52 @@ void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale) {
void LazyGlyphAtlas::ResetTextFrames() {
alpha_glyph_map_.clear();
color_glyph_map_.clear();
atlas_map_.clear();
alpha_atlas_.reset();
color_atlas_.reset();
}

std::shared_ptr<GlyphAtlas> LazyGlyphAtlas::CreateOrGetGlyphAtlas(
const std::shared_ptr<GlyphAtlas>& LazyGlyphAtlas::CreateOrGetGlyphAtlas(
Context& context,
GlyphAtlas::Type type) const {
{
auto atlas_it = atlas_map_.find(type);
if (atlas_it != atlas_map_.end()) {
return atlas_it->second;
if (type == GlyphAtlas::Type::kAlphaBitmap && alpha_atlas_) {
return alpha_atlas_;
}
if (type == GlyphAtlas::Type::kColorBitmap && color_atlas_) {
return color_atlas_;
}
}

if (!typographer_context_) {
VALIDATION_LOG << "Unable to render text because a TypographerContext has "
"not been set.";
return nullptr;
return kNullGlyphAtlas;
}
if (!typographer_context_->IsValid()) {
VALIDATION_LOG
<< "Unable to render text because the TypographerContext is invalid.";
return nullptr;
return kNullGlyphAtlas;
}

auto& glyph_map = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_glyph_map_
: color_glyph_map_;
auto atlas_context =
const std::shared_ptr<GlyphAtlasContext>& atlas_context =
type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_;
auto atlas = typographer_context_->CreateGlyphAtlas(context, type,
atlas_context, glyph_map);
std::shared_ptr<GlyphAtlas> atlas = typographer_context_->CreateGlyphAtlas(
context, type, atlas_context, glyph_map);
if (!atlas || !atlas->IsValid()) {
VALIDATION_LOG << "Could not create valid atlas.";
return nullptr;
return kNullGlyphAtlas;
}
if (type == GlyphAtlas::Type::kAlphaBitmap) {
alpha_atlas_ = std::move(atlas);
return alpha_atlas_;
}
if (type == GlyphAtlas::Type::kColorBitmap) {
color_atlas_ = std::move(atlas);
return color_atlas_;
}
atlas_map_[type] = atlas;
return atlas;
FML_UNREACHABLE();
}

} // namespace impeller
6 changes: 3 additions & 3 deletions impeller/typographer/lazy_glyph_atlas.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class LazyGlyphAtlas {

void ResetTextFrames();

std::shared_ptr<GlyphAtlas> CreateOrGetGlyphAtlas(
const std::shared_ptr<GlyphAtlas>& CreateOrGetGlyphAtlas(
Context& context,
GlyphAtlas::Type type) const;

Expand All @@ -37,8 +37,8 @@ class LazyGlyphAtlas {
FontGlyphMap color_glyph_map_;
std::shared_ptr<GlyphAtlasContext> alpha_context_;
std::shared_ptr<GlyphAtlasContext> color_context_;
mutable std::unordered_map<GlyphAtlas::Type, std::shared_ptr<GlyphAtlas>>
atlas_map_;
mutable std::shared_ptr<GlyphAtlas> alpha_atlas_;
mutable std::shared_ptr<GlyphAtlas> color_atlas_;

LazyGlyphAtlas(const LazyGlyphAtlas&) = delete;

Expand Down
2 changes: 1 addition & 1 deletion impeller/typographer/typographer_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class TypographerContext {
virtual std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
Context& context,
GlyphAtlas::Type type,
std::shared_ptr<GlyphAtlasContext> atlas_context,
const std::shared_ptr<GlyphAtlasContext>& atlas_context,
const FontGlyphMap& font_glyph_map) const = 0;

protected:
Expand Down
2 changes: 1 addition & 1 deletion impeller/typographer/typographer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) {
};
auto atlas =
context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kAlphaBitmap,
std::move(atlas_context), font_glyph_map);
atlas_context, font_glyph_map);
ASSERT_NE(atlas, nullptr);
ASSERT_NE(atlas->GetTexture(), nullptr);

Expand Down