Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 7 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
45 changes: 45 additions & 0 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3867,6 +3867,51 @@ TEST_P(AiksTest, GaussianBlurMipMapImageFilter) {
#endif
}

TEST_P(AiksTest, SaveLayersCloseToRootPassSizeAreScaledUp) {
Canvas canvas;
// Create a subpass with no bounds hint and an entity coverage of (95, 95).
canvas.SaveLayer({
.color = Color::Black().WithAlpha(0.5),
});
canvas.DrawRect(Rect::MakeLTRB(0, 0, 10, 10), {.color = Color::Red()});
canvas.DrawRect(Rect::MakeLTRB(0, 0, 95, 95), {.color = Color::Blue()});
canvas.Restore();

Picture picture = canvas.EndRecordingAsPicture();
std::shared_ptr<RenderTargetCache> cache =
std::make_shared<RenderTargetCache>(GetContext()->GetResourceAllocator());
AiksContext aiks_context(GetContext(), nullptr, cache);
picture.ToImage(aiks_context, {100, 100});

for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd();
++it) {
EXPECT_EQ(it->texture->GetTextureDescriptor().size, ISize(100, 100));
}
}

TEST_P(AiksTest, SaveLayersCloseToRootPassSizeAreNotScaledUpPastBoundsHint) {
Canvas canvas;
canvas.SaveLayer(
{
.color = Color::Black().WithAlpha(0.5),
},
Rect::MakeSize(ISize(95, 95)));
canvas.DrawRect(Rect::MakeLTRB(0, 0, 100, 100), {.color = Color::Red()});
canvas.DrawRect(Rect::MakeLTRB(50, 50, 150, 150), {.color = Color::Blue()});
canvas.Restore();

Picture picture = canvas.EndRecordingAsPicture();
std::shared_ptr<RenderTargetCache> cache =
std::make_shared<RenderTargetCache>(GetContext()->GetResourceAllocator());
AiksContext aiks_context(GetContext(), nullptr, cache);
picture.ToImage(aiks_context, {100, 100});

for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd();
++it) {
EXPECT_EQ(it->texture->GetTextureDescriptor().size, ISize(95, 95));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, one if these is 100,100, need to make the expection more specific.

}
}

TEST_P(AiksTest, ImageColorSourceEffectTransform) {
// Compare with https://fiddle.skia.org/c/6cdc5aefb291fda3833b806ca347a885

Expand Down
44 changes: 44 additions & 0 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
#include "impeller/entity/entity.h"
#include "impeller/entity/inline_pass_context.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/matrix.h"
#include "impeller/geometry/rect.h"
#include "impeller/geometry/size.h"
#include "impeller/renderer/command_buffer.h"

#ifdef IMPELLER_DEBUG
Expand Down Expand Up @@ -485,6 +487,45 @@ bool EntityPass::Render(ContentContext& renderer,
clip_coverage_stack); // clip_coverage_stack
}

// When a subpass size is close to, but still smaller than the root pass
// size and smaller than the bounds hint, we may scale it up to the root
// pass size. This will improve performance by improving the efficiency of
// the render target cache, as only textures with exactly the same sizes +
// descriptors can be recycled.
static ISize MaybeRoundUpTextureSize(ISize subpass_size,
ISize root_pass_size,
std::optional<Rect> bounds_limit,
const Matrix& subpass_transform) {
// If the subpass is already bigger than the root pass size,
// return the existing subpass size.
if (subpass_size.width > root_pass_size.width ||
subpass_size.height > root_pass_size.height) {
return subpass_size;
}

// If there is a bounds limit and it is tigher than the root pass size,
// return the existing subpass size. This case could be removed if we
// conditionally inserted clips/scissor instead.
if (bounds_limit.has_value()) {
auto user_bounds_size =
bounds_limit->TransformBounds(subpass_transform).GetSize();
Copy link
Member

@bdero bdero Jan 24, 2024

Choose a reason for hiding this comment

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

Would you mind adding a method like std::optional<Rect> EntityPass::GetTransformedBoundsLimit()? GetSubpassCoverage can use it directly and we can pass in transformed_bounds_limit for this static method instead of passing in bounds_limit + subpass_transform. Just to make it easier to not screw this up later on during refactors.

Copy link
Member

@bdero bdero Jan 24, 2024

Choose a reason for hiding this comment

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

I would have recommended we just apply the transform immediately when the bounds limit is set, but unfortunately the transform might end up getting modified if this EntityPass is appended to another EntityPass via DrawPicture...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we're need to remove drawPicture support soon, it doesn't actually work and was added for an aiks canvas API that didn't work out.

if (user_bounds_size.width < root_pass_size.width ||
user_bounds_size.height < root_pass_size.height) {
return subpass_size;
}
}

// If the subpass size is within 10% of the root pass size, round up
// to the root pass size.
if (root_pass_size.width - subpass_size.width <=
(0.1 * root_pass_size.width) &&
root_pass_size.height - subpass_size.height <=
(0.1 * root_pass_size.height)) {
return root_pass_size;
}
return subpass_size;
}

EntityPass::EntityResult EntityPass::GetEntityForElement(
const EntityPass::Element& element,
ContentContext& renderer,
Expand Down Expand Up @@ -618,6 +659,9 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
return EntityPass::EntityResult::Skip();
}

subpass_size =
MaybeRoundUpTextureSize(subpass_size, root_pass_size,
subpass->bounds_limit_, subpass->transform_);
auto subpass_target = CreateRenderTarget(
renderer, // renderer
subpass_size, // size
Expand Down