Skip to content

Commit

Permalink
[Impeller] Use scaled font to determine bounds, match Skia position r…
Browse files Browse the repository at this point in the history
…ounding behavior, add subpixel X/Y/All/None positioning. (#53042)

Multiple fixes to text rendering that match skia behavior on almost all bugs I've found, except for the glyphs are still _slightly_ too fine for some CJK text. The fixes are:

1. Compute the gylph size in the typographer context, using text size * scale factor text, instead of computing smaller bounds and scaling it up. This was not accurate and as a result we would positon glyphs incorrect by multiple pixels sometimes, causing uneven rows.

2. Match Skia's rounding behavior. previously we were rounding in multiple places, Skia rounds once. This is important to prevent jumping.

3. Use 4 subpixel X positions for rendering. This is the big one that ensures the visible layout matches exactly. Adds support for Y, both, and none positioning too. I couldn't find any examples of just Y or both. Some fonts may specify that have no subpixel positioning. So we don't bother to compute it for those.

Fixes flutter/flutter#138386 / mostly, except slightly not bold enough.
Fixes flutter/flutter#147577 / mostly, except slightly not bold enough.
Fixes flutter/flutter#140475
Fixes flutter/flutter#141467 
Fixes flutter/flutter#135523
Fixes flutter/flutter#127815
jonahwilliams authored May 28, 2024

Verified

This commit was signed with the committer’s verified signature.
0x2142 Matt Schmitz
1 parent b175108 commit 9f448d3
Showing 26 changed files with 512 additions and 378 deletions.
20 changes: 20 additions & 0 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
@@ -738,6 +738,26 @@ TEST_P(AiksTest, CanRenderTextFrame) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, CanRenderTextFrameWithHalfScaling) {
Canvas canvas;
canvas.DrawPaint({.color = Color(0.1, 0.1, 0.1, 1.0)});
canvas.Scale({0.5, 0.5, 1});
ASSERT_TRUE(RenderTextInCanvasSkia(
GetContext(), canvas, "the quick brown fox jumped over the lazy dog!.?",

This comment has been minimized.

Copy link
@ricardoboss

ricardoboss May 28, 2024

There is no "s" in this sentence. The sentence should contain "jumps" instead of "jumped"

"Roboto-Regular.ttf"));
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, CanRenderTextFrameWithFractionScaling) {
Canvas canvas;
canvas.DrawPaint({.color = Color(0.1, 0.1, 0.1, 1.0)});
canvas.Scale({2.625, 2.625, 1});
ASSERT_TRUE(RenderTextInCanvasSkia(
GetContext(), canvas, "the quick brown fox jumped over the lazy dog!.?",
"Roboto-Regular.ttf"));
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, CanRenderTextFrameSTB) {
Canvas canvas;
canvas.DrawPaint({.color = Color(0.1, 0.1, 0.1, 1.0)});
1 change: 1 addition & 0 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
@@ -883,6 +883,7 @@ void Canvas::DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
text_contents->SetTextFrame(text_frame);
text_contents->SetColor(paint.color);
text_contents->SetForceTextColor(paint.mask_blur_descriptor.has_value());
text_contents->SetOffset(position);

entity.SetTransform(GetCurrentTransform() *
Matrix::MakeTranslation(position));
4 changes: 2 additions & 2 deletions impeller/display_list/dl_dispatcher.cc
Original file line number Diff line number Diff line change
@@ -1249,8 +1249,8 @@ void TextFrameDispatcher::drawTextFrame(
const std::shared_ptr<impeller::TextFrame>& text_frame,
SkScalar x,
SkScalar y) {
renderer_.GetLazyGlyphAtlas()->AddTextFrame(*text_frame,
matrix_.GetMaxBasisLengthXY());
renderer_.GetLazyGlyphAtlas()->AddTextFrame(
*text_frame, matrix_.GetMaxBasisLengthXY(), Point(x, y));
}

void TextFrameDispatcher::drawDisplayList(
95 changes: 75 additions & 20 deletions impeller/entity/contents/text_contents.cc
Original file line number Diff line number Diff line change
@@ -13,6 +13,8 @@
#include "impeller/core/sampler_descriptor.h"
#include "impeller/entity/contents/content_context.h"
#include "impeller/entity/entity.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/point.h"
#include "impeller/renderer/render_pass.h"
#include "impeller/typographer/glyph_atlas.h"
#include "impeller/typographer/lazy_glyph_atlas.h"
@@ -36,7 +38,13 @@ Color TextContents::GetColor() const {
}

bool TextContents::CanInheritOpacity(const Entity& entity) const {
return !frame_->MaybeHasOverlapping();
// Computing whether or not opacity can be inherited requires determining if
// any glyphs can overlap exactly. While this was previously implemented
// via TextFrame::MaybeHasOverlapping, this code relied on scaling up text
// bounds for a size specified at 1.0 DPR, which was not accurate at
// higher or lower DPRs. Rather than re-implement the checks to compute exact
// glyph bounds, for now this optimization has been disabled for Text.
return false;
}

void TextContents::SetInheritedOpacity(Scalar opacity) {
@@ -58,7 +66,7 @@ std::optional<Rect> TextContents::GetCoverage(const Entity& entity) const {
void TextContents::PopulateGlyphAtlas(
const std::shared_ptr<LazyGlyphAtlas>& lazy_glyph_atlas,
Scalar scale) {
lazy_glyph_atlas->AddTextFrame(*frame_, scale);
lazy_glyph_atlas->AddTextFrame(*frame_, scale, offset_);
scale_ = scale;
}

@@ -93,13 +101,10 @@ bool TextContents::Render(const ContentContext& renderer,
VS::FrameInfo frame_info;
frame_info.mvp =
Entity::GetShaderTransform(entity.GetShaderClipDepth(), pass, Matrix());
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.is_translation_scale =
entity.GetTransform().IsTranslationScaleOnly();
frame_info.entity_transform = entity.GetTransform();
ISize atlas_size = atlas->GetTexture()->GetSize();
bool is_translation_scale = entity.GetTransform().IsTranslationScaleOnly();
Matrix entity_transform = entity.GetTransform();
Matrix basis_transform = entity_transform.Basis();

VS::BindFrameInfo(pass,
renderer.GetTransientsBuffer().EmplaceUniform(frame_info));
@@ -113,7 +118,7 @@ bool TextContents::Render(const ContentContext& renderer,
renderer.GetTransientsBuffer().EmplaceUniform(frag_info));

SamplerDescriptor sampler_desc;
if (frame_info.is_translation_scale) {
if (is_translation_scale) {
sampler_desc.min_filter = MinMagFilter::kNearest;
sampler_desc.mag_filter = MinMagFilter::kNearest;
} else {
@@ -160,7 +165,7 @@ bool TextContents::Render(const ContentContext& renderer,
VS::PerVertexData vtx;
VS::PerVertexData* vtx_contents =
reinterpret_cast<VS::PerVertexData*>(contents);
size_t offset = 0u;
size_t i = 0u;
for (const TextRun& run : frame_->GetRuns()) {
const Font& font = run.GetFont();
Scalar rounded_scale = TextFrame::RoundScaledFontSize(
@@ -172,22 +177,72 @@ bool TextContents::Render(const ContentContext& renderer,
continue;
}

// Adjust glyph position based on the subpixel rounding
// used by the font.
Point subpixel_adjustment(0.5, 0.5);
switch (font.GetAxisAlignment()) {
case AxisAlignment::kNone:
break;
case AxisAlignment::kX:
subpixel_adjustment.x = 0.125;
break;
case AxisAlignment::kY:
subpixel_adjustment.y = 0.125;
break;
case AxisAlignment::kAll:
subpixel_adjustment.x = 0.125;
subpixel_adjustment.y = 0.125;
break;
}

Point screen_offset = (entity_transform * Point(0, 0));
for (const TextRun::GlyphPosition& glyph_position :
run.GetGlyphPositions()) {
std::optional<Rect> maybe_atlas_glyph_bounds =
font_atlas->FindGlyphBounds(glyph_position.glyph);
// 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});
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();
vtx.atlas_glyph_bounds = Vector4(atlas_glyph_bounds.GetXYWH());
vtx.glyph_bounds = Vector4(glyph_position.glyph.bounds.GetXYWH());
vtx.glyph_position = glyph_position.position;

const Rect& atlas_glyph_bounds =
maybe_atlas_glyph_bounds.value().first;
Rect glyph_bounds = maybe_atlas_glyph_bounds.value().second;
// For each glyph, we compute two rectangles. One for the vertex
// positions and one for the texture coordinates (UVs). The atlas
// glyph bounds are used to compute UVs in cases where the
// destination and source sizes may differ due to clamping the sizes
// of large glyphs.
Point uv_origin =
(atlas_glyph_bounds.GetLeftTop() - Point(0.5, 0.5)) /
atlas_size;
Point uv_size =
(atlas_glyph_bounds.GetSize() + Point(1, 1)) / atlas_size;

Point unrounded_glyph_position =
(basis_transform * glyph_position.position) +
glyph_bounds.GetLeftTop();
Point screen_glyph_position =
(screen_offset + unrounded_glyph_position + subpixel_adjustment)
.Floor();

Size scaled_size = glyph_bounds.GetSize();
for (const Point& point : unit_points) {
vtx.unit_position = point;
vtx_contents[offset++] = vtx;
Point position;
if (is_translation_scale) {
position = screen_glyph_position + (point * scaled_size);
} else {
Rect scaled_bounds = glyph_bounds.Scale(1.0 / rounded_scale);
position = entity_transform * (glyph_position.position +
scaled_bounds.GetLeftTop() +
point * scaled_bounds.GetSize());
}
vtx.uv = uv_origin + (uv_size * point);
vtx.position = position;
vtx_contents[i++] = vtx;
}
}
}
1 change: 1 addition & 0 deletions impeller/entity/contents/text_contents.h
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@ class TextContents final : public Contents {
// |Contents|
void SetInheritedOpacity(Scalar opacity) override;

// The offset is only used for computing the subpixel glyph position.
void SetOffset(Vector2 offset);

std::optional<Rect> GetTextFrameBounds() const;
20 changes: 0 additions & 20 deletions impeller/entity/entity_unittests.cc
Original file line number Diff line number Diff line change
@@ -2356,26 +2356,6 @@ TEST_P(EntityTest, InheritOpacityTest) {
tiled_texture->SetInheritedOpacity(0.5);
ASSERT_EQ(tiled_texture->GetOpacityFactor(), 0.25);

// Text contents can accept opacity if the text frames do not
// overlap
SkFont font = flutter::testing::CreateTestFontOfSize(30);
auto blob = SkTextBlob::MakeFromString("A", font);
auto frame = MakeTextFrameFromTextBlobSkia(blob);
auto lazy_glyph_atlas =
std::make_shared<LazyGlyphAtlas>(TypographerContextSkia::Make());
lazy_glyph_atlas->AddTextFrame(*frame, 1.0f);

auto text_contents = std::make_shared<TextContents>();
text_contents->SetTextFrame(frame);
text_contents->SetColor(Color::Blue().WithAlpha(0.5));

ASSERT_TRUE(text_contents->CanInheritOpacity(entity));

text_contents->SetInheritedOpacity(0.5);
ASSERT_EQ(text_contents->GetColor().alpha, 0.25);
text_contents->SetInheritedOpacity(0.5);
ASSERT_EQ(text_contents->GetColor().alpha, 0.25);

// Clips and restores trivially accept opacity.
ASSERT_TRUE(ClipContents().CanInheritOpacity(entity));
ASSERT_TRUE(ClipRestoreContents().CanInheritOpacity(entity));
69 changes: 4 additions & 65 deletions impeller/entity/shaders/glyph_atlas.vert
Original file line number Diff line number Diff line change
@@ -1,81 +1,20 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <impeller/transform.glsl>
#include <impeller/types.glsl>

uniform FrameInfo {
mat4 mvp;
mat4 entity_transform;
vec2 atlas_size;
vec2 offset;
float is_translation_scale;
}
frame_info;

// XYWH.
in vec4 atlas_glyph_bounds;
// XYWH
in vec4 glyph_bounds;

in vec2 unit_position;
in vec2 glyph_position;
in vec2 uv;
in vec2 position;

out vec2 v_uv;

mat4 basis(mat4 m) {
return mat4(m[0][0], m[0][1], m[0][2], 0.0, //
m[1][0], m[1][1], m[1][2], 0.0, //
m[2][0], m[2][1], m[2][2], 0.0, //
0.0, 0.0, 0.0, 1.0 //
);
}

vec2 project(mat4 m, vec2 v) {
float w = v.x * m[0][3] + v.y * m[1][3] + m[3][3];
vec2 result = vec2(v.x * m[0][0] + v.y * m[1][0] + m[3][0],
v.x * m[0][1] + v.y * m[1][1] + m[3][1]);

// This is Skia's behavior, but it may be reasonable to allow UB for the w=0
// case.
if (w != 0) {
w = 1 / w;
}
return result * w;
}

void main() {
vec2 screen_offset =
round(project(frame_info.entity_transform, frame_info.offset));

// For each glyph, we compute two rectangles. One for the vertex positions
// and one for the texture coordinates (UVs).
vec2 uv_origin = (atlas_glyph_bounds.xy - vec2(0.5)) / frame_info.atlas_size;
vec2 uv_size = (atlas_glyph_bounds.zw + vec2(1)) / frame_info.atlas_size;

// Rounding here prevents most jitter between glyphs in the run when
// nearest sampling.
mat4 basis_transform = basis(frame_info.entity_transform);
vec2 screen_glyph_position =
screen_offset +
round(project(basis_transform, (glyph_position + glyph_bounds.xy)));

vec4 position;
if (frame_info.is_translation_scale == 1.0) {
// Rouding up here prevents the bounds from becoming 1 pixel too small
// when nearest sampling. This path breaks down for projections.
position = vec4(
screen_glyph_position +
ceil(project(basis_transform, unit_position * glyph_bounds.zw)),
0.0, 1.0);
} else {
position = frame_info.entity_transform *
vec4(frame_info.offset + glyph_position + glyph_bounds.xy +
unit_position * glyph_bounds.zw,
0.0, 1.0);
}

gl_Position = frame_info.mvp * position;
v_uv = uv_origin + unit_position * uv_size;
gl_Position = frame_info.mvp * vec4(position, 0, 1);
v_uv = uv;
}
Loading

0 comments on commit 9f448d3

Please sign in to comment.