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

Commit f48f3b6

Browse files
author
Jonah Williams
authored
[Impeller] only use porter duff or vertices.uber for drawVertices. (#52345)
Simplifcation that will allow us to remove the special case position_color.vert and vertices.frag shaders, not done here because drawAtlas also needs to be updated. This also fixes sparkle-party like rendering bugs where we incorrectly relied on a color filter. The problem with doing advanced blends between the vertices with a color filters was three fold: 1. We would render incorrectly when vertices overlapped. 2. We had to disable MSAA to remove artifacts 3. The sub render pass was slow. Below is an example of the incorrect rendering caused by overlapping vertices on the sparkle party app. ## Skia (Advanced blend) ![flutter_02](https://github.com/flutter/engine/assets/8975114/36cdd590-5b1e-4b9e-ae4d-cc187ba40bb6) ## Impeller (ToT) ![flutter_01](https://github.com/flutter/engine/assets/8975114/20ea5f42-6fb3-4f5e-b332-516ee904b8f6) ## Impeller w/ patch ![flutter_03](https://github.com/flutter/engine/assets/8975114/e88ccb6b-56ba-42de-8542-bbd94ba0fe44) Fixes flutter/flutter#131345
1 parent b9b6a91 commit f48f3b6

10 files changed

Lines changed: 94 additions & 535 deletions

File tree

ci/licenses_golden/excluded_files

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,6 @@
152152
../../../flutter/impeller/entity/contents/host_buffer_unittests.cc
153153
../../../flutter/impeller/entity/contents/test
154154
../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc
155-
../../../flutter/impeller/entity/contents/vertices_contents_unittests.cc
156155
../../../flutter/impeller/entity/entity_pass_target_unittests.cc
157156
../../../flutter/impeller/entity/entity_pass_unittests.cc
158157
../../../flutter/impeller/entity/entity_unittests.cc

impeller/aiks/canvas.cc

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "impeller/entity/contents/atlas_contents.h"
1717
#include "impeller/entity/contents/clip_contents.h"
1818
#include "impeller/entity/contents/color_source_contents.h"
19+
#include "impeller/entity/contents/content_context.h"
1920
#include "impeller/entity/contents/filters/filter_contents.h"
2021
#include "impeller/entity/contents/solid_rrect_blur_contents.h"
2122
#include "impeller/entity/contents/text_contents.h"
@@ -933,8 +934,19 @@ void Canvas::DrawVertices(const std::shared_ptr<VerticesGeometry>& vertices,
933934
return;
934935
}
935936

936-
// If there is are per-vertex colors, an image, and the blend mode
937-
// is simple we can draw without a sub-renderpass.
937+
// If the blend mode is destination don't bother to bind or create a texture.
938+
if (blend_mode == BlendMode::kDestination) {
939+
auto contents = std::make_shared<VerticesSimpleBlendContents>();
940+
contents->SetBlendMode(blend_mode);
941+
contents->SetAlpha(paint.color.alpha);
942+
contents->SetGeometry(vertices);
943+
entity.SetContents(paint.WithFilters(std::move(contents)));
944+
AddRenderEntityToCurrentPass(std::move(entity));
945+
return;
946+
}
947+
948+
// If there is a texture, use this directly. Otherwise render the color
949+
// source to a texture.
938950
if (std::optional<ImageData> maybe_image_data =
939951
GetImageColorSourceData(paint.color_source)) {
940952
const ImageData& image_data = maybe_image_data.value();
@@ -956,35 +968,35 @@ void Canvas::DrawVertices(const std::shared_ptr<VerticesGeometry>& vertices,
956968

957969
std::shared_ptr<Contents> src_contents =
958970
src_paint.CreateContentsForGeometry(vertices);
959-
if (vertices->HasTextureCoordinates()) {
960-
// If the color source has an intrinsic size, then we use that to
961-
// create the src contents as a simplification. Otherwise we use
962-
// the extent of the texture coordinates to determine how large
963-
// the src contents should be. If neither has a value we fall back
964-
// to using the geometry coverage data.
965-
Rect src_coverage;
966-
auto size = src_contents->GetColorSourceSize();
967-
if (size.has_value()) {
968-
src_coverage = Rect::MakeXYWH(0, 0, size->width, size->height);
969-
} else {
970-
auto cvg = vertices->GetCoverage(Matrix{});
971-
FML_CHECK(cvg.has_value());
972-
src_coverage =
973-
// Covered by FML_CHECK.
974-
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
975-
vertices->GetTextureCoordinateCoverge().value_or(cvg.value());
976-
}
977-
src_contents =
978-
src_paint.CreateContentsForGeometry(Geometry::MakeRect(src_coverage));
971+
972+
// If the color source has an intrinsic size, then we use that to
973+
// create the src contents as a simplification. Otherwise we use
974+
// the extent of the texture coordinates to determine how large
975+
// the src contents should be. If neither has a value we fall back
976+
// to using the geometry coverage data.
977+
Rect src_coverage;
978+
auto size = src_contents->GetColorSourceSize();
979+
if (size.has_value()) {
980+
src_coverage = Rect::MakeXYWH(0, 0, size->width, size->height);
981+
} else {
982+
auto cvg = vertices->GetCoverage(Matrix{});
983+
FML_CHECK(cvg.has_value());
984+
src_coverage =
985+
// Covered by FML_CHECK.
986+
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
987+
vertices->GetTextureCoordinateCoverge().value_or(cvg.value());
979988
}
989+
src_contents =
990+
src_paint.CreateContentsForGeometry(Geometry::MakeRect(src_coverage));
980991

981-
auto contents = std::make_shared<VerticesContents>();
982-
contents->SetAlpha(paint.color.alpha);
992+
auto contents = std::make_shared<VerticesSimpleBlendContents>();
983993
contents->SetBlendMode(blend_mode);
994+
contents->SetAlpha(paint.color.alpha);
984995
contents->SetGeometry(vertices);
985-
contents->SetSourceContents(std::move(src_contents));
996+
contents->SetLazyTexture([src_contents](const ContentContext& renderer) {
997+
return src_contents->RenderToSnapshot(renderer, {})->texture;
998+
});
986999
entity.SetContents(paint.WithFilters(std::move(contents)));
987-
9881000
AddRenderEntityToCurrentPass(std::move(entity));
9891001
}
9901002

impeller/entity/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,6 @@ impeller_component("entity_unittests") {
262262
"contents/filters/inputs/filter_input_unittests.cc",
263263
"contents/host_buffer_unittests.cc",
264264
"contents/tiled_texture_contents_unittests.cc",
265-
"contents/vertices_contents_unittests.cc",
266265
"entity_pass_target_unittests.cc",
267266
"entity_pass_unittests.cc",
268267
"entity_playground.cc",

impeller/entity/contents/content_context.cc

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "impeller/base/strings.h"
1212
#include "impeller/base/validation.h"
1313
#include "impeller/core/formats.h"
14+
#include "impeller/core/texture_descriptor.h"
1415
#include "impeller/entity/contents/framebuffer_blend_contents.h"
1516
#include "impeller/entity/entity.h"
1617
#include "impeller/entity/render_target_cache.h"
@@ -255,6 +256,18 @@ ContentContext::ContentContext(
255256
return;
256257
}
257258

259+
{
260+
TextureDescriptor desc;
261+
desc.storage_mode = StorageMode::kHostVisible;
262+
desc.format = PixelFormat::kR8G8B8A8UNormInt;
263+
desc.size = ISize{1, 1};
264+
empty_texture_ = GetContext()->GetResourceAllocator()->CreateTexture(desc);
265+
auto data = Color::BlackTransparent().ToR8G8B8A8();
266+
if (!empty_texture_->SetContents(data.data(), 4)) {
267+
VALIDATION_LOG << "Failed to create empty texture.";
268+
}
269+
}
270+
258271
auto options = ContentContextOptions{
259272
.sample_count = SampleCount::kCount4,
260273
.color_attachment_pixel_format =
@@ -448,6 +461,10 @@ bool ContentContext::IsValid() const {
448461
return is_valid_;
449462
}
450463

464+
std::shared_ptr<Texture> ContentContext::GetEmptyTexture() const {
465+
return empty_texture_;
466+
}
467+
451468
fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
452469
std::string_view label,
453470
ISize texture_size,
@@ -567,22 +584,6 @@ void ContentContext::ClearCachedRuntimeEffectPipeline(
567584
void ContentContext::InitializeCommonlyUsedShadersIfNeeded() const {
568585
TRACE_EVENT0("flutter", "InitializeCommonlyUsedShadersIfNeeded");
569586
GetContext()->InitializeCommonlyUsedShadersIfNeeded();
570-
571-
// On ARM devices, the initial usage of vkCmdCopyBufferToImage has been
572-
// observed to take 10s of ms as an internal shader is compiled to perform
573-
// the operation. Similarly, the initial render pass can also take 10s of ms
574-
// for a similar reason. Because the context object is initialized far
575-
// before the first frame, create a trivial texture and render pass to force
576-
// the driver to compiler these shaders before the frame begins.
577-
TextureDescriptor desc;
578-
desc.size = {1, 1};
579-
desc.storage_mode = StorageMode::kHostVisible;
580-
desc.format = PixelFormat::kR8G8B8A8UNormInt;
581-
auto texture = GetContext()->GetResourceAllocator()->CreateTexture(desc);
582-
uint32_t color = 0;
583-
if (!texture->SetContents(reinterpret_cast<uint8_t*>(&color), 4u)) {
584-
VALIDATION_LOG << "Failed to set bootstrap texture.";
585-
}
586587
}
587588

588589
} // namespace impeller

impeller/entity/contents/content_context.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,10 @@ class ContentContext {
714714
return GetPipeline(vertices_uber_shader_, opts);
715715
}
716716

717+
// An empty 1x1 texture for binding drawVertices/drawAtlas or other cases
718+
// that don't always have a texture (due to blending).
719+
std::shared_ptr<Texture> GetEmptyTexture() const;
720+
717721
std::shared_ptr<Context> GetContext() const;
718722

719723
const Capabilities& GetDeviceCapabilities() const;
@@ -1034,6 +1038,7 @@ class ContentContext {
10341038
#endif // IMPELLER_ENABLE_3D
10351039
std::shared_ptr<RenderTargetAllocator> render_target_cache_;
10361040
std::shared_ptr<HostBuffer> host_buffer_;
1041+
std::shared_ptr<Texture> empty_texture_;
10371042
bool wireframe_ = false;
10381043

10391044
ContentContext(const ContentContext&) = delete;

0 commit comments

Comments
 (0)