From 855da7266ce5db9890da6412e47c1a7b404c2e2b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 23 Apr 2024 16:03:04 -0700 Subject: [PATCH 1/6] [Impeller] only use porter duff or vertices.uber for drawVertices. --- impeller/aiks/canvas.cc | 64 +++-- impeller/entity/BUILD.gn | 1 - impeller/entity/contents/vertices_contents.cc | 224 +++--------------- impeller/entity/contents/vertices_contents.h | 99 +------- .../contents/vertices_contents_unittests.cc | 88 ------- 5 files changed, 74 insertions(+), 402 deletions(-) delete mode 100644 impeller/entity/contents/vertices_contents_unittests.cc diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index d5ba43fe7e8c8..c88e015edf82a 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -16,6 +16,7 @@ #include "impeller/entity/contents/atlas_contents.h" #include "impeller/entity/contents/clip_contents.h" #include "impeller/entity/contents/color_source_contents.h" +#include "impeller/entity/contents/content_context.h" #include "impeller/entity/contents/filters/filter_contents.h" #include "impeller/entity/contents/solid_rrect_blur_contents.h" #include "impeller/entity/contents/text_contents.h" @@ -936,8 +937,19 @@ void Canvas::DrawVertices(const std::shared_ptr& vertices, return; } - // If there is are per-vertex colors, an image, and the blend mode - // is simple we can draw without a sub-renderpass. + // If the blend mode is destination don't bother to bind or create a texture. + if (blend_mode == BlendMode::kDestination) { + auto contents = std::make_shared(); + contents->SetBlendMode(blend_mode); + contents->SetAlpha(paint.color.alpha); + contents->SetGeometry(vertices); + entity.SetContents(paint.WithFilters(std::move(contents))); + AddRenderEntityToCurrentPass(std::move(entity)); + return; + } + + // If there is a texture, use this directly. Otherwise render the color + // source to a texture. if (std::optional maybe_image_data = GetImageColorSourceData(paint.color_source)) { const ImageData& image_data = maybe_image_data.value(); @@ -960,35 +972,35 @@ void Canvas::DrawVertices(const std::shared_ptr& vertices, std::shared_ptr src_contents = src_paint.CreateContentsForGeometry(vertices); - if (vertices->HasTextureCoordinates()) { - // If the color source has an intrinsic size, then we use that to - // create the src contents as a simplification. Otherwise we use - // the extent of the texture coordinates to determine how large - // the src contents should be. If neither has a value we fall back - // to using the geometry coverage data. - Rect src_coverage; - auto size = src_contents->GetColorSourceSize(); - if (size.has_value()) { - src_coverage = Rect::MakeXYWH(0, 0, size->width, size->height); - } else { - auto cvg = vertices->GetCoverage(Matrix{}); - FML_CHECK(cvg.has_value()); - src_coverage = - // Covered by FML_CHECK. - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - vertices->GetTextureCoordinateCoverge().value_or(cvg.value()); - } - src_contents = - src_paint.CreateContentsForGeometry(Geometry::MakeRect(src_coverage)); + + // If the color source has an intrinsic size, then we use that to + // create the src contents as a simplification. Otherwise we use + // the extent of the texture coordinates to determine how large + // the src contents should be. If neither has a value we fall back + // to using the geometry coverage data. + Rect src_coverage; + auto size = src_contents->GetColorSourceSize(); + if (size.has_value()) { + src_coverage = Rect::MakeXYWH(0, 0, size->width, size->height); + } else { + auto cvg = vertices->GetCoverage(Matrix{}); + FML_CHECK(cvg.has_value()); + src_coverage = + // Covered by FML_CHECK. + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + vertices->GetTextureCoordinateCoverge().value_or(cvg.value()); } + src_contents = + src_paint.CreateContentsForGeometry(Geometry::MakeRect(src_coverage)); - auto contents = std::make_shared(); - contents->SetAlpha(paint.color.alpha); + auto contents = std::make_shared(); contents->SetBlendMode(blend_mode); + contents->SetAlpha(paint.color.alpha); contents->SetGeometry(vertices); - contents->SetSourceContents(std::move(src_contents)); + contents->SetLazyTexture([src_contents](const ContentContext& renderer) { + return src_contents->RenderToSnapshot(renderer, {})->texture; + }); entity.SetContents(paint.WithFilters(std::move(contents))); - AddRenderEntityToCurrentPass(std::move(entity)); } diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index 61ca78ca7eca6..1149a2f7acdf8 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -267,7 +267,6 @@ impeller_component("entity_unittests") { "contents/filters/inputs/filter_input_unittests.cc", "contents/host_buffer_unittests.cc", "contents/tiled_texture_contents_unittests.cc", - "contents/vertices_contents_unittests.cc", "entity_pass_target_unittests.cc", "entity_pass_unittests.cc", "entity_playground.cc", diff --git a/impeller/entity/contents/vertices_contents.cc b/impeller/entity/contents/vertices_contents.cc index dafc7b6e9edfb..7553f9d34a4cc 100644 --- a/impeller/entity/contents/vertices_contents.cc +++ b/impeller/entity/contents/vertices_contents.cc @@ -9,11 +9,8 @@ #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/contents/filters/color_filter_contents.h" #include "impeller/entity/geometry/geometry.h" #include "impeller/entity/geometry/vertices_geometry.h" -#include "impeller/entity/position_color.vert.h" -#include "impeller/entity/vertices.frag.h" #include "impeller/geometry/color.h" #include "impeller/renderer/render_pass.h" @@ -42,190 +39,6 @@ static std::optional TileModeToAddressMode( } } // namespace -VerticesContents::VerticesContents() = default; - -VerticesContents::~VerticesContents() = default; - -std::optional VerticesContents::GetCoverage(const Entity& entity) const { - return geometry_->GetCoverage(entity.GetTransform()); -}; - -void VerticesContents::SetGeometry(std::shared_ptr geometry) { - geometry_ = std::move(geometry); -} - -void VerticesContents::SetSourceContents(std::shared_ptr contents) { - src_contents_ = std::move(contents); -} - -std::shared_ptr VerticesContents::GetGeometry() const { - return geometry_; -} - -void VerticesContents::SetAlpha(Scalar alpha) { - alpha_ = alpha; -} - -void VerticesContents::SetBlendMode(BlendMode blend_mode) { - blend_mode_ = blend_mode; -} - -const std::shared_ptr& VerticesContents::GetSourceContents() const { - return src_contents_; -} - -bool VerticesContents::Render(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const { - if (blend_mode_ == BlendMode::kClear) { - return true; - } - - std::shared_ptr src_contents = src_contents_; - src_contents->SetCoverageHint(GetCoverageHint()); - if (geometry_->HasTextureCoordinates()) { - auto contents = std::make_shared(*this); - contents->SetCoverageHint(GetCoverageHint()); - if (!geometry_->HasVertexColors()) { - contents->SetAlpha(alpha_); - return contents->Render(renderer, entity, pass); - } - src_contents = contents; - } - - auto dst_contents = std::make_shared(*this); - dst_contents->SetCoverageHint(GetCoverageHint()); - - std::shared_ptr contents; - if (blend_mode_ == BlendMode::kDestination) { - dst_contents->SetAlpha(alpha_); - contents = dst_contents; - } else { - auto color_filter_contents = ColorFilterContents::MakeBlend( - blend_mode_, {FilterInput::Make(dst_contents, false), - FilterInput::Make(src_contents, false)}); - color_filter_contents->SetAlpha(alpha_); - color_filter_contents->SetCoverageHint(GetCoverageHint()); - contents = color_filter_contents; - } - - FML_DCHECK(contents->GetCoverageHint() == GetCoverageHint()); - return contents->Render(renderer, entity, pass); -} - -//------------------------------------------------------ -// VerticesUVContents - -VerticesUVContents::VerticesUVContents(const VerticesContents& parent) - : parent_(parent) {} - -VerticesUVContents::~VerticesUVContents() {} - -std::optional VerticesUVContents::GetCoverage( - const Entity& entity) const { - return parent_.GetCoverage(entity); -} - -void VerticesUVContents::SetAlpha(Scalar alpha) { - alpha_ = alpha; -} - -bool VerticesUVContents::Render(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const { - using VS = TexturePipeline::VertexShader; - using FS = TexturePipeline::FragmentShader; - - auto src_contents = parent_.GetSourceContents(); - - auto snapshot = - src_contents->RenderToSnapshot(renderer, // renderer - entity, // entity - GetCoverageHint(), // coverage_limit - std::nullopt, // sampler_descriptor - true, // msaa_enabled - /*mip_count=*/1, - "VerticesUVContents Snapshot"); // label - if (!snapshot.has_value()) { - return false; - } - - pass.SetCommandLabel("VerticesUV"); - auto& host_buffer = renderer.GetTransientsBuffer(); - const std::shared_ptr& geometry = parent_.GetGeometry(); - - auto coverage = src_contents->GetCoverage(Entity{}); - if (!coverage.has_value()) { - return false; - } - auto geometry_result = geometry->GetPositionUVBuffer( - coverage.value(), Matrix(), renderer, entity, pass); - auto opts = OptionsFromPassAndEntity(pass, entity); - opts.primitive_type = geometry_result.type; - pass.SetPipeline(renderer.GetTexturePipeline(opts)); - pass.SetVertexBuffer(std::move(geometry_result.vertex_buffer)); - - VS::FrameInfo frame_info; - frame_info.mvp = geometry_result.transform; - frame_info.texture_sampler_y_coord_scale = - snapshot->texture->GetYCoordScale(); - VS::BindFrameInfo(pass, host_buffer.EmplaceUniform(frame_info)); - - FS::FragInfo frag_info; - frag_info.alpha = alpha_ * snapshot->opacity; - FS::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); - FS::BindTextureSampler(pass, snapshot->texture, - renderer.GetContext()->GetSamplerLibrary()->GetSampler( - snapshot->sampler_descriptor)); - - return pass.Draw().ok(); -} - -//------------------------------------------------------ -// VerticesColorContents - -VerticesColorContents::VerticesColorContents(const VerticesContents& parent) - : parent_(parent) {} - -VerticesColorContents::~VerticesColorContents() {} - -std::optional VerticesColorContents::GetCoverage( - const Entity& entity) const { - return parent_.GetCoverage(entity); -} - -void VerticesColorContents::SetAlpha(Scalar alpha) { - alpha_ = alpha; -} - -bool VerticesColorContents::Render(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const { - using VS = GeometryColorPipeline::VertexShader; - using FS = GeometryColorPipeline::FragmentShader; - - pass.SetCommandLabel("VerticesColors"); - auto& host_buffer = renderer.GetTransientsBuffer(); - const std::shared_ptr& geometry = parent_.GetGeometry(); - - auto geometry_result = - geometry->GetPositionColorBuffer(renderer, entity, pass); - auto opts = OptionsFromPassAndEntity(pass, entity); - opts.primitive_type = geometry_result.type; - pass.SetPipeline(renderer.GetGeometryColorPipeline(opts)); - pass.SetVertexBuffer(std::move(geometry_result.vertex_buffer)); - - VS::FrameInfo frame_info; - frame_info.mvp = geometry_result.transform; - VS::BindFrameInfo(pass, host_buffer.EmplaceUniform(frame_info)); - - FS::FragInfo frag_info; - frag_info.alpha = alpha_; - FS::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); - - return pass.Draw().ok(); -} - //------------------------------------------------------ // VerticesSimpleBlendContents @@ -270,15 +83,30 @@ void VerticesSimpleBlendContents::SetEffectTransform(Matrix transform) { inverse_matrix_ = transform.Invert(); } +void VerticesSimpleBlendContents::SetLazyTexture( + const LazyTexture& lazy_texture) { + lazy_texture_ = lazy_texture; +} + bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { - FML_DCHECK(texture_); + FML_DCHECK(texture_ || lazy_texture_ || + blend_mode_ == BlendMode::kDestination); BlendMode blend_mode = blend_mode_; if (!geometry_->HasVertexColors()) { blend_mode = BlendMode::kSource; } + std::shared_ptr texture; + if (blend_mode != BlendMode::kDestination) { + if (!texture_) { + texture = lazy_texture_(renderer); + } else { + texture = texture_; + } + } + auto dst_sampler_descriptor = descriptor_; dst_sampler_descriptor.width_address_mode = TileModeToAddressMode(tile_mode_x_, renderer.GetDeviceCapabilities()) @@ -292,8 +120,8 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, dst_sampler_descriptor); GeometryResult geometry_result = geometry_->GetPositionUVColorBuffer( - Rect::MakeSize(texture_->GetSize()), inverse_matrix_, renderer, entity, - pass); + (!!texture) ? Rect::MakeSize(texture->GetSize()) : Rect{}, + inverse_matrix_, renderer, entity, pass); if (geometry_result.vertex_buffer.vertex_count == 0) { return true; } @@ -313,12 +141,16 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, options.primitive_type = geometry_result.type; pass.SetPipeline(renderer.GetPorterDuffBlendPipeline(options)); - FS::BindTextureSamplerDst(pass, texture_, dst_sampler); + if (texture) { + FS::BindTextureSamplerDst(pass, texture, dst_sampler); + } VS::FrameInfo frame_info; FS::FragInfo frag_info; - frame_info.texture_sampler_y_coord_scale = texture_->GetYCoordScale(); + if (texture) { + frame_info.texture_sampler_y_coord_scale = texture->GetYCoordScale(); + } frame_info.mvp = geometry_result.transform; frag_info.output_alpha = alpha_; @@ -356,12 +188,16 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, auto options = OptionsFromPassAndEntity(pass, entity); options.primitive_type = geometry_result.type; pass.SetPipeline(renderer.GetDrawVerticesUberShader(options)); - FS::BindTextureSampler(pass, texture_, dst_sampler); + if (texture) { + FS::BindTextureSampler(pass, texture, dst_sampler); + } VS::FrameInfo frame_info; FS::FragInfo frag_info; - frame_info.texture_sampler_y_coord_scale = texture_->GetYCoordScale(); + if (texture) { + frame_info.texture_sampler_y_coord_scale = texture->GetYCoordScale(); + } frame_info.mvp = geometry_result.transform; frag_info.alpha = alpha_; frag_info.blend_mode = static_cast(blend_mode); diff --git a/impeller/entity/contents/vertices_contents.h b/impeller/entity/contents/vertices_contents.h index 01bd369c4e5a0..863b6078edecd 100644 --- a/impeller/entity/contents/vertices_contents.h +++ b/impeller/entity/contents/vertices_contents.h @@ -5,109 +5,16 @@ #ifndef FLUTTER_IMPELLER_ENTITY_CONTENTS_VERTICES_CONTENTS_H_ #define FLUTTER_IMPELLER_ENTITY_CONTENTS_VERTICES_CONTENTS_H_ -#include #include -#include -#include "flutter/fml/macros.h" #include "impeller/core/sampler_descriptor.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/entity.h" -#include "impeller/entity/geometry/geometry.h" #include "impeller/entity/geometry/vertices_geometry.h" #include "impeller/geometry/color.h" -#include "impeller/geometry/path.h" -#include "impeller/geometry/point.h" namespace impeller { -class VerticesContents final : public Contents { - public: - VerticesContents(); - - ~VerticesContents() override; - - void SetGeometry(std::shared_ptr geometry); - - void SetAlpha(Scalar alpha); - - void SetBlendMode(BlendMode blend_mode); - - void SetSourceContents(std::shared_ptr contents); - - std::shared_ptr GetGeometry() const; - - const std::shared_ptr& GetSourceContents() const; - - // |Contents| - std::optional GetCoverage(const Entity& entity) const override; - - // |Contents| - bool Render(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const override; - - private: - Scalar alpha_; - std::shared_ptr geometry_; - BlendMode blend_mode_ = BlendMode::kSource; - std::shared_ptr src_contents_; - - VerticesContents(const VerticesContents&) = delete; - - VerticesContents& operator=(const VerticesContents&) = delete; -}; - -class VerticesColorContents final : public Contents { - public: - explicit VerticesColorContents(const VerticesContents& parent); - - ~VerticesColorContents() override; - - // |Contents| - std::optional GetCoverage(const Entity& entity) const override; - - // |Contents| - bool Render(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const override; - - void SetAlpha(Scalar alpha); - - private: - const VerticesContents& parent_; - Scalar alpha_ = 1.0; - - VerticesColorContents(const VerticesColorContents&) = delete; - - VerticesColorContents& operator=(const VerticesColorContents&) = delete; -}; - -class VerticesUVContents final : public Contents { - public: - explicit VerticesUVContents(const VerticesContents& parent); - - ~VerticesUVContents() override; - - // |Contents| - std::optional GetCoverage(const Entity& entity) const override; - - // |Contents| - bool Render(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const override; - - void SetAlpha(Scalar alpha); - - private: - const VerticesContents& parent_; - Scalar alpha_ = 1.0; - - VerticesUVContents(const VerticesUVContents&) = delete; - - VerticesUVContents& operator=(const VerticesUVContents&) = delete; -}; - /// A vertices contents for (optional) per-color vertices + texture and any /// blend mode. class VerticesSimpleBlendContents final : public Contents { @@ -116,6 +23,9 @@ class VerticesSimpleBlendContents final : public Contents { ~VerticesSimpleBlendContents() override; + using LazyTexture = + std::function(const ContentContext& renderer)>; + void SetGeometry(std::shared_ptr geometry); void SetAlpha(Scalar alpha); @@ -124,6 +34,8 @@ class VerticesSimpleBlendContents final : public Contents { void SetTexture(std::shared_ptr texture); + void SetLazyTexture(const LazyTexture& lazy_texture); + void SetSamplerDescriptor(SamplerDescriptor descriptor); void SetTileMode(Entity::TileMode tile_mode_x, Entity::TileMode tile_mode_y); @@ -147,6 +59,7 @@ class VerticesSimpleBlendContents final : public Contents { Entity::TileMode tile_mode_x_ = Entity::TileMode::kClamp; Entity::TileMode tile_mode_y_ = Entity::TileMode::kClamp; Matrix inverse_matrix_ = {}; + LazyTexture lazy_texture_; VerticesSimpleBlendContents(const VerticesSimpleBlendContents&) = delete; diff --git a/impeller/entity/contents/vertices_contents_unittests.cc b/impeller/entity/contents/vertices_contents_unittests.cc deleted file mode 100644 index 86b01643019c5..0000000000000 --- a/impeller/entity/contents/vertices_contents_unittests.cc +++ /dev/null @@ -1,88 +0,0 @@ -// 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 -#include - -#include "gtest/gtest.h" - -#include "impeller/entity/contents/contents.h" -#include "impeller/entity/contents/solid_color_contents.h" -#include "impeller/entity/contents/test/contents_test_helpers.h" -#include "impeller/entity/contents/test/recording_render_pass.h" -#include "impeller/entity/contents/vertices_contents.h" -#include "impeller/entity/entity.h" -#include "impeller/entity/entity_playground.h" -#include "impeller/entity/vertices.frag.h" -#include "impeller/geometry/path_builder.h" -#include "impeller/renderer/render_target.h" - -namespace impeller { -namespace testing { - -using EntityTest = EntityPlayground; - -std::shared_ptr CreateColorVertices( - const std::vector& vertices, - const std::vector& colors) { - auto bounds = Rect::MakePointBounds(vertices.begin(), vertices.end()); - std::vector indices = {}; - indices.reserve(vertices.size()); - for (auto i = 0u; i < vertices.size(); i++) { - indices.emplace_back(i); - } - std::vector texture_coordinates = {}; - - return std::make_shared( - vertices, indices, texture_coordinates, colors, - bounds.value_or(Rect::MakeLTRB(0, 0, 0, 0)), - VerticesGeometry::VertexMode::kTriangles); -} - -// Verifies that the destination blend fast path still sets an alpha value. -TEST_P(EntityTest, RendersDstPerColorWithAlpha) { - using FS = GeometryColorPipeline::FragmentShader; - - auto contents = std::make_shared(); - auto vertices = CreateColorVertices( - {{0, 0}, {100, 0}, {0, 100}, {100, 0}, {0, 100}, {100, 100}}, - {Color::Red(), Color::Red(), Color::Red(), Color::Red(), Color::Red(), - Color::Red()}); - auto src_contents = SolidColorContents::Make( - PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 100, 100)).TakePath(), - Color::Red()); - - contents->SetGeometry(vertices); - contents->SetAlpha(0.5); - contents->SetBlendMode(BlendMode::kDestination); - contents->SetSourceContents(std::move(src_contents)); - - auto content_context = GetContentContext(); - auto buffer = content_context->GetContext()->CreateCommandBuffer(); - auto render_target = - GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA( - *content_context->GetContext(), {100, 100}, - /*mip_count=*/1); - auto render_pass = buffer->CreateRenderPass(render_target); - auto recording_pass = std::make_shared( - render_pass, GetContext(), render_target); - Entity entity; - - ASSERT_TRUE(recording_pass->GetCommands().empty()); - ASSERT_TRUE(contents->Render(*content_context, entity, *recording_pass)); - - ASSERT_TRUE(recording_pass->GetCommands().size() > 0); - const auto& cmd = recording_pass->GetCommands()[0]; - auto* frag_uniforms = GetFragInfo(cmd); - - ASSERT_TRUE(frag_uniforms); - ASSERT_EQ(frag_uniforms->alpha, 0.5); - - if (GetParam() == PlaygroundBackend::kMetal) { - recording_pass->EncodeCommands(); - } -} - -} // namespace testing -} // namespace impeller From 7c315be1ae2ccb5c8ddf9dc6e0b432acf628e861 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 23 Apr 2024 16:13:10 -0700 Subject: [PATCH 2/6] delete moar. --- impeller/entity/contents/vertices_contents.cc | 4 +- impeller/entity/geometry/vertices_geometry.cc | 107 +----------------- impeller/entity/geometry/vertices_geometry.h | 10 -- 3 files changed, 4 insertions(+), 117 deletions(-) diff --git a/impeller/entity/contents/vertices_contents.cc b/impeller/entity/contents/vertices_contents.cc index 772ca07972435..dc96c319e496e 100644 --- a/impeller/entity/contents/vertices_contents.cc +++ b/impeller/entity/contents/vertices_contents.cc @@ -12,6 +12,7 @@ #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 { @@ -120,7 +121,8 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, dst_sampler_descriptor); GeometryResult geometry_result = geometry_->GetPositionUVColorBuffer( - (!!texture) ? Rect::MakeSize(texture->GetSize()) : Rect{}, + (!!texture) ? Rect::MakeSize(texture->GetSize()) + : Rect::MakeSize(ISize{1, 1}), inverse_matrix_, renderer, entity, pass); if (geometry_result.vertex_buffer.vertex_count == 0) { return true; diff --git a/impeller/entity/geometry/vertices_geometry.cc b/impeller/entity/geometry/vertices_geometry.cc index b0df8e1c06e83..cb5ee35c27391 100644 --- a/impeller/entity/geometry/vertices_geometry.cc +++ b/impeller/entity/geometry/vertices_geometry.cc @@ -144,107 +144,6 @@ GeometryResult VerticesGeometry::GetPositionBuffer( }; } -GeometryResult VerticesGeometry::GetPositionColorBuffer( - const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const { - using VS = GeometryColorPipeline::VertexShader; - - auto index_count = indices_.size(); - auto vertex_count = vertices_.size(); - size_t total_vtx_bytes = vertex_count * sizeof(VS::PerVertexData); - size_t total_idx_bytes = index_count * sizeof(uint16_t); - - auto vertex_buffer = renderer.GetTransientsBuffer().Emplace( - total_vtx_bytes, alignof(VS::PerVertexData), [&](uint8_t* data) { - VS::PerVertexData* vtx_contents = - reinterpret_cast(data); - for (auto i = 0u; i < vertices_.size(); i++) { - VS::PerVertexData vertex_data = { - .position = vertices_[i], - .color = colors_[i], - }; - std::memcpy(vtx_contents++, &vertex_data, sizeof(VS::PerVertexData)); - } - }); - - BufferView index_buffer = {}; - if (index_count > 0) { - index_buffer = renderer.GetTransientsBuffer().Emplace( - indices_.data(), total_idx_bytes, alignof(uint16_t)); - } - - return GeometryResult{ - .type = GetPrimitiveType(), - .vertex_buffer = - { - .vertex_buffer = vertex_buffer, - .index_buffer = index_buffer, - .vertex_count = index_count > 0 ? index_count : vertex_count, - .index_type = - index_count > 0 ? IndexType::k16bit : IndexType::kNone, - }, - .transform = entity.GetShaderTransform(pass), - }; -} - -GeometryResult VerticesGeometry::GetPositionUVBuffer( - Rect texture_coverage, - Matrix effect_transform, - const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const { - using VS = TexturePipeline::VertexShader; - - auto index_count = indices_.size(); - auto vertex_count = vertices_.size(); - auto uv_transform = - texture_coverage.GetNormalizingTransform() * effect_transform; - auto has_texture_coordinates = HasTextureCoordinates(); - - size_t total_vtx_bytes = vertices_.size() * sizeof(VS::PerVertexData); - size_t total_idx_bytes = index_count * sizeof(uint16_t); - auto vertex_buffer = renderer.GetTransientsBuffer().Emplace( - total_vtx_bytes, alignof(VS::PerVertexData), [&](uint8_t* data) { - VS::PerVertexData* vtx_contents = - reinterpret_cast(data); - for (auto i = 0u; i < vertices_.size(); i++) { - auto vertex = vertices_[i]; - auto texture_coord = - has_texture_coordinates ? texture_coordinates_[i] : vertices_[i]; - auto uv = uv_transform * texture_coord; - // From experimentation we need to clamp these values to < 1.0 or else - // there can be flickering. - VS::PerVertexData vertex_data = { - .position = vertex, - .texture_coords = - Point(std::clamp(uv.x, 0.0f, 1.0f - kEhCloseEnough), - std::clamp(uv.y, 0.0f, 1.0f - kEhCloseEnough)), - }; - std::memcpy(vtx_contents++, &vertex_data, sizeof(VS::PerVertexData)); - } - }); - - BufferView index_buffer = {}; - if (index_count > 0) { - index_buffer = renderer.GetTransientsBuffer().Emplace( - indices_.data(), total_idx_bytes, alignof(uint16_t)); - } - - return GeometryResult{ - .type = GetPrimitiveType(), - .vertex_buffer = - { - .vertex_buffer = vertex_buffer, - .index_buffer = index_buffer, - .vertex_count = index_count > 0 ? index_count : vertex_count, - .index_type = - index_count > 0 ? IndexType::k16bit : IndexType::kNone, - }, - .transform = entity.GetShaderTransform(pass), - }; -} - GeometryResult VerticesGeometry::GetPositionUVColorBuffer( Rect texture_coverage, Matrix effect_transform, @@ -269,13 +168,9 @@ GeometryResult VerticesGeometry::GetPositionUVColorBuffer( auto texture_coord = has_texture_coordinates ? texture_coordinates_[i] : vertices_[i]; auto uv = uv_transform * texture_coord; - // From experimentation we need to clamp these values to < 1.0 or else - // there can be flickering. VS::PerVertexData vertex_data = { .vertices = vertex, - .texture_coords = - Point(std::clamp(uv.x, 0.0f, 1.0f - kEhCloseEnough), - std::clamp(uv.y, 0.0f, 1.0f - kEhCloseEnough)), + .texture_coords = uv, .color = has_colors ? colors_[i] : Color::BlackTransparent(), }; std::memcpy(vtx_contents++, &vertex_data, sizeof(VS::PerVertexData)); diff --git a/impeller/entity/geometry/vertices_geometry.h b/impeller/entity/geometry/vertices_geometry.h index 397a0ebf89c1e..df125f6b2c685 100644 --- a/impeller/entity/geometry/vertices_geometry.h +++ b/impeller/entity/geometry/vertices_geometry.h @@ -27,22 +27,12 @@ class VerticesGeometry final : public Geometry { ~VerticesGeometry() = default; - GeometryResult GetPositionColorBuffer(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const; - GeometryResult GetPositionUVColorBuffer(Rect texture_coverage, Matrix effect_transform, const ContentContext& renderer, const Entity& entity, RenderPass& pass) const; - GeometryResult GetPositionUVBuffer(Rect texture_coverage, - Matrix effect_transform, - const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const; - // |Geometry| GeometryResult GetPositionBuffer(const ContentContext& renderer, const Entity& entity, From 5456954d77b37e592e49e6280b769dc6ce55fba4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 23 Apr 2024 17:03:30 -0700 Subject: [PATCH 3/6] fix licenses --- ci/licenses_golden/excluded_files | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 7fa766c0acde5..8d24606102797 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -152,7 +152,6 @@ ../../../flutter/impeller/entity/contents/host_buffer_unittests.cc ../../../flutter/impeller/entity/contents/test ../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc -../../../flutter/impeller/entity/contents/vertices_contents_unittests.cc ../../../flutter/impeller/entity/entity_pass_target_unittests.cc ../../../flutter/impeller/entity/entity_pass_unittests.cc ../../../flutter/impeller/entity/entity_unittests.cc From cb10975de265683c944642d9ccb983d7b32ac45a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 23 Apr 2024 17:55:24 -0700 Subject: [PATCH 4/6] ++ --- impeller/entity/contents/content_context.cc | 17 +++++++++++++++++ impeller/entity/contents/content_context.h | 5 +++++ impeller/entity/contents/vertices_contents.cc | 8 ++++++++ 3 files changed, 30 insertions(+) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index d22bb4cc097a2..8d806a06e33ae 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -11,6 +11,7 @@ #include "impeller/base/strings.h" #include "impeller/base/validation.h" #include "impeller/core/formats.h" +#include "impeller/core/texture_descriptor.h" #include "impeller/entity/contents/framebuffer_blend_contents.h" #include "impeller/entity/entity.h" #include "impeller/entity/render_target_cache.h" @@ -265,6 +266,18 @@ ContentContext::ContentContext( return; } + { + TextureDescriptor desc; + desc.storage_mode = StorageMode::kHostVisible; + desc.format = PixelFormat::kR8G8B8A8UNormInt; + desc.size = ISize{1, 1}; + empty_texture_ = GetContext()->GetResourceAllocator()->CreateTexture(desc); + auto data = Color::BlackTransparent().ToR8G8B8A8(); + if (!empty_texture_->SetContents(data.data(), 4)) { + VALIDATION_LOG << "Failed to create empty texture."; + } + } + auto options = ContentContextOptions{ .sample_count = SampleCount::kCount4, .color_attachment_pixel_format = @@ -467,6 +480,10 @@ bool ContentContext::IsValid() const { return is_valid_; } +std::shared_ptr ContentContext::GetEmptyTexture() const { + return empty_texture_; +} + fml::StatusOr ContentContext::MakeSubpass( std::string_view label, ISize texture_size, diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 5bbe6d0cda8f5..4510afbc9e43b 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -725,6 +725,10 @@ class ContentContext { return point_field_compute_pipelines_; } + // An empty 1x1 texture for binding drawVertices/drawAtlas or other cases + // that don't always have a texture (due to blending). + std::shared_ptr GetEmptyTexture() const; + std::shared_ptr GetContext() const; const Capabilities& GetDeviceCapabilities() const; @@ -1048,6 +1052,7 @@ class ContentContext { #endif // IMPELLER_ENABLE_3D std::shared_ptr render_target_cache_; std::shared_ptr host_buffer_; + std::shared_ptr empty_texture_; bool wireframe_ = false; ContentContext(const ContentContext&) = delete; diff --git a/impeller/entity/contents/vertices_contents.cc b/impeller/entity/contents/vertices_contents.cc index dc96c319e496e..43d95d6721308 100644 --- a/impeller/entity/contents/vertices_contents.cc +++ b/impeller/entity/contents/vertices_contents.cc @@ -145,6 +145,10 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, if (texture) { FS::BindTextureSamplerDst(pass, texture, dst_sampler); + } else { + // We need to bind something so validation layers doesn't complain. + FML_DCHECK(blend_mode == BlendMode::kDestination); + FS::BindTextureSamplerDst(pass, renderer.GetEmptyTexture(), dst_sampler); } VS::FrameInfo frame_info; @@ -193,6 +197,10 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, pass.SetPipeline(renderer.GetDrawVerticesUberShader(options)); if (texture) { FS::BindTextureSampler(pass, texture, dst_sampler); + } else { + // We need to bind something so validation layers doesn't complain. + FML_DCHECK(blend_mode == BlendMode::kDestination); + FS::BindTextureSampler(pass, renderer.GetEmptyTexture(), dst_sampler); } VS::FrameInfo frame_info; From 2af1dfc73ffb1fd13d9843168efb25da4f55f45f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 23 Apr 2024 18:33:40 -0700 Subject: [PATCH 5/6] dont need a specific shader bootstrap anymore. --- impeller/entity/contents/content_context.cc | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 8d806a06e33ae..cf3ccfce53506 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -646,22 +646,6 @@ void ContentContext::InitializeCommonlyUsedShadersIfNeeded() const { options.stencil_mode = stencil_mode; CreateIfNeeded(clip_pipelines_, options); } - - // On ARM devices, the initial usage of vkCmdCopyBufferToImage has been - // observed to take 10s of ms as an internal shader is compiled to perform - // the operation. Similarly, the initial render pass can also take 10s of ms - // for a similar reason. Because the context object is initialized far - // before the first frame, create a trivial texture and render pass to force - // the driver to compiler these shaders before the frame begins. - TextureDescriptor desc; - desc.size = {1, 1}; - desc.storage_mode = StorageMode::kHostVisible; - desc.format = PixelFormat::kR8G8B8A8UNormInt; - auto texture = GetContext()->GetResourceAllocator()->CreateTexture(desc); - uint32_t color = 0; - if (!texture->SetContents(reinterpret_cast(&color), 4u)) { - VALIDATION_LOG << "Failed to set bootstrap texture."; - } } } // namespace impeller From c44847d8e1f44237cacc15b187aadd9706f2db32 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 24 Apr 2024 15:42:03 -0700 Subject: [PATCH 6/6] chinmay feedback. --- impeller/entity/contents/content_context.h | 6 +++-- impeller/entity/contents/vertices_contents.cc | 27 +++++-------------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 4510afbc9e43b..8cc3bcb6de72a 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -725,8 +725,10 @@ class ContentContext { return point_field_compute_pipelines_; } - // An empty 1x1 texture for binding drawVertices/drawAtlas or other cases - // that don't always have a texture (due to blending). + /// A 1x1 RGBA transparent black texture. + /// + /// Used for binding drawVertices/drawAtlas or other cases that don't always + /// have a texture (due to blending). std::shared_ptr GetEmptyTexture() const; std::shared_ptr GetContext() const; diff --git a/impeller/entity/contents/vertices_contents.cc b/impeller/entity/contents/vertices_contents.cc index 43d95d6721308..0be661b68a11c 100644 --- a/impeller/entity/contents/vertices_contents.cc +++ b/impeller/entity/contents/vertices_contents.cc @@ -106,6 +106,8 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, } else { texture = texture_; } + } else { + texture = renderer.GetEmptyTexture(); } auto dst_sampler_descriptor = descriptor_; @@ -143,20 +145,12 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, options.primitive_type = geometry_result.type; pass.SetPipeline(renderer.GetPorterDuffBlendPipeline(options)); - if (texture) { - FS::BindTextureSamplerDst(pass, texture, dst_sampler); - } else { - // We need to bind something so validation layers doesn't complain. - FML_DCHECK(blend_mode == BlendMode::kDestination); - FS::BindTextureSamplerDst(pass, renderer.GetEmptyTexture(), dst_sampler); - } + FS::BindTextureSamplerDst(pass, texture, dst_sampler); VS::FrameInfo frame_info; FS::FragInfo frag_info; - if (texture) { - frame_info.texture_sampler_y_coord_scale = texture->GetYCoordScale(); - } + frame_info.texture_sampler_y_coord_scale = texture->GetYCoordScale(); frame_info.mvp = geometry_result.transform; frag_info.output_alpha = alpha_; @@ -195,20 +189,13 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, auto options = OptionsFromPassAndEntity(pass, entity); options.primitive_type = geometry_result.type; pass.SetPipeline(renderer.GetDrawVerticesUberShader(options)); - if (texture) { - FS::BindTextureSampler(pass, texture, dst_sampler); - } else { - // We need to bind something so validation layers doesn't complain. - FML_DCHECK(blend_mode == BlendMode::kDestination); - FS::BindTextureSampler(pass, renderer.GetEmptyTexture(), dst_sampler); - } + + FS::BindTextureSampler(pass, texture, dst_sampler); VS::FrameInfo frame_info; FS::FragInfo frag_info; - if (texture) { - frame_info.texture_sampler_y_coord_scale = texture->GetYCoordScale(); - } + frame_info.texture_sampler_y_coord_scale = texture->GetYCoordScale(); frame_info.mvp = geometry_result.transform; frag_info.alpha = alpha_; frag_info.blend_mode = static_cast(blend_mode);