From 5f4a81232530b714f37684dfec556b31ca313130 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 20 Jun 2023 17:09:03 -0700 Subject: [PATCH 1/4] [Impeller] Correctly compute UVs in texture fill Use decal sampling for subpass textures --- impeller/aiks/paint_pass_delegate.cc | 16 +++++++ impeller/entity/contents/texture_contents.cc | 50 ++++++++++---------- impeller/entity/contents/texture_contents.h | 4 +- impeller/entity/entity_unittests.cc | 4 +- impeller/geometry/geometry_unittests.cc | 15 ++++++ impeller/geometry/rect.h | 10 ++++ 6 files changed, 70 insertions(+), 29 deletions(-) diff --git a/impeller/aiks/paint_pass_delegate.cc b/impeller/aiks/paint_pass_delegate.cc index 019a67602386b..b46d27091a226 100644 --- a/impeller/aiks/paint_pass_delegate.cc +++ b/impeller/aiks/paint_pass_delegate.cc @@ -4,6 +4,8 @@ #include "impeller/aiks/paint_pass_delegate.h" +#include "impeller/core/formats.h" +#include "impeller/core/sampler_descriptor.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/contents/texture_contents.h" #include "impeller/entity/entity_pass.h" @@ -46,6 +48,13 @@ std::shared_ptr PaintPassDelegate::CreateContentsForSubpassTarget( contents->SetSourceRect(Rect::MakeSize(target->GetSize())); contents->SetOpacity(paint_.color.alpha); contents->SetDeferApplyingOpacity(true); + + SamplerDescriptor sampler_desc; + sampler_desc.label = "Subpass"; + sampler_desc.width_address_mode = SamplerAddressMode::kDecal; + sampler_desc.height_address_mode = SamplerAddressMode::kDecal; + contents->SetSamplerDescriptor(sampler_desc); + return paint_.WithFiltersForSubpassTarget(std::move(contents), effect_transform); } @@ -140,6 +149,13 @@ OpacityPeepholePassDelegate::CreateContentsForSubpassTarget( contents->SetSourceRect(Rect::MakeSize(target->GetSize())); contents->SetOpacity(paint_.color.alpha); contents->SetDeferApplyingOpacity(true); + + SamplerDescriptor sampler_desc; + sampler_desc.label = "Subpass"; + sampler_desc.width_address_mode = SamplerAddressMode::kDecal; + sampler_desc.height_address_mode = SamplerAddressMode::kDecal; + contents->SetSamplerDescriptor(sampler_desc); + return paint_.WithFiltersForSubpassTarget(std::move(contents), effect_transform); } diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index 90b3e157d853b..58eeb41851a72 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -26,7 +26,7 @@ TextureContents::~TextureContents() = default; std::shared_ptr TextureContents::MakeRect(Rect destination) { auto contents = std::make_shared(); - contents->rect_ = destination; + contents->destination_rect_ = destination; return contents; } @@ -34,8 +34,8 @@ void TextureContents::SetLabel(std::string label) { label_ = std::move(label); } -void TextureContents::SetRect(Rect rect) { - rect_ = rect; +void TextureContents::SetDestinationRect(Rect rect) { + destination_rect_ = rect; } void TextureContents::SetTexture(std::shared_ptr texture) { @@ -70,7 +70,7 @@ std::optional TextureContents::GetCoverage(const Entity& entity) const { if (GetOpacity() == 0) { return std::nullopt; } - return rect_.TransformBounds(entity.GetTransformation()); + return destination_rect_.TransformBounds(entity.GetTransformation()); }; std::optional TextureContents::RenderToSnapshot( @@ -82,7 +82,7 @@ std::optional TextureContents::RenderToSnapshot( const std::string& label) const { // Passthrough textures that have simple rectangle paths and complete source // rects. - auto bounds = rect_; + auto bounds = destination_rect_; auto opacity = GetOpacity(); if (source_rect_ == Rect::MakeSize(texture_->GetSize()) && (opacity >= 1 - kEhCloseEnough || defer_applying_opacity_)) { @@ -104,37 +104,37 @@ std::optional TextureContents::RenderToSnapshot( label); // label } -static TextureFillVertexShader::PerVertexData ComputeVertexData( - const Point& position, - const Rect& coverage_rect, - const ISize& texture_size, - const Rect& source_rect) { - TextureFillVertexShader::PerVertexData data; - data.position = position; - auto coverage_coords = (position - coverage_rect.origin) / coverage_rect.size; - data.texture_coords = - (source_rect.origin + source_rect.size * coverage_coords) / texture_size; - return data; -} - bool TextureContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { using VS = TextureFillVertexShader; using FS = TextureFillFragmentShader; - const auto coverage_rect = rect_; - - if (coverage_rect.size.IsEmpty() || source_rect_.IsEmpty() || + if (destination_rect_.size.IsEmpty() || source_rect_.IsEmpty() || texture_ == nullptr || texture_->GetSize().IsEmpty()) { return true; } + // Expand the destination rect by one pixel towards the bottom right. Map the + // texture coordinates such that texels are evenly spaced across the pixel + // grid. + auto destination_rect = destination_rect_.Expand(0, 0, 1, 1); + auto half_texel_size = + Size(0.5 / texture_->GetSize().width, 0.5 / texture_->GetSize().height); + auto texture_coords = + destination_rect_.Project(source_rect_.Shift(destination_rect_.origin)); + texture_coords = + texture_coords.Expand(-half_texel_size.width, -half_texel_size.height, + half_texel_size.width, half_texel_size.height); + VertexBufferBuilder vertex_builder; - for (const auto position : rect_.GetPoints()) { - vertex_builder.AppendVertex(ComputeVertexData( - position, coverage_rect, texture_->GetSize(), source_rect_)); - } + + vertex_builder.AddVertices({ + {destination_rect.GetLeftTop(), texture_coords.GetLeftTop()}, + {destination_rect.GetRightTop(), texture_coords.GetRightTop()}, + {destination_rect.GetLeftBottom(), texture_coords.GetLeftBottom()}, + {destination_rect.GetRightBottom(), texture_coords.GetRightBottom()}, + }); auto& host_buffer = pass.GetTransientsBuffer(); diff --git a/impeller/entity/contents/texture_contents.h b/impeller/entity/contents/texture_contents.h index 567e95a3c4818..4da57c1cfa465 100644 --- a/impeller/entity/contents/texture_contents.h +++ b/impeller/entity/contents/texture_contents.h @@ -30,7 +30,7 @@ class TextureContents final : public Contents { void SetLabel(std::string label); - void SetRect(Rect rect); + void SetDestinationRect(Rect rect); void SetTexture(std::shared_ptr texture); @@ -78,7 +78,7 @@ class TextureContents final : public Contents { private: std::string label_; - Rect rect_; + Rect destination_rect_; bool stencil_enabled_ = true; std::shared_ptr texture_; diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 65ceaf2ccb693..6c88a45dafe5a 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -1069,7 +1069,7 @@ TEST_P(EntityTest, GaussianBlurFilter) { if (selected_input_type == 0) { auto texture = std::make_shared(); texture->SetSourceRect(Rect::MakeSize(boston->GetSize())); - texture->SetRect(input_rect); + texture->SetDestinationRect(input_rect); texture->SetTexture(boston); texture->SetOpacity(input_color.alpha); @@ -1192,7 +1192,7 @@ TEST_P(EntityTest, MorphologyFilter) { Rect::MakeXYWH(path_rect[0], path_rect[1], path_rect[2], path_rect[3]); auto texture = std::make_shared(); texture->SetSourceRect(Rect::MakeSize(boston->GetSize())); - texture->SetRect(input_rect); + texture->SetDestinationRect(input_rect); texture->SetTexture(boston); texture->SetOpacity(input_color.alpha); diff --git a/impeller/geometry/geometry_unittests.cc b/impeller/geometry/geometry_unittests.cc index 11a57068da5bf..3a123c657ce61 100644 --- a/impeller/geometry/geometry_unittests.cc +++ b/impeller/geometry/geometry_unittests.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "gtest/gtest.h" #include "impeller/geometry/geometry_asserts.h" #include @@ -2142,6 +2143,20 @@ TEST(GeometryTest, RectScale) { } } +TEST(GeometryTest, RectDirections) { + auto r = Rect::MakeLTRB(1, 2, 3, 4); + + ASSERT_EQ(r.GetLeft(), 1); + ASSERT_EQ(r.GetTop(), 2); + ASSERT_EQ(r.GetRight(), 3); + ASSERT_EQ(r.GetBottom(), 4); + + ASSERT_POINT_NEAR(r.GetLeftTop(), Point(1, 2)); + ASSERT_POINT_NEAR(r.GetRightTop(), Point(3, 2)); + ASSERT_POINT_NEAR(r.GetLeftBottom(), Point(1, 4)); + ASSERT_POINT_NEAR(r.GetRightBottom(), Point(3, 4)); +} + TEST(GeometryTest, RectProject) { { auto r = Rect::MakeLTRB(-100, -100, 100, 100); diff --git a/impeller/geometry/rect.h b/impeller/geometry/rect.h index 7a2c18b7454fc..0bda6140d95b9 100644 --- a/impeller/geometry/rect.h +++ b/impeller/geometry/rect.h @@ -162,6 +162,16 @@ struct TRect { return std::max(origin.y, origin.y + size.height); } + constexpr TPoint GetLeftTop() const { return {GetLeft(), GetTop()}; } + + constexpr TPoint GetRightTop() const { return {GetRight(), GetTop()}; } + + constexpr TPoint GetLeftBottom() const { return {GetLeft(), GetBottom()}; } + + constexpr TPoint GetRightBottom() const { + return {GetRight(), GetBottom()}; + } + constexpr std::array GetLTRB() const { return {GetLeft(), GetTop(), GetRight(), GetBottom()}; } From ca8da505175fba699905eb7553f7477c0a14e747 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 21 Jun 2023 22:15:31 -0700 Subject: [PATCH 2/4] Fix tex coords --- impeller/entity/contents/texture_contents.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index 58eeb41851a72..c6a0830a56321 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -112,21 +112,22 @@ bool TextureContents::Render(const ContentContext& renderer, if (destination_rect_.size.IsEmpty() || source_rect_.IsEmpty() || texture_ == nullptr || texture_->GetSize().IsEmpty()) { - return true; + return true; // Nothing to render. } - // Expand the destination rect by one pixel towards the bottom right. Map the - // texture coordinates such that texels are evenly spaced across the pixel - // grid. - auto destination_rect = destination_rect_.Expand(0, 0, 1, 1); auto half_texel_size = Size(0.5 / texture_->GetSize().width, 0.5 / texture_->GetSize().height); auto texture_coords = - destination_rect_.Project(source_rect_.Shift(destination_rect_.origin)); + Rect::MakeSize(texture_->GetSize()).Project(source_rect_); texture_coords = texture_coords.Expand(-half_texel_size.width, -half_texel_size.height, half_texel_size.width, half_texel_size.height); + // Expand the destination rect by one pixel towards the bottom right. Map the + // texture coordinates such that texels are evenly spaced across the pixel + // grid. + auto destination_rect = destination_rect_.Expand(0, 0, 1, 1); + VertexBufferBuilder vertex_builder; vertex_builder.AddVertices({ From bb9c781772a5af4a71c8837a73d7e4ec29f25c24 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 21 Jun 2023 23:16:36 -0700 Subject: [PATCH 3/4] Offset half texel correctly --- impeller/entity/contents/texture_contents.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index c6a0830a56321..a6bdada0ba4cb 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -115,12 +115,14 @@ bool TextureContents::Render(const ContentContext& renderer, return true; // Nothing to render. } - auto half_texel_size = - Size(0.5 / texture_->GetSize().width, 0.5 / texture_->GetSize().height); auto texture_coords = Rect::MakeSize(texture_->GetSize()).Project(source_rect_); + + // Expand the texture coordinates + auto half_texel_size = + Size(0.5 / texture_->GetSize().width, 0.5 / texture_->GetSize().height); texture_coords = - texture_coords.Expand(-half_texel_size.width, -half_texel_size.height, + texture_coords.Expand(half_texel_size.width, half_texel_size.height, half_texel_size.width, half_texel_size.height); // Expand the destination rect by one pixel towards the bottom right. Map the From c4f30315afc23d165dc4ca698a9b1c76f1d2902b Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 21 Jun 2023 23:39:33 -0700 Subject: [PATCH 4/4] Use a simpler and more consistent solution for computing the UVs --- impeller/entity/contents/texture_contents.cc | 24 ++++++-------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index a6bdada0ba4cb..553d53c0c96ff 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -115,28 +115,18 @@ bool TextureContents::Render(const ContentContext& renderer, return true; // Nothing to render. } + // Expand the source rect by half a texel, which aligns sampled texels to the + // pixel grid if the source rect is the same size as the destination rect. auto texture_coords = - Rect::MakeSize(texture_->GetSize()).Project(source_rect_); - - // Expand the texture coordinates - auto half_texel_size = - Size(0.5 / texture_->GetSize().width, 0.5 / texture_->GetSize().height); - texture_coords = - texture_coords.Expand(half_texel_size.width, half_texel_size.height, - half_texel_size.width, half_texel_size.height); - - // Expand the destination rect by one pixel towards the bottom right. Map the - // texture coordinates such that texels are evenly spaced across the pixel - // grid. - auto destination_rect = destination_rect_.Expand(0, 0, 1, 1); + Rect::MakeSize(texture_->GetSize()).Project(source_rect_.Expand(0.5)); VertexBufferBuilder vertex_builder; vertex_builder.AddVertices({ - {destination_rect.GetLeftTop(), texture_coords.GetLeftTop()}, - {destination_rect.GetRightTop(), texture_coords.GetRightTop()}, - {destination_rect.GetLeftBottom(), texture_coords.GetLeftBottom()}, - {destination_rect.GetRightBottom(), texture_coords.GetRightBottom()}, + {destination_rect_.GetLeftTop(), texture_coords.GetLeftTop()}, + {destination_rect_.GetRightTop(), texture_coords.GetRightTop()}, + {destination_rect_.GetLeftBottom(), texture_coords.GetLeftBottom()}, + {destination_rect_.GetRightBottom(), texture_coords.GetRightBottom()}, }); auto& host_buffer = pass.GetTransientsBuffer();