Skip to content

Commit 995d0bb

Browse files
committed
[Impeller] fix incorrect origins for mesh gradient computation. (flutter#54762)
Fixes flutter/flutter#153964 Changing the origin of the rect used to render a shader could break shaders that expect to render at particular coordinates based on the input vertices. The snapshot functionality correctly handles translating a texture, so the translation was never necessary to begin with.
1 parent c9b9d57 commit 995d0bb

File tree

6 files changed

+82
-6
lines changed

6 files changed

+82
-6
lines changed

impeller/aiks/canvas.cc

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

10101009
auto contents = std::make_shared<VerticesSimpleBlendContents>();
10111010
contents->SetBlendMode(blend_mode);
10121011
contents->SetAlpha(paint.color.alpha);
10131012
contents->SetGeometry(vertices);
10141013
contents->SetLazyTextureCoverage(src_coverage);
10151014
contents->SetLazyTexture(
1016-
[src_contents, translated_coverage](const ContentContext& renderer) {
1015+
[src_contents, src_coverage](const ContentContext& renderer) {
10171016
// Applying the src coverage as the coverage limit prevents the 1px
10181017
// coverage pad from adding a border that is picked up by developer
10191018
// specified UVs.
1020-
return src_contents->RenderToSnapshot(renderer, {}, translated_coverage)
1019+
return src_contents
1020+
->RenderToSnapshot(renderer, {}, Rect::Round(src_coverage))
10211021
->texture;
10221022
});
10231023
entity.SetContents(paint.WithFilters(std::move(contents)));

impeller/display_list/aiks_dl_vertices_unittests.cc

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ TEST_P(AiksTest, DrawVerticesPremultipliesColors) {
355355
}
356356

357357
// All four vertices should form a solid red rectangle with no gaps.
358-
// The blur rectangle drawn under them should not be visible.
358+
// The blue rectangle drawn under them should not be visible.
359359
TEST_P(AiksTest, DrawVerticesTextureCoordinatesWithFragmentShader) {
360360
std::vector<SkPoint> positions_lt = {
361361
SkPoint::Make(0, 0), //
@@ -444,5 +444,54 @@ TEST_P(AiksTest, DrawVerticesTextureCoordinatesWithFragmentShader) {
444444
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
445445
}
446446

447+
// The vertices should form a solid red rectangle with no gaps.
448+
// The blue rectangle drawn under them should not be visible.
449+
TEST_P(AiksTest,
450+
DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin) {
451+
std::vector<SkPoint> positions_lt = {
452+
SkPoint::Make(200, 200), //
453+
SkPoint::Make(250, 200), //
454+
SkPoint::Make(200, 250), //
455+
SkPoint::Make(250, 250), //
456+
};
457+
458+
auto vertices = flutter::DlVertices::Make(
459+
flutter::DlVertexMode::kTriangleStrip, positions_lt.size(),
460+
positions_lt.data(),
461+
/*texture_coordinates=*/positions_lt.data(), /*colors=*/nullptr,
462+
/*index_count=*/0,
463+
/*indices=*/nullptr);
464+
465+
flutter::DisplayListBuilder builder;
466+
flutter::DlPaint paint;
467+
flutter::DlPaint rect_paint;
468+
rect_paint.setColor(DlColor::kBlue());
469+
470+
auto runtime_stages =
471+
OpenAssetAsRuntimeStage("runtime_stage_position.frag.iplr");
472+
473+
auto runtime_stage =
474+
runtime_stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())];
475+
ASSERT_TRUE(runtime_stage);
476+
477+
auto runtime_effect = DlRuntimeEffect::MakeImpeller(runtime_stage);
478+
auto rect_data = std::vector<Rect>{Rect::MakeLTRB(200, 200, 250, 250)};
479+
480+
auto uniform_data = std::make_shared<std::vector<uint8_t>>();
481+
uniform_data->resize(rect_data.size() * sizeof(Rect));
482+
memcpy(uniform_data->data(), rect_data.data(), uniform_data->size());
483+
484+
auto color_source = flutter::DlColorSource::MakeRuntimeEffect(
485+
runtime_effect, {}, uniform_data);
486+
487+
paint.setColorSource(color_source);
488+
489+
builder.Scale(GetContentScale().x, GetContentScale().y);
490+
builder.DrawRect(SkRect::MakeLTRB(200, 200, 250, 250), rect_paint);
491+
builder.DrawVertices(vertices, flutter::DlBlendMode::kSrcOver, paint);
492+
493+
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
494+
}
495+
447496
} // namespace testing
448497
} // namespace impeller

impeller/entity/contents/vertices_contents.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
#include "vertices_contents.h"
66

77
#include "fml/logging.h"
8+
#include "impeller/base/validation.h"
89
#include "impeller/core/formats.h"
910
#include "impeller/entity/contents/content_context.h"
1011
#include "impeller/entity/contents/contents.h"
1112
#include "impeller/entity/contents/filters/blend_filter_contents.h"
1213
#include "impeller/entity/geometry/geometry.h"
1314
#include "impeller/entity/geometry/vertices_geometry.h"
1415
#include "impeller/geometry/color.h"
15-
#include "impeller/geometry/size.h"
1616
#include "impeller/renderer/render_pass.h"
1717

1818
namespace impeller {
@@ -113,6 +113,10 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer,
113113
} else {
114114
texture = renderer.GetEmptyTexture();
115115
}
116+
if (!texture) {
117+
VALIDATION_LOG << "Missing texture for VerticesSimpleBlendContents";
118+
return false;
119+
}
116120

117121
auto dst_sampler_descriptor = descriptor_;
118122
dst_sampler_descriptor.width_address_mode =

impeller/fixtures/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ impellerc("runtime_stages") {
7070
"ink_sparkle.frag",
7171
"runtime_stage_example.frag",
7272
"runtime_stage_simple.frag",
73+
"runtime_stage_position.frag",
7374
"gradient.frag",
7475
"uniforms_and_sampler_1.frag",
7576
"uniforms_and_sampler_2.frag",
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
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+
#include <flutter/runtime_effect.glsl>
6+
7+
uniform vec4 ltrb;
8+
9+
out vec4 frag_color;
10+
11+
// Output solid red if frag position is within LTRB rectangle.
12+
void main() {
13+
if (FlutterFragCoord().x >= ltrb.x && FlutterFragCoord().x <= ltrb.z &&
14+
FlutterFragCoord().y >= ltrb.y && FlutterFragCoord().y <= ltrb.w) {
15+
frag_color = vec4(1.0, 0.0, 0.0, 1.0);
16+
} else {
17+
frag_color = vec4(0.0);
18+
}
19+
}

testing/impeller_golden_tests_output.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,9 @@ impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithIndices_Vulkan.png
593593
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Metal.png
594594
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_OpenGLES.png
595595
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Vulkan.png
596+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_Metal.png
597+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_OpenGLES.png
598+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_Vulkan.png
596599
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Metal.png
597600
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_OpenGLES.png
598601
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Vulkan.png

0 commit comments

Comments
 (0)