Skip to content

Commit

Permalink
[CP][Impeller] Switch to nearest sampling for the text atlas (#39180)
Browse files Browse the repository at this point in the history
* [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>
  • Loading branch information
zanderso and bdero authored Jan 31, 2023
1 parent 2a0b3ad commit 513ac2e
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 9 deletions.
20 changes: 15 additions & 5 deletions impeller/entity/contents/text_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,18 @@ static bool CommonRender(
VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info));

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

typename FS::FragInfo frag_info;
Expand Down Expand Up @@ -146,10 +156,10 @@ static bool CommonRender(
for (const auto& point : unit_points) {
typename VS::PerVertexData vtx;
vtx.unit_position = point;
vtx.destination_position = offset_glyph_position + Point(0.5, 0.5);
vtx.destination_position = offset_glyph_position;
vtx.destination_size = Point(glyph_position.glyph.bounds.size);
vtx.source_position = atlas_position + Point(0.5, 0.5);
vtx.source_glyph_size = atlas_glyph_size - Point(1.0, 1.0);
vtx.source_position = atlas_position;
vtx.source_glyph_size = atlas_glyph_size;
if constexpr (std::is_same_v<TPipeline, GlyphAtlasPipeline>) {
vtx.has_color =
glyph_position.glyph.type == Glyph::Type::kBitmap ? 1.0 : 0.0;
Expand Down
7 changes: 4 additions & 3 deletions impeller/entity/shaders/glyph_atlas.frag
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ out vec4 frag_color;

void main() {
vec2 uv_size = v_source_glyph_size / frag_info.atlas_size;
vec2 offset = v_source_position / frag_info.atlas_size;
vec2 uv_position = v_source_position / frag_info.atlas_size;
if (v_has_color == 1.0) {
frag_color =
texture(glyph_atlas_sampler, v_unit_position * uv_size + offset);
texture(glyph_atlas_sampler, v_unit_position * uv_size + uv_position);
} else {
frag_color =
texture(glyph_atlas_sampler, v_unit_position * uv_size + offset).aaaa *
texture(glyph_atlas_sampler, v_unit_position * uv_size + uv_position)
.aaaa *
frag_info.text_color;
}
}
3 changes: 2 additions & 1 deletion impeller/entity/shaders/glyph_atlas.vert
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ void main() {
gl_Position = IPPositionForGlyphPosition(
frame_info.mvp, unit_position, destination_position, destination_size);
v_unit_position = unit_position;
v_source_position = source_position;
// Pixel snap the source (sampling) start position.
v_source_position = round(source_position);
v_source_glyph_size = source_glyph_size;
v_has_color = has_color;
}
26 changes: 26 additions & 0 deletions impeller/geometry/geometry_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,32 @@ TEST(GeometryTest, MatrixIsAligned) {
}
}

TEST(GeometryTest, MatrixTranslationScaleOnly) {
{
auto m = Matrix();
bool result = m.IsTranslationScaleOnly();
ASSERT_TRUE(result);
}

{
auto m = Matrix::MakeScale(Vector3(2, 3, 4));
bool result = m.IsTranslationScaleOnly();
ASSERT_TRUE(result);
}

{
auto m = Matrix::MakeTranslation(Vector3(2, 3, 4));
bool result = m.IsTranslationScaleOnly();
ASSERT_TRUE(result);
}

{
auto m = Matrix::MakeRotationZ(Degrees(10));
bool result = m.IsTranslationScaleOnly();
ASSERT_FALSE(result);
}
}

TEST(GeometryTest, MatrixLookAt) {
{
auto m = Matrix::MakeLookAt(Vector3(0, 0, -1), Vector3(0, 0, 1),
Expand Down
13 changes: 13 additions & 0 deletions impeller/geometry/matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,19 @@ struct Matrix {
);
}

/// @brief Returns true if the matrix has a scale-only basis and is
/// non-projective. Note that an identity matrix meets this criteria.
constexpr bool IsTranslationScaleOnly() const {
return (
// clang-format off
m[0] != 0.0 && m[1] == 0.0 && m[2] == 0.0 && m[3] == 0.0 &&
m[4] == 0.0 && m[5] != 0.0 && m[6] == 0.0 && m[7] == 0.0 &&
m[8] == 0.0 && m[9] == 0.0 && m[10] != 0.0 && m[11] == 0.0 &&
m[15] == 1.0
// clang-format on
);
}

std::optional<MatrixDecomposition> Decompose() const;

constexpr bool operator==(const Matrix& m) const {
Expand Down

0 comments on commit 513ac2e

Please sign in to comment.