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

[Impeller] Fixes text scaling issues again, this time with perspective #43662

Closed
wants to merge 8 commits into from
Closed
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
48 changes: 48 additions & 0 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2877,6 +2877,35 @@ TEST_P(AiksTest, CanCanvasDrawPicture) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, DrawPictureWithText) {
Canvas subcanvas;
ASSERT_TRUE(RenderTextInCanvas(
GetContext(), subcanvas,
"the quick brown fox jumped over the lazy dog!.?", "Roboto-Regular.ttf"));
subcanvas.Translate({0, 10});
subcanvas.Scale(Vector2(3, 3));
ASSERT_TRUE(RenderTextInCanvas(
GetContext(), subcanvas,
"the quick brown fox jumped over the very big lazy dog!.?",
"Roboto-Regular.ttf"));
auto picture = subcanvas.EndRecordingAsPicture();

Canvas canvas;
canvas.Scale(Vector2(.2, .2));
canvas.Save();
canvas.Translate({200, 200});
canvas.Scale(Vector2(3.5, 3.5)); // The text must not be blurry after this.
canvas.DrawPicture(picture);
canvas.Restore();

canvas.Scale(Vector2(1.5, 1.5));
ASSERT_TRUE(RenderTextInCanvas(
GetContext(), canvas,
"the quick brown fox jumped over the smaller lazy dog!.?",
"Roboto-Regular.ttf"));
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, MatrixBackdropFilter) {
Canvas canvas;
canvas.SaveLayer({}, std::nullopt,
Expand Down Expand Up @@ -2923,5 +2952,24 @@ APPLY_COLOR_FILTER_GRADIENT_TEST(Radial);
APPLY_COLOR_FILTER_GRADIENT_TEST(Conical);
APPLY_COLOR_FILTER_GRADIENT_TEST(Sweep);

TEST_P(AiksTest, DrawScaledTextWithPerspective) {
Canvas canvas;
Paint save_paint;
canvas.SaveLayer(save_paint);
// clang-format off
canvas.Transform(Matrix(
2.000000, 0.000000, 0.000000, 0.000000,
1.445767, 2.637070, -0.507928, 0.001524,
-2.451887, -0.534662, 0.861399, -0.002584,
1063.481934, 1025.951416, -48.300270, 1.144901
));
// clang-format on

ASSERT_TRUE(RenderTextInCanvas(GetContext(), canvas, "Hello world",
"Roboto-Regular.ttf"));

ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

} // namespace testing
} // namespace impeller
5 changes: 0 additions & 5 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ void Canvas::Initialize(std::optional<Rect> cull_rect) {
base_pass_ = std::make_unique<EntityPass>();
current_pass_ = base_pass_.get();
xformation_stack_.emplace_back(CanvasStackEntry{.cull_rect = cull_rect});
lazy_glyph_atlas_ = std::make_shared<LazyGlyphAtlas>();
FML_DCHECK(GetSaveCount() == 1u);
FML_DCHECK(base_pass_->GetSubpassesDepth() == 1u);
}
Expand All @@ -51,7 +50,6 @@ void Canvas::Reset() {
base_pass_ = nullptr;
current_pass_ = nullptr;
xformation_stack_ = {};
lazy_glyph_atlas_ = nullptr;
}

void Canvas::Save() {
Expand Down Expand Up @@ -516,15 +514,12 @@ void Canvas::SaveLayer(const Paint& paint,
void Canvas::DrawTextFrame(const TextFrame& text_frame,
Point position,
const Paint& paint) {
lazy_glyph_atlas_->AddTextFrame(text_frame);

Entity entity;
entity.SetStencilDepth(GetStencilDepth());
entity.SetBlendMode(paint.blend_mode);

auto text_contents = std::make_shared<TextContents>();
text_contents->SetTextFrame(text_frame);
text_contents->SetGlyphAtlas(lazy_glyph_atlas_);

if (paint.color_source.GetType() != ColorSource::Type::kColor) {
auto color_text_contents = std::make_shared<ColorSourceTextContents>();
Expand Down
1 change: 0 additions & 1 deletion impeller/aiks/canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ class Canvas {
std::unique_ptr<EntityPass> base_pass_;
EntityPass* current_pass_ = nullptr;
std::deque<CanvasStackEntry> xformation_stack_;
std::shared_ptr<LazyGlyphAtlas> lazy_glyph_atlas_;
std::optional<Rect> initial_cull_rect_;

void Initialize(std::optional<Rect> cull_rect);
Expand Down
3 changes: 1 addition & 2 deletions impeller/display_list/dl_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1104,8 +1104,7 @@ void DlDispatcher::drawDisplayList(
void DlDispatcher::drawTextBlob(const sk_sp<SkTextBlob> blob,
SkScalar x,
SkScalar y) {
Scalar scale = canvas_.GetCurrentTransformation().GetMaxBasisLengthXY();
const auto text_frame = TextFrameFromTextBlob(blob, scale);
const auto text_frame = TextFrameFromTextBlob(blob);
if (paint_.style == Paint::Style::kStroke) {
auto path = skia_conversions::PathDataFromTextBlob(blob);
auto bounds = text_frame.GetBounds();
Expand Down
7 changes: 7 additions & 0 deletions impeller/entity/contents/color_source_text_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "impeller/entity/contents/color_source_text_contents.h"

#include "color_source_text_contents.h"
#include "impeller/entity/contents/content_context.h"
#include "impeller/entity/contents/texture_contents.h"
#include "impeller/renderer/render_pass.h"
Expand Down Expand Up @@ -33,6 +34,12 @@ void ColorSourceTextContents::SetTextPosition(Point position) {
position_ = position;
}

void ColorSourceTextContents::PopulateGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
Scalar scale) const {
text_contents_->PopulateGlyphAtlas(lazy_glyph_atlas, scale);
}

bool ColorSourceTextContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down
5 changes: 5 additions & 0 deletions impeller/entity/contents/color_source_text_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ class ColorSourceTextContents final : public Contents {
// |Contents|
std::optional<Rect> GetCoverage(const Entity& entity) const override;

// |Contents|
void PopulateGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
Scalar scale) const override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand Down
10 changes: 10 additions & 0 deletions impeller/entity/contents/content_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,18 @@ class ContentContext {
const SubpassCallback& subpass_callback,
bool msaa_enabled = true) const;

void SetLazyGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas) {
lazy_glyph_atlas_ = lazy_glyph_atlas;
}

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

private:
std::shared_ptr<Context> context_;
std::shared_ptr<LazyGlyphAtlas> lazy_glyph_atlas_;

template <class T>
using Variants = std::unordered_map<ContentContextOptions,
Expand Down
6 changes: 6 additions & 0 deletions impeller/entity/contents/contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "impeller/geometry/color.h"
#include "impeller/geometry/rect.h"
#include "impeller/renderer/snapshot.h"
#include "impeller/typographer/lazy_glyph_atlas.h"

namespace impeller {

Expand Down Expand Up @@ -53,6 +54,11 @@ class Contents {

virtual ~Contents();

/// @brief Add any text data to the specified lazy atlas.
virtual void PopulateGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
Scalar scale) const {}

virtual bool Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const = 0;
Expand Down
99 changes: 46 additions & 53 deletions impeller/entity/contents/text_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,15 @@ void TextContents::SetTextFrame(const TextFrame& frame) {
frame_ = frame;
}

void TextContents::SetGlyphAtlas(std::shared_ptr<LazyGlyphAtlas> atlas) {
lazy_atlas_ = std::move(atlas);
}

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

return nullptr;
Expand Down Expand Up @@ -78,14 +75,43 @@ std::optional<Rect> TextContents::GetCoverage(const Entity& entity) const {
return bounds->TransformBounds(entity.GetTransformation());
}

static bool CommonRender(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass,
const Color& color,
const TextFrame& frame,
Vector2 offset,
const std::shared_ptr<GlyphAtlas>& atlas,
Command& cmd) {
void TextContents::PopulateGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
Scalar scale) const {
lazy_glyph_atlas->AddTextFrame(frame_, scale);
}

bool TextContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
auto color = GetColor();
if (color.IsTransparent()) {
return true;
}

auto type = frame_.GetAtlasType();
auto scale = entity.DeriveTextScale();
Copy link
Member

Choose a reason for hiding this comment

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

Rather than using the render-time entity to derive the correct scale of the text, why not record the derived scale (or even the full matrix) to TextContents when it's being built by Aiks? We know the CTM in that moment. Then, we just use that value for both the rendering lookup + PopulateGlyphAtlas without having to worry about fuzzy lookup problems.

Copy link
Member

Choose a reason for hiding this comment

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

TextContents could technically get shared between entities given it's a shared_ptr, but in practice we don't share contents between different rendering operations today.

And if we did, we'd likely only do this for color sources, not special rendering operations that cross color source concerns with geometry concerns like with text.

auto atlas =
ResolveAtlas(type, renderer.GetLazyGlyphAtlas(),
renderer.GetGlyphAtlasContext(type), renderer.GetContext());

if (!atlas || !atlas->IsValid()) {
VALIDATION_LOG << "Cannot render glyphs without prepared atlas.";
return false;
}

// Information shared by all glyph draw calls.
Command cmd;
cmd.label = "TextFrame";
auto opts = OptionsFromPassAndEntity(pass, entity);
opts.primitive_type = PrimitiveType::kTriangle;
if (type == GlyphAtlas::Type::kAlphaBitmap) {
cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts);
} else {
cmd.pipeline = renderer.GetGlyphAtlasColorPipeline(opts);
}
cmd.stencil_reference = entity.GetStencilDepth();

using VS = GlyphAtlasPipeline::VertexShader;
using FS = GlyphAtlasPipeline::FragmentShader;

Expand All @@ -95,7 +121,7 @@ static bool CommonRender(const ContentContext& renderer,
frame_info.atlas_size =
Vector2{static_cast<Scalar>(atlas->GetTexture()->GetSize().width),
static_cast<Scalar>(atlas->GetTexture()->GetSize().height)};
frame_info.offset = offset;
frame_info.offset = offset_;
frame_info.is_translation_scale =
entity.GetTransformation().IsTranslationScaleOnly();
frame_info.entity_transform = entity.GetTransformation();
Expand Down Expand Up @@ -141,7 +167,7 @@ static bool CommonRender(const ContentContext& renderer,

auto& host_buffer = pass.GetTransientsBuffer();
size_t vertex_count = 0;
for (const auto& run : frame.GetRuns()) {
for (const auto& run : frame_.GetRuns()) {
vertex_count += run.GetGlyphPositions().size();
}
vertex_count *= 6;
Expand All @@ -151,10 +177,10 @@ static bool CommonRender(const ContentContext& renderer,
[&](uint8_t* contents) {
VS::PerVertexData vtx;
size_t vertex_offset = 0;
for (const auto& run : frame.GetRuns()) {
for (const auto& run : frame_.GetRuns()) {
const Font& font = run.GetFont();
for (const auto& glyph_position : run.GetGlyphPositions()) {
FontGlyphPair font_glyph_pair{font, glyph_position.glyph};
FontGlyphPair font_glyph_pair{font, glyph_position.glyph, scale};
auto maybe_atlas_glyph_bounds =
atlas->FindFontGlyphBounds(font_glyph_pair);
if (!maybe_atlas_glyph_bounds.has_value()) {
Expand Down Expand Up @@ -191,37 +217,4 @@ static bool CommonRender(const ContentContext& renderer,
return pass.AddCommand(cmd);
}

bool TextContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
auto color = GetColor();
if (color.IsTransparent()) {
return true;
}

auto type = frame_.GetAtlasType();
auto atlas = ResolveAtlas(type, renderer.GetGlyphAtlasContext(type),
renderer.GetContext());

if (!atlas || !atlas->IsValid()) {
VALIDATION_LOG << "Cannot render glyphs without prepared atlas.";
return false;
}

// Information shared by all glyph draw calls.
Command cmd;
cmd.label = "TextFrame";
auto opts = OptionsFromPassAndEntity(pass, entity);
opts.primitive_type = PrimitiveType::kTriangle;
if (type == GlyphAtlas::Type::kAlphaBitmap) {
cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts);
} else {
cmd.pipeline = renderer.GetGlyphAtlasColorPipeline(opts);
}
cmd.stencil_reference = entity.GetStencilDepth();

return CommonRender(renderer, entity, pass, color, frame_, offset_, atlas,
cmd);
}

} // namespace impeller
11 changes: 8 additions & 3 deletions impeller/entity/contents/text_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ class TextContents final : public Contents {

void SetTextFrame(const TextFrame& frame);

void SetGlyphAtlas(std::shared_ptr<LazyGlyphAtlas> atlas);

void SetColor(Color color);

Color GetColor() const;

// |Contents|
bool CanInheritOpacity(const Entity& entity) const override;

// |Contents|
void SetInheritedOpacity(Scalar opacity) override;

void SetOffset(Vector2 offset);
Expand All @@ -45,6 +45,11 @@ class TextContents final : public Contents {
// |Contents|
std::optional<Rect> GetCoverage(const Entity& entity) const override;

// |Contents|
void PopulateGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
Scalar scale) const override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand All @@ -54,11 +59,11 @@ class TextContents final : public Contents {
TextFrame frame_;
Color color_;
Scalar inherited_opacity_ = 1.0;
mutable std::shared_ptr<LazyGlyphAtlas> lazy_atlas_;
Vector2 offset_;

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

Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/entity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,8 @@ bool Entity::Render(const ContentContext& renderer,
return contents_->Render(renderer, *this, parent_pass);
}

Scalar Entity::DeriveTextScale() const {
return GetTransformation().GetMaxBasisLengthXY();
}

} // namespace impeller
2 changes: 2 additions & 0 deletions impeller/entity/entity.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class Entity {

std::optional<Color> AsBackgroundColor(ISize target_size) const;

Scalar DeriveTextScale() const;

private:
Matrix transformation_;
std::shared_ptr<Contents> contents_;
Expand Down
Loading