Skip to content

Commit

Permalink
[Impeller] fix incorrect origins for mesh gradient computation. (#54762)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jonahwilliams authored Aug 26, 2024
1 parent 72a64e4 commit b41ca79
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 6 deletions.
8 changes: 4 additions & 4 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -970,21 +970,21 @@ void Canvas::DrawVertices(const std::shared_ptr<VerticesGeometry>& vertices,
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
vertices->GetTextureCoordinateCoverge().value_or(cvg.value());
}
Rect translated_coverage = Rect::MakeSize(src_coverage.GetSize());
src_contents = src_paint.CreateContentsForGeometry(
Geometry::MakeRect(translated_coverage));
Geometry::MakeRect(Rect::Round(src_coverage)));

auto contents = std::make_shared<VerticesSimpleBlendContents>();
contents->SetBlendMode(blend_mode);
contents->SetAlpha(paint.color.alpha);
contents->SetGeometry(vertices);
contents->SetLazyTextureCoverage(src_coverage);
contents->SetLazyTexture(
[src_contents, translated_coverage](const ContentContext& renderer) {
[src_contents, src_coverage](const ContentContext& renderer) {
// Applying the src coverage as the coverage limit prevents the 1px
// coverage pad from adding a border that is picked up by developer
// specified UVs.
return src_contents->RenderToSnapshot(renderer, {}, translated_coverage)
return src_contents
->RenderToSnapshot(renderer, {}, Rect::Round(src_coverage))
->texture;
});
entity.SetContents(paint.WithFilters(std::move(contents)));
Expand Down
51 changes: 50 additions & 1 deletion impeller/display_list/aiks_dl_vertices_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ TEST_P(AiksTest, DrawVerticesWithInvalidIndices) {
}

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

// The vertices should form a solid red rectangle with no gaps.
// The blue rectangle drawn under them should not be visible.
TEST_P(AiksTest,
DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin) {
std::vector<SkPoint> positions_lt = {
SkPoint::Make(200, 200), //
SkPoint::Make(250, 200), //
SkPoint::Make(200, 250), //
SkPoint::Make(250, 250), //
};

auto vertices = flutter::DlVertices::Make(
flutter::DlVertexMode::kTriangleStrip, positions_lt.size(),
positions_lt.data(),
/*texture_coordinates=*/positions_lt.data(), /*colors=*/nullptr,
/*index_count=*/0,
/*indices=*/nullptr);

flutter::DisplayListBuilder builder;
flutter::DlPaint paint;
flutter::DlPaint rect_paint;
rect_paint.setColor(DlColor::kBlue());

auto runtime_stages =
OpenAssetAsRuntimeStage("runtime_stage_position.frag.iplr");

auto runtime_stage =
runtime_stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())];
ASSERT_TRUE(runtime_stage);

auto runtime_effect = DlRuntimeEffect::MakeImpeller(runtime_stage);
auto rect_data = std::vector<Rect>{Rect::MakeLTRB(200, 200, 250, 250)};

auto uniform_data = std::make_shared<std::vector<uint8_t>>();
uniform_data->resize(rect_data.size() * sizeof(Rect));
memcpy(uniform_data->data(), rect_data.data(), uniform_data->size());

auto color_source = flutter::DlColorSource::MakeRuntimeEffect(
runtime_effect, {}, uniform_data);

paint.setColorSource(color_source);

builder.Scale(GetContentScale().x, GetContentScale().y);
builder.DrawRect(SkRect::MakeLTRB(200, 200, 250, 250), rect_paint);
builder.DrawVertices(vertices, flutter::DlBlendMode::kSrcOver, paint);

ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}

} // namespace testing
} // namespace impeller
6 changes: 5 additions & 1 deletion impeller/entity/contents/vertices_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
#include "vertices_contents.h"

#include "fml/logging.h"
#include "impeller/base/validation.h"
#include "impeller/core/formats.h"
#include "impeller/entity/contents/content_context.h"
#include "impeller/entity/contents/contents.h"
#include "impeller/entity/contents/filters/blend_filter_contents.h"
#include "impeller/entity/geometry/geometry.h"
#include "impeller/entity/geometry/vertices_geometry.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/size.h"
#include "impeller/renderer/render_pass.h"

namespace impeller {
Expand Down Expand Up @@ -113,6 +113,10 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer,
} else {
texture = renderer.GetEmptyTexture();
}
if (!texture) {
VALIDATION_LOG << "Missing texture for VerticesSimpleBlendContents";
return false;
}

auto dst_sampler_descriptor = descriptor_;
dst_sampler_descriptor.width_address_mode =
Expand Down
1 change: 1 addition & 0 deletions impeller/fixtures/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ impellerc("runtime_stages") {
"ink_sparkle.frag",
"runtime_stage_example.frag",
"runtime_stage_simple.frag",
"runtime_stage_position.frag",
"gradient.frag",
"uniforms_and_sampler_1.frag",
"uniforms_and_sampler_2.frag",
Expand Down
19 changes: 19 additions & 0 deletions impeller/fixtures/runtime_stage_position.frag
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <flutter/runtime_effect.glsl>

uniform vec4 ltrb;

out vec4 frag_color;

// Output solid red if frag position is within LTRB rectangle.
void main() {
if (FlutterFragCoord().x >= ltrb.x && FlutterFragCoord().x <= ltrb.z &&
FlutterFragCoord().y >= ltrb.y && FlutterFragCoord().y <= ltrb.w) {
frag_color = vec4(1.0, 0.0, 0.0, 1.0);
} else {
frag_color = vec4(0.0);
}
}
3 changes: 3 additions & 0 deletions testing/impeller_golden_tests_output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,9 @@ impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithIndices_Vulkan.png
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Metal.png
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_OpenGLES.png
impeller_Play_AiksTest_DrawVerticesSolidColorTrianglesWithoutIndices_Vulkan.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_Metal.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_OpenGLES.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOrigin_Vulkan.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Metal.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_OpenGLES.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Vulkan.png
Expand Down

0 comments on commit b41ca79

Please sign in to comment.