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

Commit b815d21

Browse files
[Impeller] fix incorrect padding/translation in drawVertices with texture coordinates. (#53746)
Fixes flutter/flutter#151355 The coverage pad introduced by renderToSnapshot is being picked up by drawVertices with texture coordinates. The pad is showing up in the final results, which can appear to be a gap between elements. Additionally: if the coverage computed from the texture coordinates does not include the origin, then make sure the coverage used for the snapshot is translated onscreen.
1 parent 90cc72c commit b815d21

File tree

7 files changed

+124
-7
lines changed

7 files changed

+124
-7
lines changed

impeller/aiks/canvas.cc

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,16 +1003,23 @@ void Canvas::DrawVertices(const std::shared_ptr<VerticesGeometry>& vertices,
10031003
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
10041004
vertices->GetTextureCoordinateCoverge().value_or(cvg.value());
10051005
}
1006-
src_contents =
1007-
src_paint.CreateContentsForGeometry(Geometry::MakeRect(src_coverage));
1006+
Rect translated_coverage = Rect::MakeSize(src_coverage.GetSize());
1007+
src_contents = src_paint.CreateContentsForGeometry(
1008+
Geometry::MakeRect(translated_coverage));
10081009

10091010
auto contents = std::make_shared<VerticesSimpleBlendContents>();
10101011
contents->SetBlendMode(blend_mode);
10111012
contents->SetAlpha(paint.color.alpha);
10121013
contents->SetGeometry(vertices);
1013-
contents->SetLazyTexture([src_contents](const ContentContext& renderer) {
1014-
return src_contents->RenderToSnapshot(renderer, {})->texture;
1015-
});
1014+
contents->SetLazyTextureCoverage(src_coverage);
1015+
contents->SetLazyTexture(
1016+
[src_contents, translated_coverage](const ContentContext& renderer) {
1017+
// Applying the src coverage as the coverage limit prevents the 1px
1018+
// coverage pad from adding a border that is picked up by developer
1019+
// specified UVs.
1020+
return src_contents->RenderToSnapshot(renderer, {}, translated_coverage)
1021+
->texture;
1022+
});
10161023
entity.SetContents(paint.WithFilters(std::move(contents)));
10171024
AddRenderEntityToCurrentPass(std::move(entity));
10181025
}

impeller/display_list/aiks_dl_vertices_unittests.cc

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,5 +354,95 @@ TEST_P(AiksTest, DrawVerticesPremultipliesColors) {
354354
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
355355
}
356356

357+
// All four vertices should form a solid red rectangle with no gaps.
358+
// The blur rectangle drawn under them should not be visible.
359+
TEST_P(AiksTest, DrawVerticesTextureCoordinatesWithFragmentShader) {
360+
std::vector<SkPoint> positions_lt = {
361+
SkPoint::Make(0, 0), //
362+
SkPoint::Make(50, 0), //
363+
SkPoint::Make(0, 50), //
364+
SkPoint::Make(50, 50), //
365+
};
366+
367+
auto vertices_lt = flutter::DlVertices::Make(
368+
flutter::DlVertexMode::kTriangleStrip, positions_lt.size(),
369+
positions_lt.data(),
370+
/*texture_coordinates=*/positions_lt.data(), /*colors=*/nullptr,
371+
/*index_count=*/0,
372+
/*indices=*/nullptr);
373+
374+
std::vector<SkPoint> positions_rt = {
375+
SkPoint::Make(50, 0), //
376+
SkPoint::Make(100, 0), //
377+
SkPoint::Make(50, 50), //
378+
SkPoint::Make(100, 50), //
379+
};
380+
381+
auto vertices_rt = flutter::DlVertices::Make(
382+
flutter::DlVertexMode::kTriangleStrip, positions_rt.size(),
383+
positions_rt.data(),
384+
/*texture_coordinates=*/positions_rt.data(), /*colors=*/nullptr,
385+
/*index_count=*/0,
386+
/*indices=*/nullptr);
387+
388+
std::vector<SkPoint> positions_lb = {
389+
SkPoint::Make(0, 50), //
390+
SkPoint::Make(50, 50), //
391+
SkPoint::Make(0, 100), //
392+
SkPoint::Make(50, 100), //
393+
};
394+
395+
auto vertices_lb = flutter::DlVertices::Make(
396+
flutter::DlVertexMode::kTriangleStrip, positions_lb.size(),
397+
positions_lb.data(),
398+
/*texture_coordinates=*/positions_lb.data(), /*colors=*/nullptr,
399+
/*index_count=*/0,
400+
/*indices=*/nullptr);
401+
402+
std::vector<SkPoint> positions_rb = {
403+
SkPoint::Make(50, 50), //
404+
SkPoint::Make(100, 50), //
405+
SkPoint::Make(50, 100), //
406+
SkPoint::Make(100, 100), //
407+
};
408+
409+
auto vertices_rb = flutter::DlVertices::Make(
410+
flutter::DlVertexMode::kTriangleStrip, positions_rb.size(),
411+
positions_rb.data(),
412+
/*texture_coordinates=*/positions_rb.data(), /*colors=*/nullptr,
413+
/*index_count=*/0,
414+
/*indices=*/nullptr);
415+
416+
flutter::DisplayListBuilder builder;
417+
flutter::DlPaint paint;
418+
flutter::DlPaint rect_paint;
419+
rect_paint.setColor(DlColor::kBlue());
420+
421+
auto runtime_stages =
422+
OpenAssetAsRuntimeStage("runtime_stage_simple.frag.iplr");
423+
424+
auto runtime_stage =
425+
runtime_stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())];
426+
ASSERT_TRUE(runtime_stage);
427+
428+
auto runtime_effect = DlRuntimeEffect::MakeImpeller(runtime_stage);
429+
auto uniform_data = std::make_shared<std::vector<uint8_t>>();
430+
auto color_source = flutter::DlColorSource::MakeRuntimeEffect(
431+
runtime_effect, {}, uniform_data);
432+
433+
paint.setColorSource(color_source);
434+
435+
builder.Scale(GetContentScale().x, GetContentScale().y);
436+
builder.Save();
437+
builder.DrawRect(SkRect::MakeLTRB(0, 0, 100, 100), rect_paint);
438+
builder.DrawVertices(vertices_lt, flutter::DlBlendMode::kSrcOver, paint);
439+
builder.DrawVertices(vertices_rt, flutter::DlBlendMode::kSrcOver, paint);
440+
builder.DrawVertices(vertices_lb, flutter::DlBlendMode::kSrcOver, paint);
441+
builder.DrawVertices(vertices_rb, flutter::DlBlendMode::kSrcOver, paint);
442+
builder.Restore();
443+
444+
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
445+
}
446+
357447
} // namespace testing
358448
} // namespace impeller

impeller/entity/contents/vertices_contents.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ void VerticesSimpleBlendContents::SetLazyTexture(
8989
lazy_texture_ = lazy_texture;
9090
}
9191

92+
void VerticesSimpleBlendContents::SetLazyTextureCoverage(Rect rect) {
93+
lazy_texture_coverage_ = rect;
94+
}
95+
9296
bool VerticesSimpleBlendContents::Render(const ContentContext& renderer,
9397
const Entity& entity,
9498
RenderPass& pass) const {
@@ -123,8 +127,8 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer,
123127
dst_sampler_descriptor);
124128

125129
GeometryResult geometry_result = geometry_->GetPositionUVColorBuffer(
126-
(!!texture) ? Rect::MakeSize(texture->GetSize())
127-
: Rect::MakeSize(ISize{1, 1}),
130+
lazy_texture_coverage_.has_value() ? lazy_texture_coverage_.value()
131+
: Rect::MakeSize(texture->GetSize()),
128132
inverse_matrix_, renderer, entity, pass);
129133
if (geometry_result.vertex_buffer.vertex_count == 0) {
130134
return true;

impeller/entity/contents/vertices_contents.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ class VerticesSimpleBlendContents final : public Contents {
4242

4343
void SetEffectTransform(Matrix transform);
4444

45+
void SetLazyTextureCoverage(Rect rect);
46+
4547
// |Contents|
4648
std::optional<Rect> GetCoverage(const Entity& entity) const override;
4749

@@ -59,6 +61,7 @@ class VerticesSimpleBlendContents final : public Contents {
5961
Entity::TileMode tile_mode_x_ = Entity::TileMode::kClamp;
6062
Entity::TileMode tile_mode_y_ = Entity::TileMode::kClamp;
6163
Matrix inverse_matrix_ = {};
64+
std::optional<Rect> lazy_texture_coverage_;
6265
LazyTexture lazy_texture_;
6366

6467
VerticesSimpleBlendContents(const VerticesSimpleBlendContents&) = delete;

impeller/fixtures/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ impellerc("runtime_stages") {
6969
shaders = [
7070
"ink_sparkle.frag",
7171
"runtime_stage_example.frag",
72+
"runtime_stage_simple.frag",
7273
"gradient.frag",
7374
"uniforms_and_sampler_1.frag",
7475
"uniforms_and_sampler_2.frag",
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
out vec4 frag_color;
6+
7+
void main() {
8+
frag_color = vec4(1.0, 0.0, 0.0, 1.0);
9+
}

testing/impeller_golden_tests_output.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,9 @@ impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithIndices_Vulkan.png
590590
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Metal.png
591591
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_OpenGLES.png
592592
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Vulkan.png
593+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Metal.png
594+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_OpenGLES.png
595+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Vulkan.png
593596
impeller_Play_AiksTest_EmptySaveLayerIgnoresPaint_Metal.png
594597
impeller_Play_AiksTest_EmptySaveLayerIgnoresPaint_OpenGLES.png
595598
impeller_Play_AiksTest_EmptySaveLayerIgnoresPaint_Vulkan.png

0 commit comments

Comments
 (0)