diff --git a/ash/app_list/views/app_list_item_view_pixeltest.cc b/ash/app_list/views/app_list_item_view_pixeltest.cc index 7feaa807d24734..141f00346ef422 100644 --- a/ash/app_list/views/app_list_item_view_pixeltest.cc +++ b/ash/app_list/views/app_list_item_view_pixeltest.cc @@ -193,7 +193,7 @@ TEST_P(AppListItemViewPixelTest, AppListItemView) { ShowAppList(); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - GenerateScreenshotName(), /*revision_number=*/4, GetItemViewAt(0), + GenerateScreenshotName(), /*revision_number=*/3, GetItemViewAt(0), GetItemViewAt(1))); } @@ -218,12 +218,12 @@ TEST_P(AppListItemViewPixelTest, AppListFolderItemsLayoutInIcon) { if (jelly_enabled()) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - GenerateScreenshotName(), /*revision_number=*/6, GetItemViewAt(0), + GenerateScreenshotName(), /*revision_number=*/4, GetItemViewAt(0), GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3), GetItemViewAt(4))); } else { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - GenerateScreenshotName(), /*revision_number=*/7, GetItemViewAt(0), + GenerateScreenshotName(), /*revision_number=*/5, GetItemViewAt(0), GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3), GetItemViewAt(4))); } @@ -263,12 +263,12 @@ TEST_P(AppListItemViewPixelTest, AppListFolderIconExtendedState) { if (jelly_enabled()) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - GenerateScreenshotName(), /*revision_number=*/5, GetItemViewAt(0), + GenerateScreenshotName(), /*revision_number=*/4, GetItemViewAt(0), GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3), GetItemViewAt(4))); } else { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - GenerateScreenshotName(), /*revision_number=*/7, GetItemViewAt(0), + GenerateScreenshotName(), /*revision_number=*/5, GetItemViewAt(0), GetItemViewAt(1), GetItemViewAt(2), GetItemViewAt(3), GetItemViewAt(4))); } diff --git a/ash/app_list/views/app_list_view_pixeltest.cc b/ash/app_list/views/app_list_view_pixeltest.cc index 661e684965a940..e47a9a78277356 100644 --- a/ash/app_list/views/app_list_view_pixeltest.cc +++ b/ash/app_list/views/app_list_view_pixeltest.cc @@ -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() ? 10 : 8, + /*revision_number=*/JellyEnabled() ? 9 : 7, GetAppListTestHelper()->GetBubbleView(), GetPrimaryShelf()->navigation_widget())); } @@ -255,7 +255,7 @@ TEST_P(AppListViewPixelRTLTest, URLSearchResult) { UseFixedPlaceholderTextAndHideCursor(test_helper->GetSearchBoxView()); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "bubble_launcher_url_search_results", - /*revision_number=*/JellyEnabled() ? 9 : 7, + /*revision_number=*/JellyEnabled() ? 8 : 6, GetAppListTestHelper()->GetBubbleView(), GetPrimaryShelf()->navigation_widget())); } @@ -277,7 +277,7 @@ TEST_P(AppListViewPixelRTLTest, KeyboardShortcutSearchResult) { UseFixedPlaceholderTextAndHideCursor(test_helper->GetSearchBoxView()); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "bubble_launcher_ks_search_results", /*revision_number=*/1, + "bubble_launcher_ks_search_results", /*revision_number=*/0, GetAppListTestHelper()->GetBubbleView())); } @@ -291,7 +291,7 @@ TEST_P(AppListViewPixelRTLTest, Basics) { GetAppListTestHelper()->GetSearchBoxView()); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "bubble_launcher_basics", - /*revision_number=*/JellyEnabled() ? 9 : 7, + /*revision_number=*/JellyEnabled() ? 8 : 6, GetAppListTestHelper()->GetBubbleView(), GetPrimaryShelf()->navigation_widget())); } @@ -314,7 +314,7 @@ TEST_P(AppListViewPixelRTLTest, GradientZone) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "bubble_launcher_gradient_zone", - /*revision_number=*/JellyEnabled() ? 9 : 7, + /*revision_number=*/JellyEnabled() ? 8 : 6, GetAppListTestHelper()->GetBubbleView(), GetPrimaryShelf()->navigation_widget())); } @@ -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", /*revision_number=*/10, + "tablet_launcher_basics", 9, GetAppListTestHelper()->GetAppsContainerView())); } @@ -447,7 +447,7 @@ TEST_P(AppListViewTabletPixelTest, TopGradientZone) { generator->MoveTouchBy(0, -40); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "tablet_launcher_top_gradient_zone", /*revision_number=*/8, + "tablet_launcher_top_gradient_zone", 7, GetAppListTestHelper()->GetAppsContainerView())); } @@ -468,7 +468,7 @@ TEST_P(AppListViewTabletPixelTest, BottomGradientZone) { generator->MoveTouchBy(0, -90); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "tablet_launcher_bottom_gradient_zone", /*revision_number=*/10, + "tablet_launcher_bottom_gradient_zone", 9, GetAppListTestHelper()->GetAppsContainerView())); } @@ -479,7 +479,7 @@ TEST_P(AppListViewTabletPixelTest, SearchBoxViewActive) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "search_box_view_active", - /*revision_number=*/IsJellyEnabled() ? 5 : 4, search_box_view)); + /*revision_number=*/IsJellyEnabled() ? 4 : 3, search_box_view)); } class AppListViewAssistantZeroStateTest @@ -526,7 +526,7 @@ TEST_P(AppListViewAssistantZeroStateTest, Basic) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "app_list_view_assistant_zero_state", - /*revision_number=*/JellyEnabled(GetParam()) ? 6 : 5, + /*revision_number=*/JellyEnabled(GetParam()) ? 5 : 4, page_view()->GetViewByID(AssistantViewID::kZeroStateView))); } diff --git a/ash/fullscreen_pixeltest.cc b/ash/fullscreen_pixeltest.cc index 1c8267d050188a..5ba70f94624d0b 100644 --- a/ash/fullscreen_pixeltest.cc +++ b/ash/fullscreen_pixeltest.cc @@ -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=*/6, Shell::GetPrimaryRootWindow())); + "primary_display", /*revision_number=*/5, Shell::GetPrimaryRootWindow())); } } // namespace ash diff --git a/ash/glanceables/glanceables_pixeltest.cc b/ash/glanceables/glanceables_pixeltest.cc index 934c9c509801c3..644170f9eba59e 100644 --- a/ash/glanceables/glanceables_pixeltest.cc +++ b/ash/glanceables/glanceables_pixeltest.cc @@ -115,7 +115,7 @@ TEST_F(GlanceablesPixelTest, GlanceablesZeroState) { GetGlanceableTrayBubble()->GetTasksView()->ScrollViewToVisible(); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "glanceables_zero_state", /*revision_number=*/4, + "glanceables_zero_state", /*revision_number=*/3, GetGlanceableTrayBubble()->GetBubbleView())); } diff --git a/ash/shelf/scrollable_shelf_view_pixeltest.cc b/ash/shelf/scrollable_shelf_view_pixeltest.cc index 0ade761026291a..d66f30e636eabe 100644 --- a/ash/shelf/scrollable_shelf_view_pixeltest.cc +++ b/ash/shelf/scrollable_shelf_view_pixeltest.cc @@ -47,7 +47,7 @@ INSTANTIATE_TEST_SUITE_P(RTL, ScrollableShelfViewPixelRTLTest, testing::Bool()); TEST_P(ScrollableShelfViewPixelRTLTest, Basics) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "overflow", - /*revision_number=*/6, GetPrimaryShelf()->GetWindow())); + /*revision_number=*/5, GetPrimaryShelf()->GetWindow())); ASSERT_TRUE(scrollable_shelf_view_->right_arrow()); const gfx::Point right_arrow_center = @@ -58,7 +58,7 @@ TEST_P(ScrollableShelfViewPixelRTLTest, Basics) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "overflow_end", - /*revision_number=*/6, GetPrimaryShelf()->GetWindow())); + /*revision_number=*/5, GetPrimaryShelf()->GetWindow())); } TEST_P(ScrollableShelfViewPixelRTLTest, LeftRightShelfAlignment) { @@ -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=*/14, + /*revision_number=*/13, GetPrimaryShelf() ->shelf_widget() ->shelf_view_for_testing() diff --git a/ash/system/accessibility/accessibility_detailed_view_pixeltest.cc b/ash/system/accessibility/accessibility_detailed_view_pixeltest.cc index 0900fd60d1db5e..2d6e7d9d667258 100644 --- a/ash/system/accessibility/accessibility_detailed_view_pixeltest.cc +++ b/ash/system/accessibility/accessibility_detailed_view_pixeltest.cc @@ -47,7 +47,7 @@ TEST_F(AccessibilityDetailedViewPixelTest, Basics) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/7, detailed_view_container)); + /*revision_number=*/6, detailed_view_container)); } } // namespace ash diff --git a/ash/system/audio/audio_detailed_view_pixeltest.cc b/ash/system/audio/audio_detailed_view_pixeltest.cc index 42a465a6d81c7a..166389982ac425 100644 --- a/ash/system/audio/audio_detailed_view_pixeltest.cc +++ b/ash/system/audio/audio_detailed_view_pixeltest.cc @@ -57,7 +57,7 @@ TEST_F(AudioDetailedViewPixelTest, Basics) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "qs_audio_detailed_view", - /*revision_number=*/10, detailed_view)); + /*revision_number=*/8, detailed_view)); } TEST_F(AudioDetailedViewPixelTest, ShowNoiseCancellationButton) { @@ -92,7 +92,7 @@ TEST_F(AudioDetailedViewPixelTest, ShowNoiseCancellationButton) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "qs_audio_detailed_view", - /*revision_number=*/4, detailed_view)); + /*revision_number=*/2, detailed_view)); } } // namespace ash diff --git a/ash/system/bluetooth/bluetooth_detailed_view_impl_pixeltest.cc b/ash/system/bluetooth/bluetooth_detailed_view_impl_pixeltest.cc index ffcefef1badadd..a18321531d3fcf 100644 --- a/ash/system/bluetooth/bluetooth_detailed_view_impl_pixeltest.cc +++ b/ash/system/bluetooth/bluetooth_detailed_view_impl_pixeltest.cc @@ -94,7 +94,7 @@ TEST_F(BluetoothDetailedViewImplPixelTest, Basics) { // Compare pixels. EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/7, detailed_view)); + /*revision_number=*/5, detailed_view)); } } // namespace diff --git a/ash/system/brightness/display_detailed_view_pixeltest.cc b/ash/system/brightness/display_detailed_view_pixeltest.cc index 30f4463dc6fba1..0bf68737d9a08d 100644 --- a/ash/system/brightness/display_detailed_view_pixeltest.cc +++ b/ash/system/brightness/display_detailed_view_pixeltest.cc @@ -51,7 +51,7 @@ TEST_F(DisplayDetailedViewPixelTest, Basics) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "qs_display_detailed_view", - /*revision_number=*/9, detailed_view)); + /*revision_number=*/7, detailed_view)); } } // namespace ash diff --git a/ash/system/cast/cast_detailed_view_pixeltest.cc b/ash/system/cast/cast_detailed_view_pixeltest.cc index b4443c9eda5346..206464fe4e96b3 100644 --- a/ash/system/cast/cast_detailed_view_pixeltest.cc +++ b/ash/system/cast/cast_detailed_view_pixeltest.cc @@ -66,7 +66,7 @@ TEST_F(CastDetailedViewPixelTest, Basics) { ASSERT_TRUE(detailed_view); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/8, detailed_view)); + /*revision_number=*/6, detailed_view)); } } // namespace diff --git a/ash/system/cast/cast_zero_state_view_pixeltest.cc b/ash/system/cast/cast_zero_state_view_pixeltest.cc index 76724665b60d68..9f585927d74ab2 100644 --- a/ash/system/cast/cast_zero_state_view_pixeltest.cc +++ b/ash/system/cast/cast_zero_state_view_pixeltest.cc @@ -51,7 +51,7 @@ TEST_F(CastZeroStateViewPixelTest, Basics) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "cast_zero_state_view", - /*revision_number=*/10, detailed_view)); + /*revision_number=*/8, detailed_view)); } } // namespace ash diff --git a/ash/system/ime/ime_detailed_view_pixeltest.cc b/ash/system/ime/ime_detailed_view_pixeltest.cc index abd94ffa7e3fd2..4c96540b75de04 100644 --- a/ash/system/ime/ime_detailed_view_pixeltest.cc +++ b/ash/system/ime/ime_detailed_view_pixeltest.cc @@ -76,7 +76,7 @@ TEST_F(IMEDetailedViewPixelTest, Basics) { ASSERT_TRUE(detailed_view); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/9, detailed_view)); + /*revision_number=*/7, detailed_view)); } } // namespace diff --git a/ash/system/locale/locale_detailed_view_pixeltest.cc b/ash/system/locale/locale_detailed_view_pixeltest.cc index 9a3471c269babb..10cfe633fe9ae4 100644 --- a/ash/system/locale/locale_detailed_view_pixeltest.cc +++ b/ash/system/locale/locale_detailed_view_pixeltest.cc @@ -54,7 +54,7 @@ TEST_F(LocaleDetailedViewPixelTest, Basics) { ASSERT_TRUE(detailed_view); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/8, detailed_view)); + /*revision_number=*/6, detailed_view)); } } // namespace diff --git a/ash/system/message_center/ash_notification_view_pixeltest.cc b/ash/system/message_center/ash_notification_view_pixeltest.cc index 7ec7809f7abe37..5c7badc4922d83 100644 --- a/ash/system/message_center/ash_notification_view_pixeltest.cc +++ b/ash/system/message_center/ash_notification_view_pixeltest.cc @@ -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=*/3, notification_view)); + "close_button_focused", /*revision_number=*/2, notification_view)); } // Regression test for http://b/267195370. Tests that a notification with no @@ -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=*/3, notification_view)); + "collapsed_no_message", /*revision_number=*/2, notification_view)); } // Tests that a progress notification does not have its title vertically @@ -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=*/2, notification_view)); + "progress_collapsed", /*revision_number=*/1, notification_view)); } // Tests the control buttons UI for the case of a notification with just the @@ -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=*/2, notification_view)); + "close_control_button", /*revision_number=*/1, notification_view)); } // Tests the control buttons UI for the case of a notification with both the @@ -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=*/2, + "settings_and_close_control_buttons", /*revision_number=*/1, notification_view)); } @@ -253,7 +253,7 @@ TEST_P(AshNotificationViewTitlePixelTest, NotificationTitleTest) { // Compare pixels. const std::string screenshot_name = GetScreenshotName(); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - screenshot_name, /*revision_number=*/7, notification_view)); + screenshot_name, /*revision_number=*/6, notification_view)); } class ScreenCaptureNotificationPixelTest diff --git a/ash/system/network/network_detailed_network_view_pixeltest.cc b/ash/system/network/network_detailed_network_view_pixeltest.cc index b5637193ca8d3e..14b5bf0273b6b0 100644 --- a/ash/system/network/network_detailed_network_view_pixeltest.cc +++ b/ash/system/network/network_detailed_network_view_pixeltest.cc @@ -129,7 +129,7 @@ TEST_F(NetworkDetailedNetworkViewPixelTest, Basics) { // Compare pixels. EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/8, detailed_view)); + /*revision_number=*/6, detailed_view)); } } // namespace diff --git a/ash/system/network/vpn_detailed_view_pixeltest.cc b/ash/system/network/vpn_detailed_view_pixeltest.cc index d5133fb37b426c..277538ba5fbdb5 100644 --- a/ash/system/network/vpn_detailed_view_pixeltest.cc +++ b/ash/system/network/vpn_detailed_view_pixeltest.cc @@ -122,7 +122,7 @@ TEST_F(VpnDetailedViewPixelTest, OnlyBuiltInVpn) { // Compare pixels. EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/8, vpn_detailed_view_)); + /*revision_number=*/6, vpn_detailed_view_)); } TEST_F(VpnDetailedViewPixelTest, MultipleVpns) { @@ -131,7 +131,7 @@ TEST_F(VpnDetailedViewPixelTest, MultipleVpns) { // Compare pixels. EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "check_view", - /*revision_number=*/8, vpn_detailed_view_)); + /*revision_number=*/6, vpn_detailed_view_)); } } // namespace ash diff --git a/ash/system/status_area_pixeltest.cc b/ash/system/status_area_pixeltest.cc index e4762dee1d007a..9652b30ab4cf02 100644 --- a/ash/system/status_area_pixeltest.cc +++ b/ash/system/status_area_pixeltest.cc @@ -136,7 +136,7 @@ TEST_P(StatusAreaParameterizedPixelTest, SystemTrayTest) { system_tray->SetIsActive(IsActive()); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "system_tray" + GetScreenshotNameSuffix(), /*revision_number=*/2, + "system_tray" + GetScreenshotNameSuffix(), /*revision_number=*/1, system_tray)); } @@ -149,7 +149,7 @@ TEST_P(StatusAreaParameterizedPixelTest, DateTrayTest) { date_tray->SetIsActive(IsActive()); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "date_tray" + GetScreenshotNameSuffix(), /*revision_number=*/2, + "date_tray" + GetScreenshotNameSuffix(), /*revision_number=*/1, date_tray)); } diff --git a/ash/system/time/calendar_view_pixeltest.cc b/ash/system/time/calendar_view_pixeltest.cc index c4a5728cbde8e4..04582bf22234d5 100644 --- a/ash/system/time/calendar_view_pixeltest.cc +++ b/ash/system/time/calendar_view_pixeltest.cc @@ -96,7 +96,7 @@ TEST_F(CalendarViewPixelTest, Basics) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "calendar_view", - /*revision_number=*/5, GetCalendarView())); + /*revision_number=*/3, GetCalendarView())); } TEST_F(CalendarViewPixelTest, EventList) { @@ -126,7 +126,7 @@ TEST_F(CalendarViewPixelTest, EventList) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "event_list_view", - /*revision_number=*/6, GetEventListView())); + /*revision_number=*/5, GetEventListView())); } } // namespace ash diff --git a/ash/system/toast/system_toast_view_pixeltest.cc b/ash/system/toast/system_toast_view_pixeltest.cc index 8321d6c12f0e62..6004a717552191 100644 --- a/ash/system/toast/system_toast_view_pixeltest.cc +++ b/ash/system/toast/system_toast_view_pixeltest.cc @@ -80,7 +80,7 @@ TEST_F(SystemToastViewPixelTest, TextOnly) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } TEST_F(SystemToastViewPixelTest, WithLeadingIcon) { @@ -92,7 +92,7 @@ TEST_F(SystemToastViewPixelTest, WithLeadingIcon) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } TEST_F(SystemToastViewPixelTest, WithButton) { @@ -104,7 +104,7 @@ TEST_F(SystemToastViewPixelTest, WithButton) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } TEST_F(SystemToastViewPixelTest, WithLeadingIconAndButton) { @@ -117,7 +117,7 @@ TEST_F(SystemToastViewPixelTest, WithLeadingIconAndButton) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } TEST_F(SystemToastViewPixelTest, Multiline_TextOnly) { @@ -129,7 +129,7 @@ TEST_F(SystemToastViewPixelTest, Multiline_TextOnly) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } TEST_F(SystemToastViewPixelTest, Multiline_WithLeadingIcon) { @@ -142,7 +142,7 @@ TEST_F(SystemToastViewPixelTest, Multiline_WithLeadingIcon) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } TEST_F(SystemToastViewPixelTest, Multiline_WithButton) { @@ -155,7 +155,7 @@ TEST_F(SystemToastViewPixelTest, Multiline_WithButton) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } TEST_F(SystemToastViewPixelTest, Multiline_WithLeadingIconAndButton) { @@ -169,7 +169,7 @@ TEST_F(SystemToastViewPixelTest, Multiline_WithLeadingIconAndButton) { std::make_unique(toast_data)); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( - "screenshot", /*revision_number=*/1, GetContentsView())); + "screenshot", /*revision_number=*/0, GetContentsView())); } } // namespace ash diff --git a/ash/system/unified/quick_settings_footer_pixeltest.cc b/ash/system/unified/quick_settings_footer_pixeltest.cc index 22010de8b17dac..d26a8eadc3a85a 100644 --- a/ash/system/unified/quick_settings_footer_pixeltest.cc +++ b/ash/system/unified/quick_settings_footer_pixeltest.cc @@ -70,7 +70,7 @@ TEST_F(QuickSettingsFooterPixelTest, FooterShouldBeRenderedCorrectly) { InitPowerStatusAndOpenBubble(); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "with_no_extra_button", - /*revision_number=*/4, GetFooter())); + /*revision_number=*/3, GetFooter())); CloseBubble(); // Regression test for b/293484037: The settings button is missing when @@ -79,7 +79,7 @@ TEST_F(QuickSettingsFooterPixelTest, FooterShouldBeRenderedCorrectly) { InitPowerStatusAndOpenBubble(); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "with_exit_button", - /*revision_number=*/4, GetFooter())); + /*revision_number=*/3, GetFooter())); CloseBubble(); } diff --git a/ash/system/video_conference/bubble/bubble_view_pixeltest.cc b/ash/system/video_conference/bubble/bubble_view_pixeltest.cc index 1574420dbeaa84..baef77dc776292 100644 --- a/ash/system/video_conference/bubble/bubble_view_pixeltest.cc +++ b/ash/system/video_conference/bubble/bubble_view_pixeltest.cc @@ -191,7 +191,7 @@ TEST_F(BubbleViewPixelTest, Basic) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_basic", - /*revision_number=*/5, bubble_view())); + /*revision_number=*/6, bubble_view())); } // Pixel test that tests toggled on/off and focused/not focused for the toggle @@ -215,14 +215,14 @@ TEST_F(BubbleViewPixelTest, ToggleButton) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_no_focus_not_toggled", - /*revision_number=*/7, toggle_effect_button_container)); + /*revision_number=*/8, toggle_effect_button_container)); // Toggle the first button, the UI should change. LeftClickOn(first_toggle_effect_button); ASSERT_EQ(1, office_bunny()->num_activations_for_testing()); EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_no_focus_toggled", - /*revision_number=*/6, toggle_effect_button_container)); + /*revision_number=*/5, toggle_effect_button_container)); // Un-toggle the button, then keyboard focus it. LeftClickOn(first_toggle_effect_button); @@ -234,7 +234,7 @@ TEST_F(BubbleViewPixelTest, ToggleButton) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_with_focus_not_toggled", - /*revision_number=*/7, toggle_effect_button_container)); + /*revision_number=*/8, toggle_effect_button_container)); // Re-toggle the button. event_generator->PressAndReleaseKey(ui::KeyboardCode::VKEY_RETURN); @@ -243,7 +243,7 @@ TEST_F(BubbleViewPixelTest, ToggleButton) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_with_focus_toggled", - /*revision_number=*/6, toggle_effect_button_container)); + /*revision_number=*/7, toggle_effect_button_container)); } // Pixel test that tests the expanded/collapsed state of the return to app panel @@ -310,7 +310,7 @@ TEST_F(BubbleViewPixelTest, ReturnToAppLinux) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_tray_linux_bubble_one_app", - /*revision_number=*/6, video_conference_tray()->GetBubbleView())); + /*revision_number=*/5, video_conference_tray()->GetBubbleView())); controller()->AddMediaApp(CreateFakeMediaApp( /*is_capturing_camera=*/true, /*is_capturing_microphone=*/true, @@ -325,7 +325,7 @@ TEST_F(BubbleViewPixelTest, ReturnToAppLinux) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_tray_linux_bubble_two_app", - /*revision_number=*/6, video_conference_tray()->GetBubbleView())); + /*revision_number=*/5, video_conference_tray()->GetBubbleView())); } TEST_F(BubbleViewPixelTest, OneToggleEffects) { @@ -339,7 +339,7 @@ TEST_F(BubbleViewPixelTest, OneToggleEffects) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_one_toggle_effect", - /*revision_number=*/5, GetToggleEffectsView())); + /*revision_number=*/6, GetToggleEffectsView())); } TEST_F(BubbleViewPixelTest, TwoToggleEffects) { @@ -354,7 +354,7 @@ TEST_F(BubbleViewPixelTest, TwoToggleEffects) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_two_toggle_effects", - /*revision_number=*/5, GetToggleEffectsView())); + /*revision_number=*/6, GetToggleEffectsView())); } TEST_F(BubbleViewPixelTest, ThreeToggleEffects) { @@ -374,7 +374,7 @@ TEST_F(BubbleViewPixelTest, ThreeToggleEffects) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "video_conference_bubble_view_three_toggle_effects", - /*revision_number=*/5, GetToggleEffectsView())); + /*revision_number=*/6, GetToggleEffectsView())); } } // namespace ash::video_conference diff --git a/ash/wm/wm_pixel_diff_test.cc b/ash/wm/wm_pixel_diff_test.cc index 6f2a4699e87907..276011be3f519b 100644 --- a/ash/wm/wm_pixel_diff_test.cc +++ b/ash/wm/wm_pixel_diff_test.cc @@ -160,7 +160,7 @@ TEST_F(WmPixelDiffTest, WindowCycleBasic) { EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen( "window_cycle_basic", - /*revision_number=*/11, widget)); + /*revision_number=*/12, widget)); } } // namespace ash diff --git a/cc/paint/render_surface_filters.cc b/cc/paint/render_surface_filters.cc index 9c28628d425c9e..160837bcd00f2d 100644 --- a/cc/paint/render_surface_filters.cc +++ b/cc/paint/render_surface_filters.cc @@ -187,29 +187,11 @@ sk_sp RenderSurfaceFilters::BuildImageFilter( GetContrastMatrix(op.amount(), matrix); image_filter = CreateMatrixImageFilter(matrix, std::move(image_filter)); break; - case FilterOperation::BLUR: { - // SkImageFilters::Blur requires a crop rect for well-defined tiling - // behavior when the blur_tile_mode() is not kDecal. When that is not - // kDecal, setting the crop to the provided layer bounds means that - // tile mode will be applied to the layer's pixels inside its bounds, - // but pixels outside its bounds will not be read. Its output will still - // be cropped to the layer bounds automatically. - // TODO(b/1451898): The software_renderer does not calculate correct - // layer bounds (it's always empty), so rely on the legacy clamp - // handling in Skia for now. Once software_renderer does provide layer - // bounds, FilterOperations::MapRect could be updated to reflect this - // cropping, since a clamped blur doesn't actually move pixels. - SkRect sk_layer_bounds = gfx::RectToSkRect(layer_bounds); - const PaintFilter::CropRect* crop_rect = nullptr; - if (!sk_layer_bounds.isEmpty() && - op.blur_tile_mode() != SkTileMode::kDecal) { - crop_rect = &sk_layer_bounds; - } - image_filter = sk_make_sp( - op.amount(), op.amount(), op.blur_tile_mode(), - std::move(image_filter), crop_rect); + case FilterOperation::BLUR: + image_filter = sk_make_sp(op.amount(), op.amount(), + op.blur_tile_mode(), + std::move(image_filter)); break; - } case FilterOperation::DROP_SHADOW: image_filter = sk_make_sp( SkIntToScalar(op.offset().x()), SkIntToScalar(op.offset().y()), diff --git a/components/test/data/viz/backdrop_filter_offset.png b/components/test/data/viz/backdrop_filter_offset.png index 9a2828b9062cac..0516f6cca40832 100644 Binary files a/components/test/data/viz/backdrop_filter_offset.png and b/components/test/data/viz/backdrop_filter_offset.png differ diff --git a/components/test/data/viz/backdrop_filter_offset_sw.png b/components/test/data/viz/backdrop_filter_offset_sw.png deleted file mode 100644 index 69385161f2dcee..00000000000000 Binary files a/components/test/data/viz/backdrop_filter_offset_sw.png and /dev/null differ diff --git a/components/test/data/viz/backdrop_filter_rotated_skia_gl.png b/components/test/data/viz/backdrop_filter_rotated_skia_gl.png index a3fe647ed6f603..81121ab2a37b86 100644 Binary files a/components/test/data/viz/backdrop_filter_rotated_skia_gl.png and b/components/test/data/viz/backdrop_filter_rotated_skia_gl.png differ diff --git a/components/test/data/viz/unit_tests_bundle_data.filelist b/components/test/data/viz/unit_tests_bundle_data.filelist index 375af2af596898..9ba17182ce19d1 100644 --- a/components/test/data/viz/unit_tests_bundle_data.filelist +++ b/components/test/data/viz/unit_tests_bundle_data.filelist @@ -31,7 +31,6 @@ //components/test/data/viz/backdrop_filter_masked.png //components/test/data/viz/backdrop_filter_masked_sw.png //components/test/data/viz/backdrop_filter_offset.png -//components/test/data/viz/backdrop_filter_offset_sw.png //components/test/data/viz/backdrop_filter_on_scaled_layer_skia_gl.png //components/test/data/viz/backdrop_filter_on_scaled_layer_skia_graphite.png //components/test/data/viz/backdrop_filter_on_scaled_layer_skia_vk.png diff --git a/components/viz/common/quads/render_pass_internal.h b/components/viz/common/quads/render_pass_internal.h index cc2bacdb3f806b..b5824876b86907 100644 --- a/components/viz/common/quads/render_pass_internal.h +++ b/components/viz/common/quads/render_pass_internal.h @@ -54,9 +54,7 @@ class VIZ_COMMON_EXPORT RenderPassInternal { // backdrop of the render pass, from behind it. cc::FilterOperations backdrop_filters; - // Clipping bounds for backdrop filter. If defined, is in a coordinate space - // equivalent to render pass physical pixels after applying - // `RenderPassDrawQuad::filter_scale`. + // Clipping bounds for backdrop filter. absl::optional backdrop_filter_bounds; // If false, the pixels in the render pass' texture are all opaque. diff --git a/components/viz/service/display/renderer_pixeltest.cc b/components/viz/service/display/renderer_pixeltest.cc index d1b518e451e069..42da134e9ea4b8 100644 --- a/components/viz/service/display/renderer_pixeltest.cc +++ b/components/viz/service/display/renderer_pixeltest.cc @@ -3802,18 +3802,10 @@ TEST_P(RendererPixelTestWithBackdropFilter, OffsetFilter) { backdrop_filters_.Append( cc::FilterOperation::CreateOffsetFilter(gfx::Point(5, 5))); SetUpRenderPassList(); - - // TODO(989329): See comment in - // LayerTreeHostFiltersPixelTest/BackdropFilterOffsetTest. The software - // compositor does not correctly apply clamping when accessing content outside - // of the layer. - base::FilePath expected_path( - is_software_renderer() - ? FILE_PATH_LITERAL("backdrop_filter_offset_sw.png") - : FILE_PATH_LITERAL("backdrop_filter_offset.png")); - - EXPECT_TRUE( - RunPixelTest(&pass_list_, expected_path, cc::ExactPixelComparator())); + EXPECT_TRUE(RunPixelTest( + &pass_list_, + base::FilePath(FILE_PATH_LITERAL("backdrop_filter_offset.png")), + cc::ExactPixelComparator())); } TEST_P(RendererPixelTestWithBackdropFilter, InvertFilter) { diff --git a/components/viz/service/display/skia_renderer.cc b/components/viz/service/display/skia_renderer.cc index 3aabb2983bc391..23b8c41e5c5f37 100644 --- a/components/viz/service/display/skia_renderer.cc +++ b/components/viz/service/display/skia_renderer.cc @@ -713,10 +713,7 @@ struct SkiaRenderer::DrawRPDQParams { mutable sk_sp sk_shader_ = nullptr; }; - DrawRPDQParams() : filter_bounds(SkRect::MakeEmpty()) {} - - explicit DrawRPDQParams(const gfx::RectF& visible_rect) - : filter_bounds(gfx::RectFToSkRect(visible_rect)) {} + explicit DrawRPDQParams(const gfx::RectF& visible_rect); // Root of the calculated image filter DAG to be applied to the render pass. sk_sp image_filter = nullptr; @@ -724,18 +721,15 @@ struct SkiaRenderer::DrawRPDQParams { // be that filter. |image_filter| will still be non-null. sk_sp color_filter = nullptr; // Root of the calculated backdrop filter DAG to be applied to the render pass - // Backdrop filtered content must be clipped to |backdrop_filter_bounds| and - // the DrawQuad's rect (or draw_region if BSP-clipped). sk_sp backdrop_filter = nullptr; // If present, the mask image, which can be applied using SkCanvas::clipShader - // in the RPDQ's coord space. + // in RPDQ's coord space. absl::optional mask_shader; // Backdrop border box for the render pass, to clip backdrop-filtered content - // (but not the rest of the RPDQ itself). - absl::optional backdrop_filter_bounds; + absl::optional backdrop_filter_bounds; // The content space bounds that includes any filtered extents. If empty, - // the draw can be skipped.It may represent fractional pixel coverage. - SkRect filter_bounds; + // the draw can be skipped. + gfx::Rect filter_bounds; // Multiplier used for downscaling backdrop filter. float backdrop_filter_quality = 1.0f; @@ -761,29 +755,6 @@ struct SkiaRenderer::DrawRPDQParams { return !bypass_geometry->clip_rect.Contains( gfx::SkRectToRectF(content_bounds)); } - - // Returns either |params->visible_rect| or |bypass_geometry->clip_rect|, - // which corresponds to the visible_rect of the originating RPDQ. - SkRect GetContentBounds(const DrawQuadParams* params) const { - return gfx::RectFToSkRect(bypass_geometry ? bypass_geometry->clip_rect - : params->visible_rect); - } - - // Sets a clip on the canvas to restrict the size of the Skia layer that holds - // the backdrop filtered content to the size of the DrawQuad. When possible - // this is an exact clip to reduce operations performed within the backdrop - // layer; otherwise it's conservative to constrain size without impacting - // visuals. - void SetBackdropFilterClip(SkCanvas* canvas, - const DrawQuadParams* params) const; - - // Erases backdrop filtered content outside of the DrawQuad and backdrop - // filter bounds rrect within the backdrop layer. This is a no-op if exact - // clipping was used in SetBackdropFilterClip to achieve the same effect. - // Otherwise, this is necessary to limit the backdrop content without - // impacting the DrawQuad or regular filter output. - void ClearOutsideBackdropBounds(SkCanvas* canvas, - const DrawQuadParams* params) const; }; sk_sp SkiaRenderer::DrawRPDQParams::MaskShader::GetOrCreateSkShader( @@ -804,72 +775,8 @@ sk_sp SkiaRenderer::DrawRPDQParams::MaskShader::GetOrCreateSkShader( return sk_shader_; } -void SkiaRenderer::DrawRPDQParams::SetBackdropFilterClip( - SkCanvas* canvas, - const DrawQuadParams* params) const { - if (!backdrop_filter) { - return; // No clipping necessary - } - - const bool aa = params->aa_flags != SkCanvas::kNone_QuadAAFlags; - if (backdrop_filter_bounds) { - // The final backdrop content is complex, so excess filter values will be - // erased from within the layer. Only clip to the aggregate bounds. - canvas->clipRect(filter_bounds, aa); - } else { - // The backdrop content is restricted to the draw region and visible_rect - // (or bypass clip_rect, which corresponds to the visible_rect of the quad - // that had the filter on it). - canvas->clipRect(GetContentBounds(params), aa); - } - if (params->draw_region) { - SkPath clip_path = params->draw_region_in_path(); - if (bypass_geometry) { - clip_path.transform(bypass_geometry->transform); - } - canvas->clipPath(clip_path, aa); - } -} - -void SkiaRenderer::DrawRPDQParams::ClearOutsideBackdropBounds( - SkCanvas* canvas, - const DrawQuadParams* params) const { - if (!backdrop_filter || !backdrop_filter_bounds) { - return; // Nothing to clear within the layer - } - - // Must erase pixels not in the intersection of the backdrop_filter_bounds, - // visible_rect, and any draw_region. This is the union of the inverse fills - // of those shapes, which can be accomplished most efficiently by clipping - // the shape with the kDifference op and then clearing the canvas, per shape - const bool aa = params->aa_flags != SkCanvas::kNone_QuadAAFlags; - - canvas->save(); - canvas->clipRRect(*backdrop_filter_bounds, SkClipOp::kDifference, aa); - canvas->clear(SK_ColorTRANSPARENT); - canvas->restore(); - - if (params->draw_region) { - canvas->save(); - canvas->concat(bypass_geometry->transform); - canvas->clipPath(params->draw_region_in_path(), SkClipOp::kDifference, aa); - canvas->clear(SK_ColorTRANSPARENT); - canvas->restore(); - } else { - SkRect content = GetContentBounds(params); - if (!content.contains(backdrop_filter_bounds->rect())) { - // If the |draw_region| is defined, it's already a subset of |rect|, so - // we don't have to clear both. Similarly, if |backdrop_filter_bounds| - // is contained within the quad, the first clear was sufficient. - // Otherwise, have some excess backdrop content that must still be - // erased. - canvas->save(); - canvas->clipRect(content, SkClipOp::kDifference, aa); - canvas->clear(SK_ColorTRANSPARENT); - canvas->restore(); - } - } -} +SkiaRenderer::DrawRPDQParams::DrawRPDQParams(const gfx::RectF& visible_rect) + : filter_bounds(gfx::ToEnclosingRect(visible_rect)) {} // A read lock based fence that is signaled after gpu commands are completed // meaning the resource has been read. @@ -1532,10 +1439,11 @@ void SkiaRenderer::PrepareGradient( void SkiaRenderer::PrepareCanvasForRPDQ(const DrawRPDQParams& rpdq_params, DrawQuadParams* params) { - // Clip before the saveLayer() so that Skia only filters the backdrop that is - // necessary for the |backdrop_filter_bounds| (otherwise it will fill the - // quad's SharedQuadState's |clip_rect|). - rpdq_params.SetBackdropFilterClip(current_canvas_, params); + // Clip to the filter bounds prior to saving the layer, which has been + // constructed to contain the actual filtered contents (visually no + // clipping effect, but lets Skia minimize internal layer size). + bool aa = params->aa_flags != SkCanvas::kNone_QuadAAFlags; + current_canvas_->clipRect(gfx::RectToSkRect(rpdq_params.filter_bounds), aa); SkPaint layer_paint = params->paint(nullptr /* color_filter */); // The layer always consumes the opacity, but its blend mode depends on if @@ -1553,15 +1461,86 @@ void SkiaRenderer::PrepareCanvasForRPDQ(const DrawRPDQParams& rpdq_params, layer_paint.setImageFilter(rpdq_params.image_filter); } - SkRect bounds = rpdq_params.GetContentBounds(params); + // Canocalize the backdrop bounds rrect type; if there's no backdrop filter or + // filter bounds, this will be empty. If it's a rect or rrect, we must work + // around Skia's background filter auto-expansion. If it's an rrect, we must + // also clear out the rounded corners after filtering. + gfx::RRectF::Type backdrop_bounds_type = gfx::RRectF::Type::kEmpty; + if (rpdq_params.backdrop_filter && + rpdq_params.backdrop_filter_bounds.has_value()) { + backdrop_bounds_type = rpdq_params.backdrop_filter_bounds->GetType(); + } + + // Initially the backdrop filter fills the entire rect; if we draw less than + // that we need to clear the excess. + bool post_backdrop_filter_clear_needed = !!params->draw_region; + + // Explicitly crop the input and the output to the backdrop bounds; this is + // required for the backdrop-filter spec. + sk_sp backdrop_filter = rpdq_params.backdrop_filter; + if (backdrop_bounds_type != gfx::RRectF::Type::kEmpty) { + DCHECK(backdrop_filter); + + gfx::RectF crop_rect = rpdq_params.backdrop_filter_bounds->rect(); + + // Only sample from pixels behind the RPDQ for backdrop filters to avoid + // color bleeding with pixel-moving filters. + crop_rect.Intersect(params->rect); + + SkIRect sk_crop_rect = gfx::RectToSkIRect(gfx::ToEnclosingRect(crop_rect)); + + SkIRect sk_src_rect = backdrop_filter->filterBounds( + sk_crop_rect, SkMatrix::I(), SkImageFilter::kReverse_MapDirection, + &sk_crop_rect); + if (sk_crop_rect == sk_src_rect) { + // The backdrop filter does not "move" pixels, i.e. a pixel's value only + // depends on its (x,y) and prior color. Avoid cropping the input in this + // case since composing a crop rect into the filter DAG forces Skia to + // map the backdrop content into the local space, which can introduce + // filtering artifacts: crbug.com/1044032. Instead just post-filter + // clearing will achieve the same cropping of the output at higher quality + post_backdrop_filter_clear_needed = true; + } else { + // Offsetting (0,0) does nothing to the actual image, but is the most + // convenient way to embed the crop rect into the filter DAG. + // TODO(michaelludwig) - Remove this once Skia doesn't always auto-expand + sk_sp crop = + SkImageFilters::Offset(0.0f, 0.0f, nullptr, &sk_crop_rect); + backdrop_filter = SkImageFilters::Compose( + crop, SkImageFilters::Compose(std::move(backdrop_filter), crop)); + // Update whether or not a post-filter clear is needed (crop didn't + // completely match bounds) + post_backdrop_filter_clear_needed |= + backdrop_bounds_type != gfx::RRectF::Type::kRect || + crop_rect != rpdq_params.backdrop_filter_bounds->rect(); + } + } + + SkRect bounds = gfx::RectFToSkRect( + rpdq_params.bypass_geometry ? rpdq_params.bypass_geometry->clip_rect + : params->visible_rect); current_canvas_->saveLayer(SkCanvasPriv::ScaledBackdropLayer( - &bounds, &layer_paint, rpdq_params.backdrop_filter.get(), + &bounds, &layer_paint, backdrop_filter.get(), rpdq_params.backdrop_filter_quality, 0)); - // If we have backdrop filtered content (and not transparent black like with // regular render passes), we have to clear out the parts of the layer that - // shouldn't show the backdrop. - rpdq_params.ClearOutsideBackdropBounds(current_canvas_, params); + // shouldn't show the backdrop + if (backdrop_filter && post_backdrop_filter_clear_needed) { + current_canvas_->save(); + if (rpdq_params.backdrop_filter_bounds.has_value()) { + current_canvas_->clipRRect(SkRRect(*rpdq_params.backdrop_filter_bounds), + SkClipOp::kDifference, aa); + } + if (params->draw_region) { + SkPath clip_path = params->draw_region_in_path(); + if (rpdq_params.bypass_geometry) { + clip_path.transform(rpdq_params.bypass_geometry->transform); + } + current_canvas_->clipPath(clip_path, SkClipOp::kDifference, aa); + } + current_canvas_->clear(SK_ColorTRANSPARENT); + current_canvas_->restore(); + } } void SkiaRenderer::PreparePaintOrCanvasForRPDQ( @@ -2980,72 +2959,67 @@ SkiaRenderer::DrawRPDQParams SkiaRenderer::CalculateRPDQParams( return rpdq_params; } - // Calculate local matrix that's shared by filters and backdrop_filters. This - // local matrix represents the UI display scale that's already been applied to - // the DrawQuads but not any geometric properties of the filters. + // Calculate local matrix that's shared by filters and backdrop_filters SkMatrix local_matrix; local_matrix.setTranslate(quad->filters_origin.x(), quad->filters_origin.y()); local_matrix.postScale(quad->filters_scale.x(), quad->filters_scale.y()); - // Calculate local clip bounds. Note that doing this here is redundant with - // letting SkCanvas reject clipped-out draws, but detecting it early means - // SkiaRenderer does not have to prepare renderpass backings in those cases. - absl::optional local_clip_rect; - if (!params->content_device_transform.HasPerspective() && - params->content_device_transform.IsInvertible()) { - // The |clip_rect| and |current_draw_rect_| are in target space, so map - // to device space, then inverse-map to quad space. - const auto& target_to_device = current_frame()->target_to_device_transform; - gfx::RectF clip_rect = target_to_device.MapRect(gfx::RectF( - quad->shared_quad_state->clip_rect.value_or(current_draw_rect_))); - local_clip_rect = gfx::RectFToSkRect( - cc::MathUtil::InverseMapQuadToLocalSpace( - params->content_device_transform, gfx::QuadF(clip_rect)) - .BoundingBox()); - } - - auto to_sk_image_filter = - [](sk_sp paint_filter, - const SkMatrix& local_matrix) -> sk_sp { - if (paint_filter && paint_filter->cached_sk_filter_) { - return paint_filter->cached_sk_filter_->makeWithLocalMatrix(local_matrix); - } else { - return nullptr; - } - }; - // Convert CC image filters into a SkImageFilter root node if (filters) { DCHECK(!filters->IsEmpty()); auto paint_filter = cc::RenderSurfaceFilters::BuildImageFilter(*filters); - rpdq_params.image_filter = - to_sk_image_filter(std::move(paint_filter), local_matrix); + auto sk_filter = paint_filter ? paint_filter->cached_sk_filter_ : nullptr; - if (rpdq_params.image_filter) { - // Update the filter bounds to account for how the image filters - // grow or move the area touched by the base quad. - rpdq_params.filter_bounds = rpdq_params.image_filter->computeFastBounds( - rpdq_params.filter_bounds); + if (sk_filter) { + // Update the filter bounds based to account for how the image filters + // grow or expand the area touched by drawing. + rpdq_params.filter_bounds = + filters->MapRect(rpdq_params.filter_bounds, local_matrix); // If after applying the filter we would be clipped out, skip the draw. - if (local_clip_rect) { - if (!rpdq_params.filter_bounds.intersect(*local_clip_rect)) { - return {}; - } + gfx::Rect clip_rect = + quad->shared_quad_state->clip_rect.value_or(current_draw_rect_); + gfx::Transform transform = + quad->shared_quad_state->quad_to_target_transform; + transform.Flatten(); + if (!transform.IsInvertible()) { + return rpdq_params; + } + + // If the transform has perspective, there might be visible content + // outside of the bounds of the quad. + if (!transform.HasPerspective()) { + gfx::QuadF clip_quad = gfx::QuadF(gfx::RectF(clip_rect)); + gfx::QuadF local_clip = + cc::MathUtil::InverseMapQuadToLocalSpace(transform, clip_quad); + + rpdq_params.filter_bounds.Intersect( + gfx::ToEnclosingRect(local_clip.BoundingBox())); } + // If we've been fully clipped out (by crop rect or clipping), there's + // nothing to draw. + if (rpdq_params.filter_bounds.IsEmpty()) { + return rpdq_params; + } + + rpdq_params.image_filter = sk_filter->makeWithLocalMatrix(local_matrix); + // Attempt to simplify the image filter to a color filter, which enables // the RPDQ effects to be applied more efficiently. SkColorFilter* color_filter_ptr = nullptr; - if (rpdq_params.image_filter->asAColorFilter(&color_filter_ptr)) { - // asAColorFilter already ref'ed the filter when true is returned, - // reset() does not add a ref itself, so everything is okay. - rpdq_params.color_filter.reset(color_filter_ptr); + if (rpdq_params.image_filter) { + if (rpdq_params.image_filter->asAColorFilter(&color_filter_ptr)) { + // asAColorFilter already ref'ed the filter when true is returned, + // reset() does not add a ref itself, so everything is okay. + rpdq_params.color_filter.reset(color_filter_ptr); + } } } } // Convert CC image filters for the backdrop into a SkImageFilter root node + // TODO(weiliangc): ChromeOS would need backdrop_filter_quality implemented if (backdrop_filters) { DCHECK(!backdrop_filters->IsEmpty()); rpdq_params.backdrop_filter_quality = quad->backdrop_filter_quality; @@ -3063,84 +3037,44 @@ SkiaRenderer::DrawRPDQParams SkiaRenderer::CalculateRPDQParams( auto bg_paint_filter = cc::RenderSurfaceFilters::BuildImageFilter( *backdrop_filters, gfx::SkIRectToRect(filter_rect)); - rpdq_params.backdrop_filter = - to_sk_image_filter(std::move(bg_paint_filter), local_matrix); + auto sk_bg_filter = + bg_paint_filter ? bg_paint_filter->cached_sk_filter_ : nullptr; + + if (sk_bg_filter) { + rpdq_params.backdrop_filter = + sk_bg_filter->makeWithLocalMatrix(local_matrix); + } } } - // Determine the clipping to apply to the backdrop filter. Skia normally - // fills layers with the backdrop content, whereas viz wants the backdrop - // content restricted to the intersection of the DrawQuad and any defined - // |backdrop_filter_bounds|. + // Determine if the backdrop filter has its own clip (which only needs to be + // checked when we have a backdrop filter to apply) if (rpdq_params.backdrop_filter) { - SkRect backdrop_rect = gfx::RectFToSkRect(params->visible_rect); - // Pass bounds do not match the display scale; they will be scaled and - // converted into an SkRRect in |backdrop_filter_bounds| if defined. - absl::optional pass_bounds = + const absl::optional backdrop_filter_bounds = BackdropFilterBoundsForPass(quad->render_pass_id); - absl::optional backdrop_filter_bounds; - if (pass_bounds) { - // Scale by the filter's scale, but don't apply filter origin - SkRRect result; - if (!SkRRect(*pass_bounds).transform(local_matrix, &result) || - !backdrop_rect.intersect(result.rect())) { - // No visible backdrop filter + if (backdrop_filter_bounds) { + // The backdrop filters effect will be cropped by these bounds. If the + // bounds are empty, discard the backdrop filter now since none of it + // would have been visible anyways. + if (backdrop_filter_bounds->IsEmpty()) { rpdq_params.backdrop_filter = nullptr; - return rpdq_params; } else { - backdrop_filter_bounds = result; - } - - if (backdrop_filter_bounds->contains(rpdq_params.filter_bounds)) { - // The backdrop filter bounds are a no-op since the quad rect or region - // fully limits the backdrop filter. - backdrop_filter_bounds.reset(); - } else { - // The backdrop filter bounds might have an effect, but a simple case to - // check for is if the backdrop rounded corners are identical to the - // quad's rounded corner mask info. In that case, the prior contains() - // check would be false, but we can still discard these bounds since the - // final mask clip will achieve the same visual effect. - if (params->mask_filter_info) { - SkMatrix m = gfx::TransformToFlattenedSkMatrix( - params->content_device_transform); - if (backdrop_filter_bounds->transform(m, &result) && - SkRRect(params->mask_filter_info->rounded_corner_bounds()) == - result) { - backdrop_filter_bounds.reset(); - } + rpdq_params.backdrop_filter_bounds = *backdrop_filter_bounds; + // Scale by the filter's scale, but don't apply filter origin + rpdq_params.backdrop_filter_bounds->Scale(quad->filters_scale.x(), + quad->filters_scale.y()); + + // If there are also regular image filters, they apply to the area of + // the backdrop_filter_bounds too, so expand the backdrop bounds and + // join it with the main filter bounds. + if (rpdq_params.image_filter) { + gfx::Rect backdrop_rect = + gfx::ToEnclosingRect(rpdq_params.backdrop_filter_bounds->rect()); + rpdq_params.filter_bounds.Union( + filters->MapRect(backdrop_rect, local_matrix)); } } } - - if (local_clip_rect && !backdrop_rect.intersect(*local_clip_rect)) { - rpdq_params.backdrop_filter = nullptr; - return rpdq_params; - } - - // Besides ensuring the output of the backdrop filter doesn't go beyond its - // bounds, it should not read pixels outside of its bounds to prevent color - // bleeding. If it's a pixel-moving filter, we compose a kClamp-tiling Crop - // image filter to enforce this requirement. - // TODO(crbug.com/978031): Revisit the tilemode, perhaps kMirror is better - SkIRect sk_crop_rect = backdrop_rect.roundOut(); - SkIRect sk_src_rect = rpdq_params.backdrop_filter->filterBounds( - sk_crop_rect, SkMatrix::I(), SkImageFilter::kReverse_MapDirection, - /*inputRect=*/nullptr); - if (!sk_crop_rect.contains(sk_src_rect)) { - rpdq_params.backdrop_filter = SkImageFilters::Compose( - /*outer=*/std::move(rpdq_params.backdrop_filter), - /*inner=*/SkImageFilters::Crop(backdrop_rect, SkTileMode::kClamp, - nullptr)); - } - - // Update |filter_bounds| to include content produced by the backdrop. Under - // most circumstances this will be a no-op since content is restricted to - // underneath the RPDQ's draw region, but if a backdrop filter is combined - // with some pixel-moving filters, that may not remain the case and this - // ensures |filter_bounds| will contain all possible output. - rpdq_params.filter_bounds.join(backdrop_rect); - rpdq_params.backdrop_filter_bounds = backdrop_filter_bounds; } return rpdq_params; @@ -3157,9 +3091,8 @@ void SkiaRenderer::DrawRenderPassQuad( // |filter_bounds| is the content space bounds that includes any filtered // extents. If empty, the draw can be skipped. - if (rpdq_params.filter_bounds.isEmpty()) { + if (rpdq_params.filter_bounds.IsEmpty()) return; - } auto bypass = render_pass_bypass_quads_.find(quad->render_pass_id); // When Render Pass has a single quad inside we would draw that directly. @@ -3684,14 +3617,12 @@ void SkiaRenderer::PrepareRenderPassOverlay( rpdq_params = CalculateRPDQParams(quad, ¶ms); } - const gfx::Rect filter_bounds = - gfx::SkIRectToRect(rpdq_params.filter_bounds.roundOut()); + const auto& filter_bounds = rpdq_params.filter_bounds; // |filter_bounds| is the content space bounds that includes any filtered // extents. If empty, the draw can be skipped. - if (filter_bounds.IsEmpty()) { + if (filter_bounds.IsEmpty()) return; - } SharedImageFormat buffer_format; gfx::ColorSpace color_space;