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 12 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
9d59af1
[Impeller] separate algorithm for computing render target size.
Aug 18, 2024
a935594
++
Aug 18, 2024
7aa3a48
Update save_layer_utils_unittests.cc
Aug 18, 2024
54cea5a
nolint.
Aug 19, 2024
e6e56d8
oh boy here we go.
Aug 19, 2024
98a5d68
cleanups.
Aug 19, 2024
81ccaa0
remove print.
Aug 19, 2024
55a6a50
Update save_layer_utils_unittests.cc
Aug 19, 2024
3591395
dont overwrite save layer bounds.
Aug 19, 2024
a8f0e2c
Merge branch 'render_target_sizing' of github.com:jonahwilliams/engin…
Aug 19, 2024
1c55cf5
++
Aug 20, 2024
a6f5f21
Update dl_dispatcher.cc
Aug 20, 2024
a646c1b
++
Aug 20, 2024
8daf291
Merge branch 'main' of github.com:flutter/engine into render_target_s…
Aug 20, 2024
199a846
++
Aug 20, 2024
b2eb6b9
++
Aug 20, 2024
d347783
++
Aug 20, 2024
24442e0
flood clip.
Aug 21, 2024
78b7f60
add flood clip.
Aug 21, 2024
2a4ac2d
++
Aug 21, 2024
3eff88b
++
Aug 21, 2024
04b5be5
Update entity_pass.cc
Aug 21, 2024
086abb8
Update entity_pass.h
Aug 21, 2024
4a5f30b
++
Aug 21, 2024
fa3afcc
Merge branch 'render_target_sizing' of github.com:jonahwilliams/engin…
Aug 21, 2024
4e7a707
partial review feedback.
Aug 23, 2024
e0105d7
more adjustments.
Aug 23, 2024
e8f34ac
oops
Aug 23, 2024
8a3d1ef
Merge branch 'main' into render_target_sizing
Aug 27, 2024
24ef59a
Merge branch 'main' into render_target_sizing
Aug 28, 2024
0cae7a8
flar review.
Aug 29, 2024
ae57e62
Merge branch 'main' of github.com:flutter/engine into render_target_s…
Aug 29, 2024
bc2276e
no bds from caller.
Aug 30, 2024
b3ab833
update comment.
Aug 30, 2024
334e5e9
++
Aug 30, 2024
82a6621
++
Aug 30, 2024
5011b3c
++
Aug 30, 2024
008b6f9
++
Aug 30, 2024
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
1 change: 1 addition & 0 deletions ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@
../../../flutter/impeller/entity/entity_unittests.cc
../../../flutter/impeller/entity/geometry/geometry_unittests.cc
../../../flutter/impeller/entity/render_target_cache_unittests.cc
../../../flutter/impeller/entity/save_layer_utils_unittests.cc
../../../flutter/impeller/fixtures
../../../flutter/impeller/geometry/README.md
../../../flutter/impeller/geometry/geometry_unittests.cc
Expand Down
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -42870,6 +42870,8 @@ ORIGIN: ../../../flutter/impeller/entity/inline_pass_context.cc + ../../../flutt
ORIGIN: ../../../flutter/impeller/entity/inline_pass_context.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/render_target_cache.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/render_target_cache.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/save_layer_utils.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/save_layer_utils.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.frag + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.vert + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/shaders/blending/blend_select.glsl + ../../../flutter/LICENSE
Expand Down Expand Up @@ -45749,6 +45751,8 @@ FILE: ../../../flutter/impeller/entity/inline_pass_context.cc
FILE: ../../../flutter/impeller/entity/inline_pass_context.h
FILE: ../../../flutter/impeller/entity/render_target_cache.cc
FILE: ../../../flutter/impeller/entity/render_target_cache.h
FILE: ../../../flutter/impeller/entity/save_layer_utils.cc
FILE: ../../../flutter/impeller/entity/save_layer_utils.h
FILE: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.frag
FILE: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.vert
FILE: ../../../flutter/impeller/entity/shaders/blending/blend_select.glsl
Expand Down
1 change: 1 addition & 0 deletions display_list/dl_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ void DisplayListBuilder::RestoreLayer() {
content_bounds.intersect(layer_op->rect);
}
}

layer_op->rect = content_bounds;
layer_op->max_blend_mode = current_layer().max_blend_mode;

Expand Down
9 changes: 5 additions & 4 deletions impeller/aiks/aiks_blend_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@ TEST_P(AiksTest, ColorWheel) {

draw_color_wheel(canvas);
auto color_wheel_picture = canvas.EndRecordingAsPicture();
auto snapshot = color_wheel_picture.Snapshot(renderer);
if (!snapshot.has_value() || !snapshot->texture) {
auto image = color_wheel_picture.ToImage(
renderer, ISize{GetWindowSize().width, GetWindowSize().height});
if (!image || !image->GetTexture()) {
return std::nullopt;
}
color_wheel_image = std::make_shared<Image>(snapshot->texture);
color_wheel_transform = snapshot->transform;
color_wheel_image = std::make_shared<Image>(image->GetTexture());
color_wheel_transform = Matrix();
}

Canvas canvas;
Expand Down
2 changes: 1 addition & 1 deletion impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ void Canvas::SaveLayer(const Paint& paint,

auto& new_layer_pass = GetCurrentPass();
if (bounds) {
new_layer_pass.SetBoundsLimit(bounds, bounds_promise);
new_layer_pass.SetBoundsLimit(bounds, bounds_from_caller);
}

// When applying a save layer, absorb any pending distributed opacity.
Expand Down
1 change: 1 addition & 0 deletions impeller/aiks/canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "flutter/testing/testing.h"
#include "impeller/aiks/canvas.h"
#include "impeller/aiks/image_filter.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/path_builder.h"

// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
Expand Down
53 changes: 24 additions & 29 deletions impeller/aiks/experimental_canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
#include "impeller/base/validation.h"
#include "impeller/core/allocator.h"
#include "impeller/core/formats.h"
#include "impeller/entity/contents/filters/filter_contents.h"
#include "impeller/entity/contents/framebuffer_blend_contents.h"
#include "impeller/entity/contents/text_contents.h"
#include "impeller/entity/entity.h"
#include "impeller/entity/entity_pass_clip_stack.h"
#include "impeller/entity/save_layer_utils.h"
#include "impeller/geometry/color.h"
#include "impeller/renderer/render_target.h"

Expand Down Expand Up @@ -310,11 +312,6 @@ void ExperimentalCanvas::SaveLayer(
bool bounds_from_caller) {
TRACE_EVENT0("flutter", "Canvas::saveLayer");

if (bounds.has_value() && bounds->IsEmpty()) {
Save(total_content_depth);
return;
}

if (!clip_coverage_stack_.HasCoverage()) {
// The current clip is empty. This means the pass texture won't be
// visible, so skip it.
Expand All @@ -338,13 +335,6 @@ void ExperimentalCanvas::SaveLayer(
->GetSize()))
.Intersection(current_clip_coverage);

if (!maybe_coverage_limit.has_value()) {
Save(total_content_depth);
return;
}
maybe_coverage_limit = maybe_coverage_limit->Intersection(
Rect::MakeSize(render_target_.GetRenderTargetSize()));

if (!maybe_coverage_limit.has_value() || maybe_coverage_limit->IsEmpty()) {
Save(total_content_depth);
return;
Expand All @@ -359,12 +349,32 @@ void ExperimentalCanvas::SaveLayer(
return;
}

std::shared_ptr<FilterContents> filter_contents;
if (paint.image_filter) {
filter_contents = paint.image_filter->GetFilterContents();
}

std::optional<Rect> maybe_subpass_coverage = ComputeSaveLayerCoverage(
bounds.value_or(Rect::MakeMaximum()),
transform_stack_.back().transform, //
coverage_limit, //
filter_contents, //
/*destructive_blend=*/Entity::IsBlendModeDestructive(paint.blend_mode),
/*has_backdrop_filter=*/!!backdrop_filter,
/*bounds_from_caller=*/bounds_from_caller);

if (!maybe_subpass_coverage.has_value() ||
maybe_subpass_coverage->IsEmpty()) {
Save(total_content_depth);
return;
}
auto subpass_coverage = maybe_subpass_coverage.value();

// Backdrop filter state, ignored if there is no BDF.
std::shared_ptr<FilterContents> backdrop_filter_contents;
Point local_position = {0, 0};
if (backdrop_filter) {
local_position =
current_clip_coverage.GetOrigin() - GetGlobalPassPosition();
local_position = subpass_coverage.GetOrigin() - GetGlobalPassPosition();
EntityPass::BackdropFilterProc backdrop_filter_proc =
[backdrop_filter = backdrop_filter->Clone()](
const FilterInput::Ref& input, const Matrix& effect_transform,
Expand Down Expand Up @@ -397,21 +407,6 @@ void ExperimentalCanvas::SaveLayer(
paint_copy.color.alpha *= transform_stack_.back().distributed_opacity;
transform_stack_.back().distributed_opacity = 1.0;

// Backdrop Filter must expand bounds to at least the clip stack, otherwise
// the coverage of the parent render pass.
Rect subpass_coverage;
if (backdrop_filter_contents ||
Entity::IsBlendModeDestructive(paint.blend_mode) || !bounds.has_value()) {
subpass_coverage = coverage_limit;
// TODO(jonahwilliams): if we have tight bounds we should be able to reduce
// this size here. if (bounds.has_value() && bounds_from_caller) {
// subpass_coverage =
// coverage_limit.Intersection(bounds.value()).value_or(bounds.value());
// }
} else {
subpass_coverage = bounds->TransformBounds(GetCurrentTransform());
}

render_passes_.push_back(LazyRenderingConfig(
renderer_, //
CreateRenderTarget(renderer_, //
Expand Down
16 changes: 0 additions & 16 deletions impeller/aiks/picture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,10 @@
#include <optional>

#include "impeller/base/validation.h"
#include "impeller/entity/entity.h"
#include "impeller/renderer/render_target.h"
#include "impeller/renderer/snapshot.h"

namespace impeller {

std::optional<Snapshot> Picture::Snapshot(AiksContext& context) {
auto coverage = pass->GetElementsCoverage(std::nullopt);
if (!coverage.has_value() || coverage->IsEmpty()) {
return std::nullopt;
}

const auto translate = Matrix::MakeTranslation(-coverage->GetOrigin());
auto texture =
RenderToTexture(context, ISize(coverage->GetSize()), translate);
return impeller::Snapshot{
.texture = std::move(texture),
.transform = Matrix::MakeTranslation(coverage->GetOrigin())};
}

std::shared_ptr<Image> Picture::ToImage(AiksContext& context,
ISize size) const {
if (size.IsEmpty()) {
Expand Down
2 changes: 0 additions & 2 deletions impeller/aiks/picture.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ namespace impeller {
struct Picture {
std::unique_ptr<EntityPass> pass;

std::optional<Snapshot> Snapshot(AiksContext& context);

std::shared_ptr<Image> ToImage(AiksContext& context, ISize size) const;

private:
Expand Down
16 changes: 2 additions & 14 deletions impeller/display_list/dl_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -622,23 +622,11 @@ void DlDispatcherBase::saveLayer(const SkRect& bounds,
auto promise = options.content_is_clipped()
? ContentBoundsPromise::kMayClipContents
: ContentBoundsPromise::kContainsContents;
std::optional<Rect> impeller_bounds;
if (!options.content_is_unbounded() || options.bounds_from_caller()) {
impeller_bounds = skia_conversions::ToRect(bounds);
}

// Empty bounds on a save layer that contains a BDF or destructive blend
// should be treated as unbounded. All other empty bounds can be skipped.
if (impeller_bounds.has_value() && impeller_bounds->IsEmpty() &&
(backdrop != nullptr ||
Entity::IsBlendModeDestructive(paint.blend_mode))) {
impeller_bounds = std::nullopt;
}

std::optional<Rect> impeller_bounds = skia_conversions::ToRect(bounds);
GetCanvas().SaveLayer(paint, impeller_bounds, ToImageFilter(backdrop),
promise, total_content_depth,
options.can_distribute_opacity(),
options.bounds_from_caller());
options.bounds_from_caller() && !backdrop);
Copy link
Contributor

Choose a reason for hiding this comment

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

The "bounds from caller" hint isn't useful here AFAICT. If it matters then the bounds are being used wrong. I use it internally in the Builder to know if the recorded op has bounds from the caller or "dummy" bounds. I then combine the 2 in the restore call. At that point "bounds from caller" has essentially completed its entire "raison d'être" and is obsolete. I leave it in there because it isn't really useful to delete it - the receiver's shouldn't have any use for it.

In all cases the bounds are the bounds of the content of the saveLayer. How they were determined may be interesting, but not really relevant to the fact that they are accurately the bounds of what the code between the saveLayer and its corresponding restore draw, potentially clipped by the user supplied bounds (in which case the "clipped" flag will be set in the options.

When determining how much to allocate, you would determine:

  • If there is a BDF, then allocate enough to map to the current clip bounds.
  • If the BDF somehow were to indicate that it isn't going to draw into those entire bounds, you can take that into account, but then you would also need to factor in the contents.
  • If you don't query the BDF about its output intentions, or if the BDF does indicate that it will draw to the entire target area, then you are done and the clip bounds are the answer.
  • If there is no BDF, then the RT needs to cover the indicated bounds from the saveLayer call transformed by the CTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another way to look at this, the saveLayer will cause 2 kinds of rendering:

  • If there is a BDF, then the BDF will filter everything underneath the saveLayer and come up with some output pixels. That will produce output relative as per the BDF and the CTM of the saveLayer and the input of the BDF is the pixels from the parent RT which are in device pixels. That's one source of rendering.
  • Whether there is a BDF, the indicated saveLayer bounds will represent how much you need to hold the content that was between saveLayer and Restore. These bounds are relative to the CTM.
  • Combine those 2, and clip to the current clip bounds and that represents the pixels you want to capture for the eventual blit to the parent RT.

The saveLayer bounds may be irrelevant if the BDF says "I plan to render to the entire surface" then what does it matter what the saveLayer content does? But, if we ask the BDF "what's your output if you transform these pixels expressed in the device coordinate system by the BDF acting in the CTM?" then the 2 bounds may need to be combined.

}

// |flutter::DlOpReceiver|
Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ impeller_component("entity") {
"inline_pass_context.h",
"render_target_cache.cc",
"render_target_cache.h",
"save_layer_utils.cc",
"save_layer_utils.h",
]

public_deps = [
Expand Down Expand Up @@ -257,6 +259,7 @@ impeller_component("entity_unittests") {
"entity_unittests.cc",
"geometry/geometry_unittests.cc",
"render_target_cache_unittests.cc",
"save_layer_utils_unittests.cc",
]

deps = [
Expand Down
Loading