Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Impeller] Fix text scaling issues again, this time with perspective when a save layer is involved #43695

Merged
merged 3 commits into from
Jul 14, 2023
Merged

[Impeller] Fix text scaling issues again, this time with perspective when a save layer is involved #43695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e6f1d0
Revert "Revert https://github.com/flutter/engine/pull/43533 (#43654)"
dnfield Jul 14, 2023
c288069
Record the CTM scale independent of how EntityPass::Render may change it
dnfield Jul 14, 2023
9fdf775
Update impeller/entity/contents/text_contents.h
dnfield Jul 14, 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
65 changes: 65 additions & 0 deletions impeller/aiks/aiks_unittests.cc
View file
Open in desktop
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) {
Copy link
Member

Choose a reason for hiding this comment

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

Sorta nit: The text seems to either be invisible or hard to see in the goldens with the golden service's white background. Maybe throw in black backgrounds with DrawPaint?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, seems like the new goldens aren't rendering anything for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did the gold test part have a chance to run?

The text is red, I thought that should render on either black or white background fine :\

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But before this change, the new tests would've had validation errors (the atlas would not contain the scale and the test would crash)

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,41 @@ APPLY_COLOR_FILTER_GRADIENT_TEST(Radial);
APPLY_COLOR_FILTER_GRADIENT_TEST(Conical);
APPLY_COLOR_FILTER_GRADIENT_TEST(Sweep);

TEST_P(AiksTest, DrawScaledTextWithPerspectiveNoSaveLayer) {
Canvas canvas;
// 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()));
}

TEST_P(AiksTest, DrawScaledTextWithPerspectiveSaveLayer) {
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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) {
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
View file
Open in desktop
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) override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand Down
10 changes: 10 additions & 0 deletions impeller/entity/contents/content_context.h
View file
Open in desktop
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
7 changes: 7 additions & 0 deletions impeller/entity/contents/contents.h
View file
Open in desktop
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,12 @@ class Contents {

virtual ~Contents();

/// @brief Add any text data to the specified lazy atlas. The scale parameter
/// must be used again later when drawing the text.
virtual void PopulateGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
Scalar scale) {}

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
View file
Open in desktop
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) {
lazy_glyph_atlas->AddTextFrame(frame_, scale);
scale_ = 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 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
12 changes: 9 additions & 3 deletions impeller/entity/contents/text_contents.h
View file
Open in desktop
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,20 +45,26 @@ 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) override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const override;

private:
TextFrame frame_;
Scalar scale_ = 1.0;
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
View file
Open in desktop
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
Loading