Fix glyph sampling overlap (#37764)
diff --git a/impeller/compiler/shader_lib/impeller/transform.glsl b/impeller/compiler/shader_lib/impeller/transform.glsl
index f4ee335..444419d 100644
--- a/impeller/compiler/shader_lib/impeller/transform.glsl
+++ b/impeller/compiler/shader_lib/impeller/transform.glsl
@@ -13,22 +13,16 @@
// Returns the transformed gl_Position for a given glyph position in a glyph
// atlas.
-vec4 IPPositionForGlyphPosition(mat4 mvp, vec2 unit_vertex, vec2 glyph_position, vec2 glyph_size) {
- vec4 translate = mvp[0] * glyph_position.x
- + mvp[1] * glyph_position.y
- + mvp[3];
- mat4 translated_mvp = mat4(
- mvp[0],
- mvp[1],
- mvp[2],
- vec4(
- translate.xyz,
- mvp[3].w
- )
- );
- return translated_mvp *
- vec4(unit_vertex.x * glyph_size.x,
- unit_vertex.y * glyph_size.y, 0.0, 1.0);
+vec4 IPPositionForGlyphPosition(mat4 mvp,
+ vec2 unit_position,
+ vec2 glyph_position,
+ vec2 glyph_size) {
+ vec4 translate =
+ mvp[0] * glyph_position.x + mvp[1] * glyph_position.y + mvp[3];
+ mat4 translated_mvp =
+ mat4(mvp[0], mvp[1], mvp[2], vec4(translate.xyz, mvp[3].w));
+ return translated_mvp * vec4(unit_position.x * glyph_size.x,
+ unit_position.y * glyph_size.y, 0.0, 1.0);
}
#endif
diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
index d2d1c0d..1d6727d 100644
--- a/impeller/entity/contents/text_contents.cc
+++ b/impeller/entity/contents/text_contents.cc
@@ -80,6 +80,7 @@
SamplerDescriptor sampler_desc;
sampler_desc.min_filter = MinMagFilter::kLinear;
sampler_desc.mag_filter = MinMagFilter::kLinear;
+ sampler_desc.mip_filter = MipFilter::kNone;
typename FS::FragInfo frag_info;
frag_info.text_color = ToVector(color.Premultiply());
@@ -103,9 +104,9 @@
// interpolated vertex information is also used in the fragment shader to
// sample from the glyph atlas.
- const std::vector<Point> unit_vertex_points = {
- {0, 0}, {1, 0}, {0, 1}, {1, 1}};
- const std::vector<uint32_t> indices = {0, 1, 2, 1, 2, 3};
+ const std::array<Point, 4> unit_points = {Point{0, 0}, Point{1, 0},
+ Point{0, 1}, Point{1, 1}};
+ const std::array<uint32_t, 6> indices = {0, 1, 2, 1, 2, 3};
VertexBufferBuilder<typename VS::PerVertexData> vertex_builder;
@@ -127,7 +128,7 @@
for (const auto& run : frame.GetRuns()) {
auto font = run.GetFont();
- auto glyph_size_ = ISize::Ceil(font.GetMetrics().GetBoundingBox().size);
+ auto glyph_size_ = font.GetMetrics().GetBoundingBox().size;
auto glyph_size = Point{static_cast<Scalar>(glyph_size_.width),
static_cast<Scalar>(glyph_size_.height)};
auto metrics_offset =
@@ -141,22 +142,20 @@
return false;
}
- auto atlas_position =
- atlas_glyph_pos->origin + Point{1 / atlas_glyph_pos->size.width,
- 1 / atlas_glyph_pos->size.height};
+ auto atlas_position = atlas_glyph_pos->origin;
auto atlas_glyph_size =
Point{atlas_glyph_pos->size.width, atlas_glyph_pos->size.height};
auto offset_glyph_position = glyph_position.position + metrics_offset;
- for (const auto& point : unit_vertex_points) {
+ for (const auto& point : unit_points) {
typename VS::PerVertexData vtx;
- vtx.unit_vertex = point;
- vtx.glyph_position = offset_glyph_position;
- vtx.glyph_size = glyph_size;
- vtx.atlas_position = atlas_position;
- vtx.atlas_glyph_size = atlas_glyph_size;
+ vtx.unit_position = point;
+ vtx.destination_position = offset_glyph_position + Point(0.5, 0.5);
+ vtx.destination_size = glyph_size - Point(1.0, 1.0);
+ vtx.source_position = atlas_position + Point(0.5, 0.5);
+ vtx.source_glyph_size = atlas_glyph_size - Point(1.0, 1.0);
if constexpr (std::is_same_v<TPipeline, GlyphAtlasPipeline>) {
- vtx.color_glyph =
+ vtx.has_color =
glyph_position.glyph.type == Glyph::Type::kBitmap ? 1.0 : 0.0;
}
vertex_builder.AppendVertex(std::move(vtx));
diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag
index 6fa09ad..4c9175b 100644
--- a/impeller/entity/shaders/glyph_atlas.frag
+++ b/impeller/entity/shaders/glyph_atlas.frag
@@ -7,27 +7,25 @@
uniform FragInfo {
vec2 atlas_size;
vec4 text_color;
-} frag_info;
+}
+frag_info;
-in vec2 v_unit_vertex;
-in vec2 v_atlas_position;
-in vec2 v_atlas_glyph_size;
-in float v_color_glyph;
+in vec2 v_unit_position;
+in vec2 v_source_position;
+in vec2 v_source_glyph_size;
+in float v_has_color;
out vec4 frag_color;
void main() {
- vec2 scale_perspective = v_atlas_glyph_size / frag_info.atlas_size;
- vec2 offset = v_atlas_position / frag_info.atlas_size;
- if (v_color_glyph == 1.0) {
- frag_color = texture(
- glyph_atlas_sampler,
- v_unit_vertex * scale_perspective + offset
- );
+ vec2 uv_size = v_source_glyph_size / frag_info.atlas_size;
+ vec2 offset = 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);
} else {
- frag_color = texture(
- glyph_atlas_sampler,
- v_unit_vertex * scale_perspective + offset
- ).aaaa * frag_info.text_color;
+ frag_color =
+ texture(glyph_atlas_sampler, v_unit_position * uv_size + offset).aaaa *
+ frag_info.text_color;
}
}
diff --git a/impeller/entity/shaders/glyph_atlas.vert b/impeller/entity/shaders/glyph_atlas.vert
index e332d67..365fc72 100644
--- a/impeller/entity/shaders/glyph_atlas.vert
+++ b/impeller/entity/shaders/glyph_atlas.vert
@@ -6,24 +6,26 @@
uniform FrameInfo {
mat4 mvp;
-} frame_info;
+}
+frame_info;
-in vec2 unit_vertex;
-in vec2 glyph_position;
-in vec2 glyph_size;
-in vec2 atlas_position;
-in vec2 atlas_glyph_size;
-in float color_glyph;
+in vec2 unit_position;
+in vec2 destination_position;
+in vec2 destination_size;
+in vec2 source_position;
+in vec2 source_glyph_size;
+in float has_color;
-out vec2 v_unit_vertex;
-out vec2 v_atlas_position;
-out vec2 v_atlas_glyph_size;
-out float v_color_glyph;
+out vec2 v_unit_position;
+out vec2 v_source_position;
+out vec2 v_source_glyph_size;
+out float v_has_color;
void main() {
- gl_Position = IPPositionForGlyphPosition(frame_info.mvp, unit_vertex, glyph_position, glyph_size);
- v_unit_vertex = unit_vertex;
- v_atlas_position = atlas_position;
- v_atlas_glyph_size = atlas_glyph_size;
- v_color_glyph = color_glyph;
+ gl_Position = IPPositionForGlyphPosition(
+ frame_info.mvp, unit_position, destination_position, destination_size);
+ v_unit_position = unit_position;
+ v_source_position = source_position;
+ v_source_glyph_size = source_glyph_size;
+ v_has_color = has_color;
}
diff --git a/impeller/entity/shaders/glyph_atlas_sdf.frag b/impeller/entity/shaders/glyph_atlas_sdf.frag
index 38518ce..9947566 100644
--- a/impeller/entity/shaders/glyph_atlas_sdf.frag
+++ b/impeller/entity/shaders/glyph_atlas_sdf.frag
@@ -7,28 +7,35 @@
uniform FragInfo {
vec2 atlas_size;
vec4 text_color;
-} frag_info;
+}
+frag_info;
-in vec2 v_unit_vertex;
-in vec2 v_atlas_position;
-in vec2 v_atlas_glyph_size;
+in vec2 v_unit_position;
+in vec2 v_source_position;
+in vec2 v_source_glyph_size;
out vec4 frag_color;
void main() {
- vec2 scale_perspective = v_atlas_glyph_size / frag_info.atlas_size;
- vec2 offset = v_atlas_position / frag_info.atlas_size;
+ vec2 scale_perspective = v_source_glyph_size / frag_info.atlas_size;
+ vec2 offset = v_source_position / frag_info.atlas_size;
// Inspired by Metal by Example's SDF text rendering shader:
// https://github.com/metal-by-example/sample-code/blob/master/objc/12-TextRendering/TextRendering/Shaders.metal
// Outline of glyph is the isocontour with value 50%
float edge_distance = 0.5;
- // Sample the signed-distance field to find distance from this fragment to the glyph outline
- float sample_distance = texture(glyph_atlas_sampler, v_unit_vertex * scale_perspective + offset).a;
- // Use local automatic gradients to find anti-aliased anisotropic edge width, cf. Gustavson 2012
+ // Sample the signed-distance field to find distance from this fragment to the
+ // glyph outline
+ float sample_distance =
+ texture(glyph_atlas_sampler, v_unit_position * scale_perspective + offset)
+ .a;
+ // Use local automatic gradients to find anti-aliased anisotropic edge width,
+ // cf. Gustavson 2012
float edge_width = length(vec2(dFdx(sample_distance), dFdy(sample_distance)));
- // Smooth the glyph edge by interpolating across the boundary in a band with the width determined above
- float insideness = smoothstep(edge_distance - edge_width, edge_distance + edge_width, sample_distance);
+ // Smooth the glyph edge by interpolating across the boundary in a band with
+ // the width determined above
+ float insideness = smoothstep(edge_distance - edge_width,
+ edge_distance + edge_width, sample_distance);
frag_color = frag_info.text_color * insideness;
}
diff --git a/impeller/entity/shaders/glyph_atlas_sdf.vert b/impeller/entity/shaders/glyph_atlas_sdf.vert
index 8305663..cfbb2d3 100644
--- a/impeller/entity/shaders/glyph_atlas_sdf.vert
+++ b/impeller/entity/shaders/glyph_atlas_sdf.vert
@@ -6,21 +6,23 @@
uniform FrameInfo {
mat4 mvp;
-} frame_info;
+}
+frame_info;
-in vec2 unit_vertex;
-in vec2 glyph_position;
-in vec2 glyph_size;
-in vec2 atlas_position;
-in vec2 atlas_glyph_size;
+in vec2 unit_position;
+in vec2 destination_position;
+in vec2 destination_size;
+in vec2 source_position;
+in vec2 source_glyph_size;
-out vec2 v_unit_vertex;
-out vec2 v_atlas_position;
-out vec2 v_atlas_glyph_size;
+out vec2 v_unit_position;
+out vec2 v_source_position;
+out vec2 v_source_glyph_size;
void main() {
- gl_Position = IPPositionForGlyphPosition(frame_info.mvp, unit_vertex, glyph_position, glyph_size);
- v_unit_vertex = unit_vertex;
- v_atlas_position = atlas_position;
- v_atlas_glyph_size = atlas_glyph_size;
+ gl_Position = IPPositionForGlyphPosition(
+ frame_info.mvp, unit_position, destination_position, destination_size);
+ v_unit_position = unit_position;
+ v_source_position = source_position;
+ v_source_glyph_size = source_glyph_size;
}
diff --git a/impeller/fixtures/struct_def_bug.vert b/impeller/fixtures/struct_def_bug.vert
index 800dc53..2781034 100644
--- a/impeller/fixtures/struct_def_bug.vert
+++ b/impeller/fixtures/struct_def_bug.vert
@@ -10,25 +10,25 @@
in vec2 unit_vertex;
in mat4 glyph_position; // <--- Causes multiple slots to be used and is a failure.
-in vec2 glyph_size;
-in vec2 atlas_position;
-in vec2 atlas_glyph_size;
+in vec2 destination_size;
+in vec2 source_position;
+in vec2 source_glyph_size;
out vec2 v_unit_vertex;
-out vec2 v_atlas_position;
-out vec2 v_atlas_glyph_size;
+out vec2 v_source_position;
+out vec2 v_source_glyph_size;
out vec2 v_atlas_size;
out vec4 v_text_color;
void main() {
gl_Position = frame_info.mvp
* glyph_position
- * vec4(unit_vertex.x * glyph_size.x,
- unit_vertex.y * glyph_size.y, 0.0, 1.0);
+ * vec4(unit_vertex.x * destination_size.x,
+ unit_vertex.y * destination_size.y, 0.0, 1.0);
v_unit_vertex = unit_vertex;
- v_atlas_position = atlas_position;
- v_atlas_glyph_size = atlas_glyph_size;
+ v_source_position = source_position;
+ v_source_glyph_size = source_glyph_size;
v_atlas_size = frame_info.atlas_size;
v_text_color = frame_info.text_color;
}
diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
index c1b6405..1641171 100644
--- a/impeller/typographer/backends/skia/text_render_context_skia.cc
+++ b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -71,15 +71,19 @@
glyph_positions.clear();
glyph_positions.reserve(pairs.size());
+ // TODO(114563): We might be able to remove this per-glyph padding if we fix
+ // the underlying causes of the overlap.
+ constexpr auto padding = 2;
+
for (size_t i = 0; i < pairs.size(); i++) {
const auto& pair = pairs[i];
const auto glyph_size =
ISize::Ceil(pair.font.GetMetrics().GetBoundingBox().size *
pair.font.GetMetrics().scale);
SkIPoint16 location_in_atlas;
- if (!rect_packer->addRect(glyph_size.width, //
- glyph_size.height, //
- &location_in_atlas //
+ if (!rect_packer->addRect(glyph_size.width + padding, //
+ glyph_size.height + padding, //
+ &location_in_atlas //
)) {
return pairs.size() - i;
}
