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

Commit b41ca79

Browse files
author
Jonah Williams
authored
[Impeller] fix incorrect origins for mesh gradient computation. (#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 72a64e4 commit b41ca79

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
@@ -970,21 +970,21 @@ void Canvas::DrawVertices(const std::shared_ptr<VerticesGeometry>& vertices,
970970
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
971971
vertices->GetTextureCoordinateCoverge().value_or(cvg.value());
972972
}
973-
Rect translated_coverage = Rect::MakeSize(src_coverage.GetSize());
974973
src_contents = src_paint.CreateContentsForGeometry(
975-
Geometry::MakeRect(translated_coverage));
974+
Geometry::MakeRect(Rect::Round(src_coverage)));
976975

977976
auto contents = std::make_shared<VerticesSimpleBlendContents>();
978977
contents->SetBlendMode(blend_mode);
979978
contents->SetAlpha(paint.color.alpha);
980979
contents->SetGeometry(vertices);
981980
contents->SetLazyTextureCoverage(src_coverage);
982981
contents->SetLazyTexture(
983-
[src_contents, translated_coverage](const ContentContext& renderer) {
982+
[src_contents, src_coverage](const ContentContext& renderer) {
984983
// Applying the src coverage as the coverage limit prevents the 1px
985984
// coverage pad from adding a border that is picked up by developer
986985
// specified UVs.
987-
return src_contents->RenderToSnapshot(renderer, {}, translated_coverage)
986+
return src_contents
987+
->RenderToSnapshot(renderer, {}, Rect::Round(src_coverage))
988988
->texture;
989989
});
990990
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
@@ -379,7 +379,7 @@ TEST_P(AiksTest, DrawVerticesWithInvalidIndices) {
379379
}
380380

381381
// All four vertices should form a solid red rectangle with no gaps.
382-
// The blur rectangle drawn under them should not be visible.
382+
// The blue rectangle drawn under them should not be visible.
383383
TEST_P(AiksTest, DrawVerticesTextureCoordinatesWithFragmentShader) {
384384
std::vector<SkPoint> positions_lt = {
385385
SkPoint::Make(0, 0), //
@@ -468,5 +468,54 @@ TEST_P(AiksTest, DrawVerticesTextureCoordinatesWithFragmentShader) {
468468
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
469469
}
470470

471+
// The vertices should form a solid red rectangle with no gaps.
472+
// The blue rectangle drawn under them should not be visible.
473+
TEST_P(AiksTest,
474+
DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin) {
475+
std::vector<SkPoint> positions_lt = {
476+
SkPoint::Make(200, 200), //
477+
SkPoint::Make(250, 200), //
478+
SkPoint::Make(200, 250), //
479+
SkPoint::Make(250, 250), //
480+
};
481+
482+
auto vertices = flutter::DlVertices::Make(
483+
flutter::DlVertexMode::kTriangleStrip, positions_lt.size(),
484+
positions_lt.data(),
485+
/*texture_coordinates=*/positions_lt.data(), /*colors=*/nullptr,
486+
/*index_count=*/0,
487+
/*indices=*/nullptr);
488+
489+
flutter::DisplayListBuilder builder;
490+
flutter::DlPaint paint;
491+
flutter::DlPaint rect_paint;
492+
rect_paint.setColor(DlColor::kBlue());
493+
494+
auto runtime_stages =
495+
OpenAssetAsRuntimeStage("runtime_stage_position.frag.iplr");
496+
497+
auto runtime_stage =
498+
runtime_stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())];
499+
ASSERT_TRUE(runtime_stage);
500+
501+
auto runtime_effect = DlRuntimeEffect::MakeImpeller(runtime_stage);
502+
auto rect_data = std::vector<Rect>{Rect::MakeLTRB(200, 200, 250, 250)};
503+
504+
auto uniform_data = std::make_shared<std::vector<uint8_t>>();
505+
uniform_data->resize(rect_data.size() * sizeof(Rect));
506+
memcpy(uniform_data->data(), rect_data.data(), uniform_data->size());
507+
508+
auto color_source = flutter::DlColorSource::MakeRuntimeEffect(
509+
runtime_effect, {}, uniform_data);
510+
511+
paint.setColorSource(color_source);
512+
513+
builder.Scale(GetContentScale().x, GetContentScale().y);
514+
builder.DrawRect(SkRect::MakeLTRB(200, 200, 250, 250), rect_paint);
515+
builder.DrawVertices(vertices, flutter::DlBlendMode::kSrcOver, paint);
516+
517+
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
518+
}
519+
471520
} // namespace testing
472521
} // 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
@@ -91,6 +91,7 @@ impellerc("runtime_stages") {
9191
"ink_sparkle.frag",
9292
"runtime_stage_example.frag",
9393
"runtime_stage_simple.frag",
94+
"runtime_stage_position.frag",
9495
"gradient.frag",
9596
"uniforms_and_sampler_1.frag",
9697
"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
@@ -608,6 +608,9 @@ impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithIndices_Vulkan.png
608608
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Metal.png
609609
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_OpenGLES.png
610610
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Vulkan.png
611+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_Metal.png
612+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_OpenGLES.png
613+
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_Vulkan.png
611614
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Metal.png
612615
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_OpenGLES.png
613616
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Vulkan.png

0 commit comments

Comments
 (0)