Skip to content

Commit

Permalink
Improve backdrop filter calculations in SkiaRenderer
Browse files Browse the repository at this point in the history
Pulls all of the bounds analysis for how to clip the output and input
of a backdrop filter into the CalculateRPDQParams function. Improves
the calculations for the `filter_bounds` value to be tighter for
backdrop filters since their output is always restricted.

Optimizes more cases to have backdrop output restricted by just a
clip instead of introducing an image filter node at the end. Switches
the input cropping node from a no-op Offset(0,0) to a proper `Crop`
that can define the boundary tilemode to apply (kClamp). This
actually fixes the behavior in the backdrop-offset unit test. It also
makes it possible to easily experiment with using mirror tiling for
content that is off screen.

Re-organizes some of the logic for clipping or clearing the backdrop
bounds into helper functions in RPDQParams so that
PrepareCanvasForRPDQ is more readable.

The backdrop logic to determine if an input crop is needed or not
used to have a bug (exposed when removing the staging flag
SK_USE_LEGACY_CONTENT_BOUNDS_PROPAGATION). The input bounds
calculation needs to use a null src rect so that the returned value
has no additional content restriction. If it *still* matches the
target output bounds, then we know it's not a pixel-moving backdrop
filter. I confirmed that the backdrop boundary layout tests work
correctly with and without the above staging flag.

Updates the conversion of FilterOperations to PaintFilter to define
the tiling geometry, so that clamp, mirror, and repeat explicitly
apply to the edges of a layer (instead of previously just the edge
of whatever image size happened to come through the pipeline). This
currently is only taken advantage of in SkiaRenderer,
SoftwareRenderer continues to use the legacy (less stable/defined)
tiling behavior.

Bug: b/40040615
Bug: chromium:978031
Bug: chromium:1473071
Bug: chromium:1451898
Change-Id: I65aef30266129b2b76cb7c9d38a38e8444c6789a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936570
Commit-Queue: Michael Ludwig <[email protected]>
Reviewed-by: Kyle Charbonneau <[email protected]>
Reviewed-by: Andrew Xu <[email protected]>
Reviewed-by: Xiyuan Xia <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1215501}
  • Loading branch information
lhkbob authored and Chromium LUCI CQ committed Oct 26, 2023
1 parent e731a3c commit bbbcef0
Show file tree
Hide file tree
Showing 30 changed files with 329 additions and 231 deletions.
10 changes: 5 additions & 5 deletions ash/app_list/views/app_list_item_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ TEST_P(AppListItemViewPixelTest, AppListItemView) {

ShowAppList();
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
GenerateScreenshotName(), /*revision_number=*/3, GetItemViewAt(0),
GenerateScreenshotName(), /*revision_number=*/4, GetItemViewAt(0),
GetItemViewAt(1)));
}

Expand All @@ -218,12 +218,12 @@ TEST_P(AppListItemViewPixelTest, AppListFolderItemsLayoutInIcon) {

if (jelly_enabled()) {
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
GenerateScreenshotName(), /*revision_number=*/4, GetItemViewAt(0),
GenerateScreenshotName(), /*revision_number=*/6, GetItemViewAt(0),
GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3),
GetItemViewAt(4)));
} else {
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
GenerateScreenshotName(), /*revision_number=*/5, GetItemViewAt(0),
GenerateScreenshotName(), /*revision_number=*/7, GetItemViewAt(0),
GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3),
GetItemViewAt(4)));
}
Expand Down Expand Up @@ -263,12 +263,12 @@ TEST_P(AppListItemViewPixelTest, AppListFolderIconExtendedState) {

if (jelly_enabled()) {
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
GenerateScreenshotName(), /*revision_number=*/4, GetItemViewAt(0),
GenerateScreenshotName(), /*revision_number=*/5, GetItemViewAt(0),
GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3),
GetItemViewAt(4)));
} else {
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
GenerateScreenshotName(), /*revision_number=*/5, GetItemViewAt(0),
GenerateScreenshotName(), /*revision_number=*/7, GetItemViewAt(0),
GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3),
GetItemViewAt(4)));
}
Expand Down
20 changes: 10 additions & 10 deletions ash/app_list/views/app_list_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ TEST_P(AppListViewPixelRTLTest, AnswerCardSearchResult) {
UseFixedPlaceholderTextAndHideCursor(test_helper->GetSearchBoxView());
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"bubble_launcher_answer_card_search_results",
/*revision_number=*/JellyEnabled() ? 9 : 7,
/*revision_number=*/JellyEnabled() ? 10 : 8,
GetAppListTestHelper()->GetBubbleView(),
GetPrimaryShelf()->navigation_widget()));
}
Expand All @@ -255,7 +255,7 @@ TEST_P(AppListViewPixelRTLTest, URLSearchResult) {
UseFixedPlaceholderTextAndHideCursor(test_helper->GetSearchBoxView());
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"bubble_launcher_url_search_results",
/*revision_number=*/JellyEnabled() ? 8 : 6,
/*revision_number=*/JellyEnabled() ? 9 : 7,
GetAppListTestHelper()->GetBubbleView(),
GetPrimaryShelf()->navigation_widget()));
}
Expand All @@ -277,7 +277,7 @@ TEST_P(AppListViewPixelRTLTest, KeyboardShortcutSearchResult) {

UseFixedPlaceholderTextAndHideCursor(test_helper->GetSearchBoxView());
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"bubble_launcher_ks_search_results", /*revision_number=*/0,
"bubble_launcher_ks_search_results", /*revision_number=*/1,
GetAppListTestHelper()->GetBubbleView()));
}

Expand All @@ -291,7 +291,7 @@ TEST_P(AppListViewPixelRTLTest, Basics) {
GetAppListTestHelper()->GetSearchBoxView());
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"bubble_launcher_basics",
/*revision_number=*/JellyEnabled() ? 8 : 6,
/*revision_number=*/JellyEnabled() ? 9 : 7,
GetAppListTestHelper()->GetBubbleView(),
GetPrimaryShelf()->navigation_widget()));
}
Expand All @@ -314,7 +314,7 @@ TEST_P(AppListViewPixelRTLTest, GradientZone) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"bubble_launcher_gradient_zone",
/*revision_number=*/JellyEnabled() ? 8 : 6,
/*revision_number=*/JellyEnabled() ? 9 : 7,
GetAppListTestHelper()->GetBubbleView(),
GetPrimaryShelf()->navigation_widget()));
}
Expand Down Expand Up @@ -426,7 +426,7 @@ INSTANTIATE_TEST_SUITE_P(RTL,
// Verifies the default layout for tablet mode launcher.
TEST_P(AppListViewTabletPixelTest, Basic) {
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"tablet_launcher_basics", 9,
"tablet_launcher_basics", /*revision_number=*/10,
GetAppListTestHelper()->GetAppsContainerView()));
}

Expand All @@ -447,7 +447,7 @@ TEST_P(AppListViewTabletPixelTest, TopGradientZone) {
generator->MoveTouchBy(0, -40);

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"tablet_launcher_top_gradient_zone", 7,
"tablet_launcher_top_gradient_zone", /*revision_number=*/8,
GetAppListTestHelper()->GetAppsContainerView()));
}

Expand All @@ -468,7 +468,7 @@ TEST_P(AppListViewTabletPixelTest, BottomGradientZone) {
generator->MoveTouchBy(0, -90);

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"tablet_launcher_bottom_gradient_zone", 9,
"tablet_launcher_bottom_gradient_zone", /*revision_number=*/10,
GetAppListTestHelper()->GetAppsContainerView()));
}

Expand All @@ -479,7 +479,7 @@ TEST_P(AppListViewTabletPixelTest, SearchBoxViewActive) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"search_box_view_active",
/*revision_number=*/IsJellyEnabled() ? 4 : 3, search_box_view));
/*revision_number=*/IsJellyEnabled() ? 5 : 4, search_box_view));
}

class AppListViewAssistantZeroStateTest
Expand Down Expand Up @@ -526,7 +526,7 @@ TEST_P(AppListViewAssistantZeroStateTest, Basic) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"app_list_view_assistant_zero_state",
/*revision_number=*/JellyEnabled(GetParam()) ? 5 : 4,
/*revision_number=*/JellyEnabled(GetParam()) ? 6 : 5,
page_view()->GetViewByID(AssistantViewID::kZeroStateView)));
}

Expand Down
2 changes: 1 addition & 1 deletion ash/fullscreen_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class FullscreenPixelTest : public AshTestBase {
// Verifies the primary fullscreen of an active user session.
TEST_F(FullscreenPixelTest, VerifyDefaultPrimaryDisplay) {
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"primary_display", /*revision_number=*/5, Shell::GetPrimaryRootWindow()));
"primary_display", /*revision_number=*/6, Shell::GetPrimaryRootWindow()));
}

} // namespace ash
2 changes: 1 addition & 1 deletion ash/glanceables/glanceables_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ TEST_F(GlanceablesPixelTest, GlanceablesZeroState) {
GetGlanceableTrayBubble()->GetTasksView()->ScrollViewToVisible();

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"glanceables_zero_state", /*revision_number=*/3,
"glanceables_zero_state", /*revision_number=*/4,
GetGlanceableTrayBubble()->GetBubbleView()));
}

Expand Down
6 changes: 3 additions & 3 deletions ash/shelf/scrollable_shelf_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ INSTANTIATE_TEST_SUITE_P(RTL, ScrollableShelfViewPixelRTLTest, testing::Bool());
TEST_P(ScrollableShelfViewPixelRTLTest, Basics) {
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"overflow",
/*revision_number=*/5, GetPrimaryShelf()->GetWindow()));
/*revision_number=*/6, GetPrimaryShelf()->GetWindow()));

ASSERT_TRUE(scrollable_shelf_view_->right_arrow());
const gfx::Point right_arrow_center =
Expand All @@ -58,7 +58,7 @@ TEST_P(ScrollableShelfViewPixelRTLTest, Basics) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"overflow_end",
/*revision_number=*/5, GetPrimaryShelf()->GetWindow()));
/*revision_number=*/6, GetPrimaryShelf()->GetWindow()));
}

TEST_P(ScrollableShelfViewPixelRTLTest, LeftRightShelfAlignment) {
Expand Down Expand Up @@ -116,7 +116,7 @@ TEST_P(ScrollableShelfViewWithGuestModePixelTest, VerifyShelfContextMenu) {
// Verify the shelf context menu and the shelf.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"shelf_context_menu",
/*revision_number=*/13,
/*revision_number=*/14,
GetPrimaryShelf()
->shelf_widget()
->shelf_view_for_testing()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ TEST_F(AccessibilityDetailedViewPixelTest, Basics) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/6, detailed_view_container));
/*revision_number=*/7, detailed_view_container));
}

} // namespace ash
4 changes: 2 additions & 2 deletions ash/system/audio/audio_detailed_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ TEST_F(AudioDetailedViewPixelTest, Basics) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"qs_audio_detailed_view",
/*revision_number=*/8, detailed_view));
/*revision_number=*/10, detailed_view));
}

TEST_F(AudioDetailedViewPixelTest, ShowNoiseCancellationButton) {
Expand Down Expand Up @@ -92,7 +92,7 @@ TEST_F(AudioDetailedViewPixelTest, ShowNoiseCancellationButton) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"qs_audio_detailed_view",
/*revision_number=*/2, detailed_view));
/*revision_number=*/4, detailed_view));
}

} // namespace ash
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ TEST_F(BluetoothDetailedViewImplPixelTest, Basics) {
// Compare pixels.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/5, detailed_view));
/*revision_number=*/7, detailed_view));
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion ash/system/brightness/display_detailed_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ TEST_F(DisplayDetailedViewPixelTest, Basics) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"qs_display_detailed_view",
/*revision_number=*/7, detailed_view));
/*revision_number=*/9, detailed_view));
}

} // namespace ash
2 changes: 1 addition & 1 deletion ash/system/cast/cast_detailed_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ TEST_F(CastDetailedViewPixelTest, Basics) {
ASSERT_TRUE(detailed_view);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/6, detailed_view));
/*revision_number=*/8, detailed_view));
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion ash/system/cast/cast_zero_state_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ TEST_F(CastZeroStateViewPixelTest, Basics) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"cast_zero_state_view",
/*revision_number=*/8, detailed_view));
/*revision_number=*/10, detailed_view));
}

} // namespace ash
2 changes: 1 addition & 1 deletion ash/system/ime/ime_detailed_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ TEST_F(IMEDetailedViewPixelTest, Basics) {
ASSERT_TRUE(detailed_view);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/7, detailed_view));
/*revision_number=*/9, detailed_view));
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion ash/system/locale/locale_detailed_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ TEST_F(LocaleDetailedViewPixelTest, Basics) {
ASSERT_TRUE(detailed_view);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/6, detailed_view));
/*revision_number=*/8, detailed_view));
}

} // namespace
Expand Down
12 changes: 6 additions & 6 deletions ash/system/message_center/ash_notification_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ TEST_F(AshNotificationViewPixelTest, CloseButtonFocused) {
EXPECT_TRUE(close_button->HasFocus());
EXPECT_EQ(control_buttons_layer->opacity(), 1);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"close_button_focused", /*revision_number=*/2, notification_view));
"close_button_focused", /*revision_number=*/3, notification_view));
}

// Regression test for http://b/267195370. Tests that a notification with no
Expand All @@ -141,7 +141,7 @@ TEST_F(AshNotificationViewPixelTest, CollapsedNoMessage) {
// Verify with a pixel test that the notification's title is vertically
// centered.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"collapsed_no_message", /*revision_number=*/2, notification_view));
"collapsed_no_message", /*revision_number=*/3, notification_view));
}

// Tests that a progress notification does not have its title vertically
Expand All @@ -162,7 +162,7 @@ TEST_F(AshNotificationViewPixelTest, ProgressCollapsed) {
// Verify with a pixel test that the notification's title is not vertically
// centered.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"progress_collapsed", /*revision_number=*/1, notification_view));
"progress_collapsed", /*revision_number=*/2, notification_view));
}

// Tests the control buttons UI for the case of a notification with just the
Expand All @@ -184,7 +184,7 @@ TEST_F(AshNotificationViewPixelTest, CloseControlButton) {
// Verify with a pixel test that the close control button is visible and has
// the proper placement.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"close_control_button", /*revision_number=*/1, notification_view));
"close_control_button", /*revision_number=*/2, notification_view));
}

// Tests the control buttons UI for the case of a notification with both the
Expand All @@ -206,7 +206,7 @@ TEST_F(AshNotificationViewPixelTest, SettingsAndCloseControlButtons) {
// Verify with a pixel test that the control buttons are visible and have
// proper spacing between them.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"settings_and_close_control_buttons", /*revision_number=*/1,
"settings_and_close_control_buttons", /*revision_number=*/2,
notification_view));
}

Expand Down Expand Up @@ -253,7 +253,7 @@ TEST_P(AshNotificationViewTitlePixelTest, NotificationTitleTest) {
// Compare pixels.
const std::string screenshot_name = GetScreenshotName();
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
screenshot_name, /*revision_number=*/6, notification_view));
screenshot_name, /*revision_number=*/7, notification_view));
}

class ScreenCaptureNotificationPixelTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ TEST_F(NetworkDetailedNetworkViewPixelTest, Basics) {
// Compare pixels.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/6, detailed_view));
/*revision_number=*/8, detailed_view));
}

} // namespace
Expand Down
4 changes: 2 additions & 2 deletions ash/system/network/vpn_detailed_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ TEST_F(VpnDetailedViewPixelTest, OnlyBuiltInVpn) {
// Compare pixels.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/6, vpn_detailed_view_));
/*revision_number=*/8, vpn_detailed_view_));
}

TEST_F(VpnDetailedViewPixelTest, MultipleVpns) {
Expand All @@ -138,7 +138,7 @@ TEST_F(VpnDetailedViewPixelTest, MultipleVpns) {
// Compare pixels.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"check_view",
/*revision_number=*/6, vpn_detailed_view_));
/*revision_number=*/8, vpn_detailed_view_));
}

} // namespace ash
4 changes: 2 additions & 2 deletions ash/system/status_area_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ TEST_P(StatusAreaParameterizedPixelTest, SystemTrayTest) {
system_tray->SetIsActive(IsActive());

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"system_tray" + GetScreenshotNameSuffix(), /*revision_number=*/1,
"system_tray" + GetScreenshotNameSuffix(), /*revision_number=*/2,
system_tray));
}

Expand All @@ -149,7 +149,7 @@ TEST_P(StatusAreaParameterizedPixelTest, DateTrayTest) {
date_tray->SetIsActive(IsActive());

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"date_tray" + GetScreenshotNameSuffix(), /*revision_number=*/1,
"date_tray" + GetScreenshotNameSuffix(), /*revision_number=*/2,
date_tray));
}

Expand Down
4 changes: 2 additions & 2 deletions ash/system/time/calendar_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ TEST_F(CalendarViewPixelTest, Basics) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"calendar_view",
/*revision_number=*/3, GetCalendarView()));
/*revision_number=*/5, GetCalendarView()));
}

TEST_F(CalendarViewPixelTest, EventList) {
Expand Down Expand Up @@ -141,7 +141,7 @@ TEST_F(CalendarViewPixelTest, EventList) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"event_list_view",
/*revision_number=*/5, GetEventListView()));
/*revision_number=*/6, GetEventListView()));
}

} // namespace ash
Loading

0 comments on commit bbbcef0

Please sign in to comment.