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

[Impeller] separate algorithm for computing render target size. #54604

Merged
auto-submit merged 38 commits into from
Aug 31, 2024
Merged

[Impeller] separate algorithm for computing render target size. #54604

Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
cleanups.
  • Loading branch information
jonahwilliams committed Aug 19, 2024
commit 98a5d68624995c6a00bd8a189f9f4927375809e3
2 changes: 1 addition & 1 deletion ci/licenses_golden/excluded_files
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@
../../../flutter/impeller/aiks/aiks_unittests.cc
../../../flutter/impeller/aiks/aiks_unittests.h
../../../flutter/impeller/aiks/canvas_unittests.cc
../../../flutter/impeller/aiks/save_layer_utils_unittests.cc
../../../flutter/impeller/aiks/testing
../../../flutter/impeller/base/README.md
../../../flutter/impeller/base/allocation_size_unittests.cc
Expand Down Expand Up @@ -174,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
8 changes: 4 additions & 4 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42638,8 +42638,6 @@ ORIGIN: ../../../flutter/impeller/aiks/paint_pass_delegate.cc + ../../../flutter
ORIGIN: ../../../flutter/impeller/aiks/paint_pass_delegate.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/aiks/picture.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/aiks/picture.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/aiks/save_layer_utils.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/aiks/save_layer_utils.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/base/allocation.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/base/allocation.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/base/allocation_size.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -42872,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 @@ -45519,8 +45519,6 @@ FILE: ../../../flutter/impeller/aiks/paint_pass_delegate.cc
FILE: ../../../flutter/impeller/aiks/paint_pass_delegate.h
FILE: ../../../flutter/impeller/aiks/picture.cc
FILE: ../../../flutter/impeller/aiks/picture.h
FILE: ../../../flutter/impeller/aiks/save_layer_utils.cc
FILE: ../../../flutter/impeller/aiks/save_layer_utils.h
FILE: ../../../flutter/impeller/base/allocation.cc
FILE: ../../../flutter/impeller/base/allocation.h
FILE: ../../../flutter/impeller/base/allocation_size.cc
Expand Down Expand Up @@ -45753,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
16 changes: 0 additions & 16 deletions impeller/aiks/picture.cc
View file
Open in desktop
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
View file
Open in desktop
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: 9 additions & 7 deletions impeller/entity/entity_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,17 +484,19 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
subpass->delegate_->WithImageFilter(Rect(), subpass->transform_);

auto subpass_coverage = ComputeSaveLayerCoverage(
subpass->bounds_limit_.value_or(Rect::MakeMaximum()), //
subpass->transform_, //
coverage_limit.value(), //
image_filter, //
/*destructive_blend=*/subpass->flood_clip_,
/*has_backdrop_filter=*/!!subpass_backdrop_filter_contents,
/*bounds_from_caller=*/subpass->bounds_from_caller_);
subpass->bounds_limit_.value_or(Rect::MakeMaximum()), //
subpass->transform_, //
coverage_limit.value(), //
image_filter, //
/*destructive_blend=*/subpass->flood_clip_, //
/*has_backdrop_filter=*/!!subpass_backdrop_filter_contents, //
/*bounds_from_caller=*/subpass->bounds_from_caller_ //
);

if (!subpass_coverage.has_value()) {
return EntityPass::EntityResult::Skip();
}
FML_LOG(ERROR) << "subpass_coverage: " << subpass_coverage.value();

auto subpass_size = ISize(subpass_coverage->GetSize());
if (subpass_size.IsEmpty()) {
Expand Down
23 changes: 16 additions & 7 deletions impeller/entity/save_layer_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,25 @@ std::optional<Rect> ComputeSaveLayerCoverage(
return std::nullopt;
}
Copy link
Contributor

@flar flar Aug 29, 2024

Choose a reason for hiding this comment

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

I'm trying to wrap my head around this. Why aren't we handing in content_coverage? That might be the same as coverage_limit if there was flooding, but it could be less.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Aug 29, 2024

Choose a reason for hiding this comment

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

Not sure what you mean exactly. A case where this would fire is, you an IF that translates a SL entirely offscreen, or somethng like that.

Copy link
Contributor

@flar flar Aug 29, 2024

Choose a reason for hiding this comment

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

This was a case of clicking on the wrong line. I was referring to the fact that you pass the coverage_limit in to the GetSourceCoverage call and I was pointing out that the content size might be much smaller.

But, in typing that out I suddenly realized that the point there is not to compute the actual result, but to compute the "worst case scenario for input pixels" against which you clip the indicated coverage. So, I think this is fine, but I didn't get the explanation on my first reading.

The intersection with the source coverage happens right after this comment, so this code is only making the current input coverage smaller - but based on the reverse filter size of the output limit - which means passing in coverage_limit is the right data to compute on.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Aug 29, 2024

Choose a reason for hiding this comment

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

I moved the comment around a bit so it should be clearer.

result_bounds = input_coverage.Intersection(source_coverage_limit.value());
} else {
result_bounds = input_coverage.Intersection(coverage_limit);

// Finally, transform the resulting bounds back into the global coordinate
// space.
if (result_bounds.has_value()) {
return result_bounds->TransformBounds(effect_transform);
}
return std::nullopt;
}

// Finally, transform the resulting bounds back into the global coordinate
// space.
if (result_bounds.has_value()) {
return result_bounds->TransformBounds(effect_transform);
// If the input coverage is maximum, just return the coverage limit that
// is already in the global coordinate space.
if (input_coverage.IsMaximum()) {
return coverage_limit;
}
return std::nullopt;

// Transform the input coverage into the global coordinate space before
// computing the bounds limit intersection.
return input_coverage.TransformBounds(effect_transform)
.Intersection(coverage_limit);
}

} // namespace impeller