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

Commit 513ac2e

Browse files
zandersobdero
andauthored
[CP][Impeller] Switch to nearest sampling for the text atlas (#39180)
* [Impeller] Switch to nearest sampling for the text atlas (#39104) * [Impeller] Linear sample atlas glyphs when the CTM isn't translation/scale only (#39112) --------- Co-authored-by: Brandon DeRosier <bdero@google.com>
1 parent 2a0b3ad commit 513ac2e

File tree

5 files changed

+60
-9
lines changed

5 files changed

+60
-9
lines changed

impeller/entity/contents/text_contents.cc

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,18 @@ static bool CommonRender(
7878
VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info));
7979

8080
SamplerDescriptor sampler_desc;
81-
sampler_desc.min_filter = MinMagFilter::kLinear;
82-
sampler_desc.mag_filter = MinMagFilter::kLinear;
81+
if (entity.GetTransformation().IsTranslationScaleOnly()) {
82+
sampler_desc.min_filter = MinMagFilter::kNearest;
83+
sampler_desc.mag_filter = MinMagFilter::kNearest;
84+
} else {
85+
// Currently, we only propagate the scale of the transform to the atlas
86+
// renderer, so if the transform has more than just a translation, we turn
87+
// on linear sampling to prevent crunchiness caused by the pixel grid not
88+
// being perfectly aligned.
89+
// The downside is that this slightly over-blurs rotated/skewed text.
90+
sampler_desc.min_filter = MinMagFilter::kLinear;
91+
sampler_desc.mag_filter = MinMagFilter::kLinear;
92+
}
8393
sampler_desc.mip_filter = MipFilter::kNone;
8494

8595
typename FS::FragInfo frag_info;
@@ -146,10 +156,10 @@ static bool CommonRender(
146156
for (const auto& point : unit_points) {
147157
typename VS::PerVertexData vtx;
148158
vtx.unit_position = point;
149-
vtx.destination_position = offset_glyph_position + Point(0.5, 0.5);
159+
vtx.destination_position = offset_glyph_position;
150160
vtx.destination_size = Point(glyph_position.glyph.bounds.size);
151-
vtx.source_position = atlas_position + Point(0.5, 0.5);
152-
vtx.source_glyph_size = atlas_glyph_size - Point(1.0, 1.0);
161+
vtx.source_position = atlas_position;
162+
vtx.source_glyph_size = atlas_glyph_size;
153163
if constexpr (std::is_same_v<TPipeline, GlyphAtlasPipeline>) {
154164
vtx.has_color =
155165
glyph_position.glyph.type == Glyph::Type::kBitmap ? 1.0 : 0.0;

impeller/entity/shaders/glyph_atlas.frag

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ out vec4 frag_color;
2121

2222
void main() {
2323
vec2 uv_size = v_source_glyph_size / frag_info.atlas_size;
24-
vec2 offset = v_source_position / frag_info.atlas_size;
24+
vec2 uv_position = v_source_position / frag_info.atlas_size;
2525
if (v_has_color == 1.0) {
2626
frag_color =
27-
texture(glyph_atlas_sampler, v_unit_position * uv_size + offset);
27+
texture(glyph_atlas_sampler, v_unit_position * uv_size + uv_position);
2828
} else {
2929
frag_color =
30-
texture(glyph_atlas_sampler, v_unit_position * uv_size + offset).aaaa *
30+
texture(glyph_atlas_sampler, v_unit_position * uv_size + uv_position)
31+
.aaaa *
3132
frag_info.text_color;
3233
}
3334
}

impeller/entity/shaders/glyph_atlas.vert

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ void main() {
2626
gl_Position = IPPositionForGlyphPosition(
2727
frame_info.mvp, unit_position, destination_position, destination_size);
2828
v_unit_position = unit_position;
29-
v_source_position = source_position;
29+
// Pixel snap the source (sampling) start position.
30+
v_source_position = round(source_position);
3031
v_source_glyph_size = source_glyph_size;
3132
v_has_color = has_color;
3233
}

impeller/geometry/geometry_unittests.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,32 @@ TEST(GeometryTest, MatrixIsAligned) {
424424
}
425425
}
426426

427+
TEST(GeometryTest, MatrixTranslationScaleOnly) {
428+
{
429+
auto m = Matrix();
430+
bool result = m.IsTranslationScaleOnly();
431+
ASSERT_TRUE(result);
432+
}
433+
434+
{
435+
auto m = Matrix::MakeScale(Vector3(2, 3, 4));
436+
bool result = m.IsTranslationScaleOnly();
437+
ASSERT_TRUE(result);
438+
}
439+
440+
{
441+
auto m = Matrix::MakeTranslation(Vector3(2, 3, 4));
442+
bool result = m.IsTranslationScaleOnly();
443+
ASSERT_TRUE(result);
444+
}
445+
446+
{
447+
auto m = Matrix::MakeRotationZ(Degrees(10));
448+
bool result = m.IsTranslationScaleOnly();
449+
ASSERT_FALSE(result);
450+
}
451+
}
452+
427453
TEST(GeometryTest, MatrixLookAt) {
428454
{
429455
auto m = Matrix::MakeLookAt(Vector3(0, 0, -1), Vector3(0, 0, 1),

impeller/geometry/matrix.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,19 @@ struct Matrix {
322322
);
323323
}
324324

325+
/// @brief Returns true if the matrix has a scale-only basis and is
326+
/// non-projective. Note that an identity matrix meets this criteria.
327+
constexpr bool IsTranslationScaleOnly() const {
328+
return (
329+
// clang-format off
330+
m[0] != 0.0 && m[1] == 0.0 && m[2] == 0.0 && m[3] == 0.0 &&
331+
m[4] == 0.0 && m[5] != 0.0 && m[6] == 0.0 && m[7] == 0.0 &&
332+
m[8] == 0.0 && m[9] == 0.0 && m[10] != 0.0 && m[11] == 0.0 &&
333+
m[15] == 1.0
334+
// clang-format on
335+
);
336+
}
337+
325338
std::optional<MatrixDecomposition> Decompose() const;
326339

327340
constexpr bool operator==(const Matrix& m) const {

0 commit comments

Comments
 (0)