Bug 1699937 - Don't draw focus outlines for box-shadow in the non-nat…

…ive theme. r=mstange This is a relatively easy way to improve the rendering with the non-native theme while not regressing anything. In the future, once non-native-theme is shipped and default everywhere, I think we could provide something like: bool nsITheme::GetShadowRect(nsIFrame*, StyleAppearance, LayoutDeviceRect&, RectCornerRadii&) or such, where we can provide a precise rect + radii, and we would avoid painting the shadow if the function returned false. That would allow us to remove the native theme box shadow code path, and avoid WR fallback, all at once. Differential Revision: https://phabricator.services.mozilla.com/D109209
iaxat · Mar 20, 2021 · 4cf5ef7 · 4cf5ef7
1 parent 2ccf297
commit 4cf5ef7
Show file tree

Hide file tree

Showing 15 changed files with 45 additions and 22 deletions.
diff --git a/gfx/src/nsITheme.h b/gfx/src/nsITheme.h
@@ -76,11 +76,16 @@ class nsITheme : public nsISupports {
    * @param aWidgetType the -moz-appearance value to draw
    * @param aRect the rectangle defining the area occupied by the widget
    * @param aDirtyRect the rectangle that needs to be drawn
+   * @param DrawOverflow whether outlines, shadows and other such overflowing
+   *        things should be drawn. Honoring this creates better results for
+   *        box-shadow, though it's not a hard requirement.
    */
+  enum class DrawOverflow { No, Yes };
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                   StyleAppearance aWidgetType,
                                   const nsRect& aRect,
-                                  const nsRect& aDirtyRect) = 0;
+                                  const nsRect& aDirtyRect,
+                                  DrawOverflow = DrawOverflow::Yes) = 0;
 
   /**
    * Create WebRender commands for the theme background.

diff --git a/layout/painting/nsCSSRendering.cpp b/layout/painting/nsCSSRendering.cpp
@@ -1520,7 +1520,7 @@ void nsCSSRendering::PaintBoxShadowOuter(nsPresContext* aPresContext,
       nativeRect.IntersectRect(frameRect, nativeRect);
       aPresContext->Theme()->DrawWidgetBackground(
           shadowContext, aForFrame, styleDisplay->EffectiveAppearance(),
-          aFrameArea, nativeRect);
+          aFrameArea, nativeRect, nsITheme::DrawOverflow::No);
 
       blurringArea.DoPaint();
       aRenderingContext.Restore();

diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp
@@ -5117,12 +5117,13 @@ bool nsDisplayBoxShadowOuter::CanBuildWebRenderDisplayItems() {
   }
 
   bool hasBorderRadius;
-  bool nativeTheme =
-      nsCSSRendering::HasBoxShadowNativeTheme(mFrame, hasBorderRadius);
-
   // We don't support native themed things yet like box shadows around
   // input buttons.
-  if (nativeTheme) {
+  //
+  // TODO(emilio): The non-native theme could provide the right rect+radius
+  // instead relatively painlessly, if we find this causes performance issues or
+  // what not.
+  if (nsCSSRendering::HasBoxShadowNativeTheme(mFrame, hasBorderRadius)) {
     return false;
   }
 

diff --git a/widget/android/nsNativeThemeAndroid.cpp b/widget/android/nsNativeThemeAndroid.cpp
@@ -479,7 +479,8 @@ nsNativeThemeAndroid::DrawWidgetBackground(gfxContext* aContext,
                                            nsIFrame* aFrame,
                                            StyleAppearance aAppearance,
                                            const nsRect& aRect,
-                                           const nsRect& /* aDirtyRect */) {
+                                           const nsRect& /* aDirtyRect */,
+                                           DrawOverflow) {
   DrawTarget* dt = aContext->GetDrawTarget();
   const nscoord twipsPerPixel = aFrame->PresContext()->AppUnitsPerDevPixel();
   EventStates eventState = GetContentState(aFrame, aAppearance);

diff --git a/widget/android/nsNativeThemeAndroid.h b/widget/android/nsNativeThemeAndroid.h
@@ -20,7 +20,8 @@ class nsNativeThemeAndroid : private nsNativeTheme, public nsITheme {
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                   StyleAppearance aAppearance,
                                   const nsRect& aRect,
-                                  const nsRect& aDirtyRect) override;
+                                  const nsRect& aDirtyRect,
+                                  DrawOverflow) override;
   /*bool CreateWebRenderCommandsForWidget(mozilla::wr::DisplayListBuilder&
      aBuilder, mozilla::wr::IpcResourceUpdateQueue& aResources, const
      mozilla::layers::StackingContextHelper& aSc,

diff --git a/widget/cocoa/nsNativeThemeCocoa.h b/widget/cocoa/nsNativeThemeCocoa.h
@@ -315,7 +315,8 @@ class nsNativeThemeCocoa : private nsNativeTheme, public nsITheme {
   // The nsITheme interface.
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                   StyleAppearance aAppearance, const nsRect& aRect,
-                                  const nsRect& aDirtyRect) override;
+                                  const nsRect& aDirtyRect,
+                                  DrawOverflow) override;
   bool CreateWebRenderCommandsForWidget(mozilla::wr::DisplayListBuilder& aBuilder,
                                         mozilla::wr::IpcResourceUpdateQueue& aResources,
                                         const mozilla::layers::StackingContextHelper& aSc,

diff --git a/widget/cocoa/nsNativeThemeCocoa.mm b/widget/cocoa/nsNativeThemeCocoa.mm
@@ -2646,7 +2646,8 @@ static bool IsHiDPIContext(nsDeviceContext* aContext) {
 NS_IMETHODIMP
 nsNativeThemeCocoa::DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                          StyleAppearance aAppearance, const nsRect& aRect,
-                                         const nsRect& aDirtyRect) {
+                                         const nsRect& aDirtyRect,
+                                         DrawOverflow) {
   NS_OBJC_BEGIN_TRY_BLOCK_RETURN;
 
   Maybe<WidgetInfo> widgetInfo = ComputeWidgetInfo(aFrame, aAppearance, aRect);

diff --git a/widget/gtk/nsNativeThemeGTK.cpp b/widget/gtk/nsNativeThemeGTK.cpp
@@ -1093,7 +1093,8 @@ NS_IMETHODIMP
 nsNativeThemeGTK::DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                        StyleAppearance aAppearance,
                                        const nsRect& aRect,
-                                       const nsRect& aDirtyRect) {
+                                       const nsRect& aDirtyRect,
+                                       DrawOverflow) {
   GtkWidgetState state;
   WidgetNodeType gtkWidgetType;
   GtkTextDirection direction = GetTextDirection(aFrame);

diff --git a/widget/gtk/nsNativeThemeGTK.h b/widget/gtk/nsNativeThemeGTK.h
@@ -28,7 +28,8 @@ class nsNativeThemeGTK final : private nsNativeTheme,
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                   StyleAppearance aAppearance,
                                   const nsRect& aRect,
-                                  const nsRect& aDirtyRect) override;
+                                  const nsRect& aDirtyRect,
+                                  DrawOverflow) override;
 
   bool CreateWebRenderCommandsForWidget(
       mozilla::wr::DisplayListBuilder& aBuilder,

diff --git a/widget/headless/HeadlessThemeGTK.cpp b/widget/headless/HeadlessThemeGTK.cpp
@@ -18,7 +18,8 @@ NS_IMETHODIMP
 HeadlessThemeGTK::DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                        StyleAppearance aAppearance,
                                        const nsRect& aRect,
-                                       const nsRect& aDirtyRect) {
+                                       const nsRect& aDirtyRect,
+                                       DrawOverflow) {
   return NS_OK;
 }
 

diff --git a/widget/headless/HeadlessThemeGTK.h b/widget/headless/HeadlessThemeGTK.h
@@ -20,7 +20,8 @@ class HeadlessThemeGTK final : private nsNativeTheme, public nsITheme {
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                   StyleAppearance aAppearance,
                                   const nsRect& aRect,
-                                  const nsRect& aDirtyRect) override;
+                                  const nsRect& aDirtyRect,
+                                  DrawOverflow) override;
 
   [[nodiscard]] LayoutDeviceIntMargin GetWidgetBorder(
       nsDeviceContext* aContext, nsIFrame* aFrame,

diff --git a/widget/nsNativeBasicTheme.cpp b/widget/nsNativeBasicTheme.cpp
@@ -1580,9 +1580,10 @@ NS_IMETHODIMP
 nsNativeBasicTheme::DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                          StyleAppearance aAppearance,
                                          const nsRect& aRect,
-                                         const nsRect& /* aDirtyRect */) {
+                                         const nsRect& /* aDirtyRect */,
+                                         DrawOverflow aDrawOverflow) {
   if (!DoDrawWidgetBackground(*aContext->GetDrawTarget(), aFrame, aAppearance,
-                              aRect)) {
+                              aRect, aDrawOverflow)) {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
   return NS_OK;
@@ -1598,7 +1599,7 @@ bool nsNativeBasicTheme::CreateWebRenderCommandsForWidget(
     return false;
   }
   WebRenderBackendData data{aBuilder, aResources, aSc, aManager};
-  return DoDrawWidgetBackground(data, aFrame, aAppearance, aRect);
+  return DoDrawWidgetBackground(data, aFrame, aAppearance, aRect, DrawOverflow::Yes);
 }
 
 static LayoutDeviceRect ToSnappedRect(const nsRect& aRect,
@@ -1626,7 +1627,8 @@ template <typename PaintBackendData>
 bool nsNativeBasicTheme::DoDrawWidgetBackground(PaintBackendData& aPaintData,
                                                 nsIFrame* aFrame,
                                                 StyleAppearance aAppearance,
-                                                const nsRect& aRect) {
+                                                const nsRect& aRect,
+                                                DrawOverflow aDrawOverflow) {
   static_assert(std::is_same_v<PaintBackendData, DrawTarget> ||
                 std::is_same_v<PaintBackendData, WebRenderBackendData>);
 
@@ -1649,6 +1651,10 @@ bool nsNativeBasicTheme::DoDrawWidgetBackground(PaintBackendData& aPaintData,
     }
   }
 
+  if (aDrawOverflow == DrawOverflow::No) {
+    eventState &= ~NS_EVENT_STATE_FOCUSRING;
+  }
+
   // Hack to avoid skia fuzziness: Add a dummy clip if the widget doesn't
   // overflow devPxRect.
   Maybe<AutoClipRect> maybeClipRect;

diff --git a/widget/nsNativeBasicTheme.h b/widget/nsNativeBasicTheme.h
@@ -113,7 +113,8 @@ class nsNativeBasicTheme : protected nsNativeTheme, public nsITheme {
   // The nsITheme interface.
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame*,
                                   StyleAppearance, const nsRect& aRect,
-                                  const nsRect& aDirtyRect) override;
+                                  const nsRect& aDirtyRect,
+                                  DrawOverflow) override;
 
   struct WebRenderBackendData {
     mozilla::wr::DisplayListBuilder& mBuilder;
@@ -135,7 +136,7 @@ class nsNativeBasicTheme : protected nsNativeTheme, public nsITheme {
   // given back-end.
   template <typename PaintBackendData>
   bool DoDrawWidgetBackground(PaintBackendData&, nsIFrame*, StyleAppearance,
-                              const nsRect& aRect);
+                              const nsRect&, DrawOverflow);
 
   [[nodiscard]] LayoutDeviceIntMargin GetWidgetBorder(nsDeviceContext* aContext,
                                                       nsIFrame*,

diff --git a/widget/windows/nsNativeThemeWin.cpp b/widget/windows/nsNativeThemeWin.cpp
@@ -1479,7 +1479,8 @@ NS_IMETHODIMP
 nsNativeThemeWin::DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                        StyleAppearance aAppearance,
                                        const nsRect& aRect,
-                                       const nsRect& aDirtyRect) {
+                                       const nsRect& aDirtyRect,
+                                       DrawOverflow) {
   if (IsWidgetScrollbarPart(aAppearance)) {
     if (MayDrawCustomScrollbarPart(aContext, aFrame, aAppearance, aRect,
                                    aDirtyRect)) {

diff --git a/widget/windows/nsNativeThemeWin.h b/widget/windows/nsNativeThemeWin.h
@@ -33,7 +33,8 @@ class nsNativeThemeWin : private nsNativeTheme, public nsITheme {
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
                                   StyleAppearance aAppearance,
                                   const nsRect& aRect,
-                                  const nsRect& aDirtyRect) override;
+                                  const nsRect& aDirtyRect,
+                                  DrawOverflow) override;
 
   [[nodiscard]] LayoutDeviceIntMargin GetWidgetBorder(
       nsDeviceContext* aContext, nsIFrame* aFrame,