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

[Impeller] Coerce opaque ColorSourceContents to Source #41525

Merged
merged 2 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2136,5 +2136,32 @@ TEST_P(AiksTest, CanRenderOffscreenCheckerboard) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, OpaqueEntitiesGetCoercedToSource) {
Canvas canvas;
canvas.Scale(Vector2(1.618, 1.618));
canvas.DrawCircle(Point(), 10,
{
.color = Color::CornflowerBlue(),
.blend_mode = BlendMode::kSourceOver,
});
Picture picture = canvas.EndRecordingAsPicture();

// Extract the SolidColorSource.
Entity entity;
std::shared_ptr<SolidColorContents> contents;
picture.pass->IterateAllEntities([&e = entity, &contents](Entity& entity) {
if (ScalarNearlyEqual(entity.GetTransformation().GetScale().x, 1.618f)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been occasionally using this pattern as a sort of poor man's predicate test for inspecting what Aiks + EntityPass is doing, but I don't think it's a sustainable solution in the longterm. @chinmaygarde recall you writing a doc around this kind of thing many moons ago, but I can't seem to find it...

e = entity;
contents =
std::static_pointer_cast<SolidColorContents>(entity.GetContents());
return false;
}
return true;
});

ASSERT_TRUE(contents->IsOpaque());
ASSERT_EQ(entity.GetBlendMode(), BlendMode::kSource);
}

} // namespace testing
} // namespace impeller
1 change: 1 addition & 0 deletions impeller/aiks/color_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "impeller/entity/contents/solid_color_contents.h"
#include "impeller/entity/contents/sweep_gradient_contents.h"
#include "impeller/entity/contents/tiled_texture_contents.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/matrix.h"
#include "impeller/geometry/scalar.h"
#include "impeller/runtime_stage/runtime_stage.h"
Expand Down
7 changes: 5 additions & 2 deletions impeller/aiks/paint_pass_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "impeller/entity/contents/contents.h"
#include "impeller/entity/contents/texture_contents.h"
#include "impeller/entity/entity_pass.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/path_builder.h"

namespace impeller {
Expand Down Expand Up @@ -72,6 +73,8 @@ bool OpacityPeepholePassDelegate::CanElide() {
// |EntityPassDelgate|
bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass(
EntityPass* entity_pass) {
// OpacityPeepholePassDelegate will only get used if the pass's blend mode is
// SourceOver, so no need to check here.
if (paint_.color.alpha <= 0.0 || paint_.color.alpha >= 1.0 ||
paint_.image_filter.has_value() || paint_.color_filter.has_value()) {
return false;
Expand All @@ -97,7 +100,7 @@ bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass(
auto had_subpass = entity_pass->IterateUntilSubpass(
[&all_coverages, &all_can_accept](Entity& entity) {
auto contents = entity.GetContents();
if (!contents->CanInheritOpacity(entity)) {
if (!entity.CanInheritOpacity()) {
all_can_accept = false;
return false;
}
Expand All @@ -119,7 +122,7 @@ bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass(
}
auto alpha = paint_.color.alpha;
entity_pass->IterateUntilSubpass([&alpha](Entity& entity) {
entity.GetContents()->SetInheritedOpacity(alpha);
entity.SetInheritedOpacity(alpha);
return true;
});
return true;
Expand Down
12 changes: 10 additions & 2 deletions impeller/core/texture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ Texture::~Texture() = default;

bool Texture::SetContents(const uint8_t* contents,
size_t length,
size_t slice) {
size_t slice,
bool is_opaque) {
if (!IsSliceValid(slice)) {
VALIDATION_LOG << "Invalid slice for texture.";
return false;
Expand All @@ -23,11 +24,13 @@ bool Texture::SetContents(const uint8_t* contents,
return false;
}
intent_ = TextureIntent::kUploadFromHost;
is_opaque_ = is_opaque;
return true;
}

bool Texture::SetContents(std::shared_ptr<const fml::Mapping> mapping,
size_t slice) {
size_t slice,
bool is_opaque) {
if (!IsSliceValid(slice)) {
VALIDATION_LOG << "Invalid slice for texture.";
return false;
Expand All @@ -39,9 +42,14 @@ bool Texture::SetContents(std::shared_ptr<const fml::Mapping> mapping,
return false;
}
intent_ = TextureIntent::kUploadFromHost;
is_opaque_ = is_opaque;
return true;
}

bool Texture::IsOpaque() const {
return is_opaque_;
}

size_t Texture::GetMipCount() const {
return GetTextureDescriptor().mip_count;
}
Expand Down
9 changes: 7 additions & 2 deletions impeller/core/texture.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@ class Texture {

[[nodiscard]] bool SetContents(const uint8_t* contents,
size_t length,
size_t slice = 0);
size_t slice = 0,
bool is_opaque = false);

[[nodiscard]] bool SetContents(std::shared_ptr<const fml::Mapping> mapping,
size_t slice = 0);
size_t slice = 0,
bool is_opaque = false);

virtual bool IsValid() const = 0;

virtual ISize GetSize() const = 0;

bool IsOpaque() const;

size_t GetMipCount() const;

const TextureDescriptor& GetTextureDescriptor() const;
Expand Down Expand Up @@ -59,6 +63,7 @@ class Texture {
private:
TextureIntent intent_ = TextureIntent::kRenderToTexture;
const TextureDescriptor desc_;
bool is_opaque_ = false;

bool IsSliceValid(size_t slice) const;

Expand Down
12 changes: 12 additions & 0 deletions impeller/entity/contents/conical_gradient_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,18 @@ void ConicalGradientContents::SetFocus(std::optional<Point> focus,
focus_radius_ = radius;
}

bool ConicalGradientContents::IsOpaque() const {
if (GetOpacity() < 1) {
return false;
}
for (auto color : colors_) {
if (!color.IsOpaque()) {
return false;
}
}
return true;
}

bool ConicalGradientContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/contents/conical_gradient_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class ConicalGradientContents final : public ColorSourceContents {

~ConicalGradientContents() override;

// |Contents|
bool IsOpaque() const override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/contents/contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ Contents::Contents() = default;

Contents::~Contents() = default;

bool Contents::IsOpaque() const {
return false;
}

Contents::StencilCoverage Contents::GetStencilCoverage(
const Entity& entity,
const std::optional<Rect>& current_stencil_coverage) const {
Expand Down
6 changes: 6 additions & 0 deletions impeller/entity/contents/contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ class Contents {
/// @brief Get the screen space bounding rectangle that this contents affects.
virtual std::optional<Rect> GetCoverage(const Entity& entity) const = 0;

/// @brief Whether this Contents only emits opaque source colors from the
/// fragment stage. This value does not account for any entity
/// properties (e.g. the blend mode), clips/visibility culling, or
/// inherited opacity.
virtual bool IsOpaque() const;

/// @brief Given the current screen space bounding rectangle of the stencil,
/// return the expected stencil coverage after this draw call. This
/// should only be implemented for contents that may write to the
Expand Down
12 changes: 12 additions & 0 deletions impeller/entity/contents/linear_gradient_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ void LinearGradientContents::SetTileMode(Entity::TileMode tile_mode) {
tile_mode_ = tile_mode;
}

bool LinearGradientContents::IsOpaque() const {
if (GetOpacity() < 1) {
return false;
}
for (auto color : colors_) {
if (!color.IsOpaque()) {
return false;
}
}
return true;
}

bool LinearGradientContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/contents/linear_gradient_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class LinearGradientContents final : public ColorSourceContents {

~LinearGradientContents() override;

// |Contents|
bool IsOpaque() const override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand Down
12 changes: 12 additions & 0 deletions impeller/entity/contents/radial_gradient_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@ const std::vector<Scalar>& RadialGradientContents::GetStops() const {
return stops_;
}

bool RadialGradientContents::IsOpaque() const {
if (GetOpacity() < 1) {
return false;
}
for (auto color : colors_) {
if (!color.IsOpaque()) {
return false;
}
}
return true;
}

bool RadialGradientContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/contents/radial_gradient_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class RadialGradientContents final : public ColorSourceContents {

~RadialGradientContents() override;

// |Contents|
bool IsOpaque() const override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/contents/solid_color_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ Color SolidColorContents::GetColor() const {
return color_.WithAlpha(color_.alpha * GetOpacity());
}

bool SolidColorContents::IsOpaque() const {
return GetColor().IsOpaque();
}

std::optional<Rect> SolidColorContents::GetCoverage(
const Entity& entity) const {
if (GetColor().IsTransparent()) {
Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/contents/solid_color_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class SolidColorContents final : public ColorSourceContents {

Color GetColor() const;

// |Contents|
bool IsOpaque() const override;

// |Contents|
std::optional<Rect> GetCoverage(const Entity& entity) const override;

Expand Down
12 changes: 12 additions & 0 deletions impeller/entity/contents/sweep_gradient_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ const std::vector<Scalar>& SweepGradientContents::GetStops() const {
return stops_;
}

bool SweepGradientContents::IsOpaque() const {
if (GetOpacity() < 1) {
return false;
}
for (auto color : colors_) {
if (!color.IsOpaque()) {
return false;
}
}
return true;
}

bool SweepGradientContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/contents/sweep_gradient_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class SweepGradientContents final : public ColorSourceContents {

~SweepGradientContents() override;

// |Contents|
bool IsOpaque() const override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand Down
11 changes: 11 additions & 0 deletions impeller/entity/contents/tiled_texture_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ bool TiledTextureContents::UsesEmulatedTileMode(
!TileModeToAddressMode(y_tile_mode_, capabilities).has_value();
}

// |Contents|
bool TiledTextureContents::IsOpaque() const {
if (GetOpacity() < 1) {
return false;
}
if (color_filter_.has_value()) {
return false;
}
return texture_->IsOpaque();
}

bool TiledTextureContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/contents/tiled_texture_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class TiledTextureContents final : public ColorSourceContents {
using ColorFilterProc =
std::function<std::shared_ptr<ColorFilterContents>(FilterInput::Ref)>;

// |Contents|
bool IsOpaque() const override;

// |Contents|
bool Render(const ContentContext& renderer,
const Entity& entity,
Expand Down
23 changes: 23 additions & 0 deletions impeller/entity/entity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "impeller/entity/contents/filters/filter_contents.h"
#include "impeller/entity/contents/texture_contents.h"
#include "impeller/entity/entity_pass.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/vector.h"
#include "impeller/renderer/render_pass.h"

Expand Down Expand Up @@ -101,6 +102,28 @@ BlendMode Entity::GetBlendMode() const {
return blend_mode_;
}

bool Entity::CanInheritOpacity() const {
if (!contents_) {
return false;
}
if (!((blend_mode_ == BlendMode::kSource && contents_->IsOpaque()) ||
blend_mode_ == BlendMode::kSourceOver)) {
return false;
}
return contents_->CanInheritOpacity(*this);
}

bool Entity::SetInheritedOpacity(Scalar alpha) {
if (!CanInheritOpacity()) {
return false;
}
if (blend_mode_ == BlendMode::kSource && contents_->IsOpaque()) {
blend_mode_ = BlendMode::kSourceOver;
}
contents_->SetInheritedOpacity(alpha);
return true;
}

/// @brief Returns true if the blend mode is "destrictive", meaning that even
/// fully transparent source colors would result in the destination
/// getting changed.
Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/entity.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ class Entity {

static bool IsBlendModeDestructive(BlendMode blend_mode);

bool CanInheritOpacity() const;

bool SetInheritedOpacity(Scalar alpha);

private:
Matrix transformation_;
std::shared_ptr<Contents> contents_;
Expand Down
5 changes: 5 additions & 0 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ void EntityPass::SetDelegate(std::unique_ptr<EntityPassDelegate> delegate) {
}

void EntityPass::AddEntity(Entity entity) {
if (entity.GetBlendMode() == BlendMode::kSourceOver &&
entity.GetContents()->IsOpaque()) {
entity.SetBlendMode(BlendMode::kSource);
}
Comment on lines +51 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this happen before or after we do the opacity peephole check?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens before (when Aiks pushes an Entity into the pass).

Hmm, it looks like we weren't handling blend modes in the opacity peephole. I added some restrictions and made it so it's able to unfold things with the Source blend mode. There's a new utility where the Entity handles SetInheritedOpacity with the blend mode in mind. Working on test cases for this at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like if the pass has kLuminosity as its blend mode and the child entities have kHue, it'll incorrectly collapse the pass.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nvm about the pass. I think the entities are still not being checked right, though.


if (entity.GetBlendMode() > Entity::kLastPipelineBlendMode) {
advanced_blend_reads_from_pass_texture_ += 1;
}
Expand Down
Loading