Skip to content

Commit

Permalink
[Impeller] Avoid culling when the current matrix has perspective. (fl…
Browse files Browse the repository at this point in the history
…utter#44089)

Fixes flutter/flutter#130613

Before this patch, the test would fail to render anything because the culling would decide it fell outside the cull rect when the transform has perspective sometimes. We should fix our Rect::TransformBounds implementation, but I can't quite seem to get it happy enough here so I'm just bailing out on culling if there is perspective instead, which is safe. This also makes the patch a bit easier/safer to cherry pick since it's a simple de-optimization when perspective is involved for the sake of fidelity, instead of a larger change that may have other side effects.

Filed flutter/flutter#131445 to track the perspective issue.
  • Loading branch information
dnfield authored Jul 28, 2023
1 parent 7cbc8a8 commit 182e118
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
4 changes: 3 additions & 1 deletion impeller/display_list/dl_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,9 @@ void DlDispatcher::drawDisplayList(
canvas_.SaveLayer(save_paint);
}

if (display_list->has_rtree()) {
// TODO(131445): Remove this restriction if we can correctly cull with
// perspective transforms.
if (display_list->has_rtree() && !initial_matrix_.HasPerspective()) {
// The canvas remembers the screen-space culling bounds clipped by
// the surface and the history of clip calls. DisplayList can cull
// the ops based on a rectangle expressed in its "destination bounds"
Expand Down
37 changes: 37 additions & 0 deletions impeller/display_list/dl_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "impeller/display_list/dl_dispatcher.h"
#include "impeller/display_list/dl_image_impeller.h"
#include "impeller/display_list/dl_playground.h"
#include "impeller/entity/contents/solid_color_contents.h"
#include "impeller/entity/contents/solid_rrect_blur_contents.h"
#include "impeller/geometry/constants.h"
#include "impeller/geometry/point.h"
Expand Down Expand Up @@ -854,6 +855,42 @@ TEST_P(DisplayListTest, CanDrawShadow) {
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}

TEST_P(DisplayListTest,
DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists) {
// Regression test for https://github.com/flutter/flutter/issues/130613
flutter::DisplayListBuilder sub_builder(true);
sub_builder.DrawRect(SkRect::MakeXYWH(0, 0, 50, 50),
flutter::DlPaint(flutter::DlColor::kRed()));
auto display_list = sub_builder.Build();

DlDispatcher dispatcher(Rect::MakeLTRB(0, 0, 2400, 1800));
dispatcher.scale(2.0, 2.0);
dispatcher.translate(-93.0, 0.0);
// clang-format off
dispatcher.transformFullPerspective(
0.8, -0.2, -0.1, -0.0,
0.0, 1.0, 0.0, 0.0,
1.4, 1.3, 1.0, 0.0,
63.2, 65.3, 48.6, 1.1
);
// clang-format on
dispatcher.translate(35.0, 75.0);
dispatcher.drawDisplayList(display_list, 1.0f);
auto picture = dispatcher.EndRecordingAsPicture();

bool found = false;
picture.pass->IterateAllEntities([&found](Entity& entity) {
if (std::static_pointer_cast<SolidColorContents>(entity.GetContents())
->GetColor() == Color::Red()) {
found = true;
return false;
}

return true;
});
EXPECT_TRUE(found);
}

TEST_P(DisplayListTest, TransparentShadowProducesCorrectColor) {
DlDispatcher dispatcher;
dispatcher.save();
Expand Down
4 changes: 4 additions & 0 deletions impeller/geometry/matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ struct Matrix {
m[9] == 0 && m[10] == 1 && m[11] == 0 && m[14] == 0 && m[15] == 1);
}

constexpr bool HasPerspective() const {
return m[3] != 0 || m[7] != 0 || m[11] != 0 || m[15] != 1;
}

constexpr bool IsAligned(Scalar tolerance = 0) const {
int v[] = {!ScalarNearlyZero(m[0], tolerance), //
!ScalarNearlyZero(m[1], tolerance), //
Expand Down

0 comments on commit 182e118

Please sign in to comment.