[Impeller] if drawTextFrame scale is massive, convert to Path. (#166234)
Fixes https://github.com/flutter/flutter/issues/165583 Fixes https://github.com/flutter/flutter/issues/136112 If the text scale gets too big, its faster (and higher res) to switch to path rendering than to keep using the glyph atlas cache.
This commit is contained in:
@@ -3900,7 +3900,6 @@ TEST_F(DisplayListRendering, DrawTextBlob) {
|
||||
// ~12 on top and ~8 on the bottom, so we add 33h & 13v allowed
|
||||
// padding to the tolerance
|
||||
CanvasCompareTester::DefaultTolerance.addBoundsPadding(33, 13));
|
||||
EXPECT_TRUE(blob->unique());
|
||||
#endif // OS_FUCHSIA
|
||||
}
|
||||
|
||||
|
||||
@@ -190,6 +190,34 @@ TEST_P(AiksTest, ScaledK) {
|
||||
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
|
||||
}
|
||||
|
||||
// This is a test that looks for glyph artifacts we've see.
|
||||
TEST_P(AiksTest, MassiveScaleConvertToPath) {
|
||||
Scalar scale = 16.0;
|
||||
auto callback = [&]() -> sk_sp<DisplayList> {
|
||||
if (AiksTest::ImGuiBegin("Controls", nullptr,
|
||||
ImGuiWindowFlags_AlwaysAutoResize)) {
|
||||
ImGui::SliderFloat("Scale", &scale, 4, 20);
|
||||
ImGui::End();
|
||||
}
|
||||
|
||||
DisplayListBuilder builder;
|
||||
DlPaint paint;
|
||||
paint.setColor(DlColor::ARGB(1, 0.1, 0.1, 0.1));
|
||||
builder.DrawPaint(paint);
|
||||
|
||||
builder.Scale(scale, scale);
|
||||
RenderTextInCanvasSkia(
|
||||
GetContext(), builder, "HELLO", "Roboto-Regular.ttf",
|
||||
TextRenderOptions{.font_size = 16,
|
||||
.color = (16 * scale >= 250) ? DlColor::kYellow()
|
||||
: DlColor::kOrange(),
|
||||
.position = DlPoint(0, 20)});
|
||||
return builder.Build();
|
||||
};
|
||||
|
||||
ASSERT_TRUE(OpenPlaygroundHere(callback));
|
||||
}
|
||||
|
||||
TEST_P(AiksTest, CanRenderTextFrameWithScalingOverflow) {
|
||||
Scalar scale = 60.0;
|
||||
Scalar offsetx = -500.0;
|
||||
@@ -844,5 +872,32 @@ TEST_P(AiksTest, SingleIconShadowTest) {
|
||||
1u);
|
||||
}
|
||||
|
||||
TEST_P(AiksTest, VarietyOfTextScalesShowingRasterAndPath) {
|
||||
DisplayListBuilder builder;
|
||||
DlPaint paint;
|
||||
paint.setColor(DlColor::ARGB(1, 0.1, 0.1, 0.1));
|
||||
builder.DrawPaint(paint);
|
||||
builder.Scale(GetContentScale().x, GetContentScale().y);
|
||||
|
||||
std::vector<Scalar> scales = {4, 8, 16, 24, 32};
|
||||
std::vector<Scalar> spacing = {8, 8, 8, 8, 8};
|
||||
Scalar space = 16;
|
||||
Scalar x = 0;
|
||||
for (auto i = 0u; i < scales.size(); i++) {
|
||||
builder.Save();
|
||||
builder.Scale(scales[i], scales[i]);
|
||||
RenderTextInCanvasSkia(
|
||||
GetContext(), builder, "lo", "Roboto-Regular.ttf",
|
||||
TextRenderOptions{.font_size = 16, .position = DlPoint(x, space)});
|
||||
space += spacing[i];
|
||||
if (i == 3) {
|
||||
x = 10;
|
||||
space = 16;
|
||||
}
|
||||
builder.Restore();
|
||||
}
|
||||
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
|
||||
}
|
||||
|
||||
} // namespace testing
|
||||
} // namespace impeller
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
#include "display_list/effects/dl_color_filter.h"
|
||||
#include "display_list/effects/dl_color_source.h"
|
||||
#include "display_list/effects/dl_image_filter.h"
|
||||
#include "display_list/geometry/dl_path.h"
|
||||
#include "display_list/image/dl_image.h"
|
||||
#include "flutter/fml/logging.h"
|
||||
#include "flutter/fml/trace_event.h"
|
||||
@@ -1511,9 +1512,27 @@ bool Canvas::AttemptBlurredTextOptimization(
|
||||
}
|
||||
}
|
||||
|
||||
// If the text point size * max basis XY is larger than this value,
|
||||
// render the text as paths (if available) for faster and higher
|
||||
// fidelity rendering. This is a somewhat arbitrary cutoff
|
||||
static constexpr Scalar kMaxTextScale = 250;
|
||||
|
||||
void Canvas::DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
|
||||
Point position,
|
||||
const Paint& paint) {
|
||||
Scalar max_scale = GetCurrentTransform().GetMaxBasisLengthXY();
|
||||
if (max_scale * text_frame->GetFont().GetMetrics().point_size >
|
||||
kMaxTextScale) {
|
||||
fml::StatusOr<flutter::DlPath> path = text_frame->GetPath();
|
||||
if (path.ok()) {
|
||||
Save(1);
|
||||
Concat(Matrix::MakeTranslation(position));
|
||||
DrawPath(path.value().GetPath(), paint);
|
||||
Restore();
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
Entity entity;
|
||||
entity.SetClipDepth(GetClipHeight());
|
||||
entity.SetBlendMode(paint.blend_mode);
|
||||
@@ -1521,7 +1540,7 @@ void Canvas::DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
|
||||
auto text_contents = std::make_shared<TextContents>();
|
||||
text_contents->SetTextFrame(text_frame);
|
||||
text_contents->SetForceTextColor(paint.mask_blur_descriptor.has_value());
|
||||
text_contents->SetScale(GetCurrentTransform().GetMaxBasisLengthXY());
|
||||
text_contents->SetScale(max_scale);
|
||||
text_contents->SetColor(paint.color);
|
||||
text_contents->SetOffset(position);
|
||||
text_contents->SetTextProperties(paint.color, //
|
||||
|
||||
@@ -39,7 +39,10 @@ impeller_component("typographer") {
|
||||
[ "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map" ]
|
||||
}
|
||||
|
||||
deps = [ "//flutter/fml" ]
|
||||
deps = [
|
||||
"//flutter/fml",
|
||||
"//flutter/skia",
|
||||
]
|
||||
}
|
||||
|
||||
impeller_component("typographer_unittests") {
|
||||
|
||||
@@ -6,7 +6,9 @@
|
||||
|
||||
#include <vector>
|
||||
|
||||
#include "flutter/display_list/geometry/dl_path.h"
|
||||
#include "flutter/fml/logging.h"
|
||||
#include "fml/status.h"
|
||||
#include "impeller/typographer/backends/skia/typeface_skia.h"
|
||||
#include "impeller/typographer/font.h"
|
||||
#include "impeller/typographer/glyph.h"
|
||||
@@ -14,6 +16,7 @@
|
||||
#include "third_party/skia/include/core/SkFontMetrics.h"
|
||||
#include "third_party/skia/include/core/SkPaint.h"
|
||||
#include "third_party/skia/include/core/SkRect.h"
|
||||
#include "third_party/skia/modules/skparagraph/include/Paragraph.h" // nogncheck
|
||||
#include "third_party/skia/src/core/SkStrikeSpec.h" // nogncheck
|
||||
#include "third_party/skia/src/core/SkTextBlobPriv.h" // nogncheck
|
||||
|
||||
@@ -83,7 +86,17 @@ std::shared_ptr<TextFrame> MakeTextFrameFromTextBlobSkia(
|
||||
continue;
|
||||
}
|
||||
}
|
||||
return std::make_shared<TextFrame>(runs, ToRect(blob->bounds()), has_color);
|
||||
return std::make_shared<TextFrame>(
|
||||
runs, ToRect(blob->bounds()), has_color,
|
||||
[blob]() -> fml::StatusOr<flutter::DlPath> {
|
||||
SkPath path = skia::textlayout::Paragraph::GetPath(blob.get());
|
||||
if (path.isEmpty()) {
|
||||
return fml::Status(fml::StatusCode::kCancelled, "No path available");
|
||||
}
|
||||
SkPath transformed = path.makeTransform(
|
||||
SkMatrix::Translate(blob->bounds().left(), blob->bounds().top()));
|
||||
return flutter::DlPath(transformed);
|
||||
});
|
||||
}
|
||||
|
||||
} // namespace impeller
|
||||
|
||||
@@ -3,6 +3,8 @@
|
||||
// found in the LICENSE file.
|
||||
|
||||
#include "impeller/typographer/text_frame.h"
|
||||
#include "flutter/display_list/geometry/dl_path.h" // nogncheck
|
||||
#include "fml/status.h"
|
||||
#include "impeller/geometry/scalar.h"
|
||||
#include "impeller/typographer/font.h"
|
||||
#include "impeller/typographer/font_glyph_pair.h"
|
||||
@@ -11,8 +13,14 @@ namespace impeller {
|
||||
|
||||
TextFrame::TextFrame() = default;
|
||||
|
||||
TextFrame::TextFrame(std::vector<TextRun>& runs, Rect bounds, bool has_color)
|
||||
: runs_(std::move(runs)), bounds_(bounds), has_color_(has_color) {}
|
||||
TextFrame::TextFrame(std::vector<TextRun>& runs,
|
||||
Rect bounds,
|
||||
bool has_color,
|
||||
const PathCreator& path_creator)
|
||||
: runs_(std::move(runs)),
|
||||
bounds_(bounds),
|
||||
has_color_(has_color),
|
||||
path_creator_(path_creator) {}
|
||||
|
||||
TextFrame::~TextFrame() = default;
|
||||
|
||||
@@ -138,6 +146,13 @@ void TextFrame::ClearFrameBounds() {
|
||||
bound_values_.clear();
|
||||
}
|
||||
|
||||
fml::StatusOr<flutter::DlPath> TextFrame::GetPath() const {
|
||||
if (path_creator_) {
|
||||
return path_creator_();
|
||||
}
|
||||
return fml::Status(fml::StatusCode::kCancelled, "no path creator specified.");
|
||||
}
|
||||
|
||||
bool TextFrame::IsFrameComplete() const {
|
||||
size_t run_size = 0;
|
||||
for (const auto& x : runs_) {
|
||||
|
||||
@@ -6,13 +6,23 @@
|
||||
#define FLUTTER_IMPELLER_TYPOGRAPHER_TEXT_FRAME_H_
|
||||
|
||||
#include <cstdint>
|
||||
|
||||
#include "fml/status_or.h"
|
||||
#include "impeller/geometry/rational.h"
|
||||
#include "impeller/typographer/glyph.h"
|
||||
#include "impeller/typographer/glyph_atlas.h"
|
||||
#include "impeller/typographer/text_run.h"
|
||||
|
||||
// TODO(https://github.com/flutter/flutter/issues/166593): This is required to
|
||||
// break a cyclical dependency between display list, impeller, and typographer.
|
||||
namespace flutter {
|
||||
class DlPath;
|
||||
}
|
||||
|
||||
namespace impeller {
|
||||
|
||||
using PathCreator = std::function<fml::StatusOr<flutter::DlPath>()>;
|
||||
|
||||
//------------------------------------------------------------------------------
|
||||
/// @brief Represents a collection of shaped text runs.
|
||||
///
|
||||
@@ -25,7 +35,10 @@ class TextFrame {
|
||||
public:
|
||||
TextFrame();
|
||||
|
||||
TextFrame(std::vector<TextRun>& runs, Rect bounds, bool has_color);
|
||||
TextFrame(std::vector<TextRun>& runs,
|
||||
Rect bounds,
|
||||
bool has_color,
|
||||
const PathCreator& path_creator = {});
|
||||
|
||||
~TextFrame();
|
||||
|
||||
@@ -103,12 +116,10 @@ class TextFrame {
|
||||
|
||||
Rational GetScale() const;
|
||||
|
||||
TextFrame& operator=(TextFrame&& other) = default;
|
||||
|
||||
TextFrame(const TextFrame& other) = default;
|
||||
|
||||
const Matrix& GetTransform() const { return transform_; }
|
||||
|
||||
fml::StatusOr<flutter::DlPath> GetPath() const;
|
||||
|
||||
Point GetOffset() const;
|
||||
|
||||
Matrix GetOffsetTransform() const;
|
||||
@@ -128,6 +139,7 @@ class TextFrame {
|
||||
std::vector<TextRun> runs_;
|
||||
Rect bounds_;
|
||||
bool has_color_;
|
||||
const PathCreator path_creator_;
|
||||
|
||||
// Data that is cached when rendering the text frame and is only
|
||||
// valid for the current atlas generation.
|
||||
|
||||
@@ -76,7 +76,7 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter {
|
||||
if (impeller_enabled_) {
|
||||
SkTextBlobRunIterator run(blob.get());
|
||||
if (ShouldRenderAsPath(dl_paints_[paint_id])) {
|
||||
auto path = skia::textlayout::Paragraph::GetPath(blob.get());
|
||||
SkPath path = skia::textlayout::Paragraph::GetPath(blob.get());
|
||||
// If there is no path, this is an emoji and should be drawn as is,
|
||||
// ignoring the color source.
|
||||
if (path.isEmpty()) {
|
||||
|
||||
Reference in New Issue
Block a user