Bug 1673121 - Simplify nsNativeBasicTheme scrollbar painting methods. r=spohl
authorMarkus Stange <mstange.moz@gmail.com>
Tue, 27 Oct 2020 15:27:31 +0000
changeset 554702 d00aefbdfbd767da9c085aba3d5991bdaef71d14
parent 554701 53a3638bf4ef14d017e8c8877135f11af794fbc4
child 554703 c586fc967b88780319e11b598db03ff7f7e9b63f
push id37898
push userabutkovits@mozilla.com
push dateWed, 28 Oct 2020 09:24:21 +0000
treeherdermozilla-central@83bf4fd3b1fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersspohl
bugs1673121
milestone84.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1673121 - Simplify nsNativeBasicTheme scrollbar painting methods. r=spohl This removes some duplication and replaces it with a bool aHorizontal parameter. It also splits out scroll corner drawing into its own method. And it consistently passes down the DPI ratio into all methods. Drive-by fixes: - The border of horizontal scrollbars in the default theme now scales with the DPI ratio. - The border is device-pixel aligned by insetting the stroked rectangle by half the stroke width. This patch also changes the scroll corner drawing in the default theme to no longer draw a border on the left. We can revert that if needed Differential Revision: https://phabricator.services.mozilla.com/D94662
widget/gtk/nsNativeBasicThemeGTK.cpp
widget/gtk/nsNativeBasicThemeGTK.h
widget/nsNativeBasicTheme.cpp
widget/nsNativeBasicTheme.h
--- a/widget/gtk/nsNativeBasicThemeGTK.cpp
+++ b/widget/gtk/nsNativeBasicThemeGTK.cpp
@@ -84,43 +84,35 @@ nsNativeBasicThemeGTK::GetMinimumWidgetS
     default:
       break;
   }
 
   *aIsOverridable = true;
   return NS_OK;
 }
 
-void nsNativeBasicThemeGTK::PaintScrollbarthumbHorizontal(
-    DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-    const EventStates& aElementState, const EventStates& aDocumentState) {
+void nsNativeBasicThemeGTK::PaintScrollbarThumb(
+    DrawTarget* aDrawTarget, const Rect& aRect, bool aHorizontal,
+    const ComputedStyle& aStyle, const EventStates& aElementState,
+    const EventStates& aDocumentState, uint32_t aDpiRatio) {
   sRGBColor thumbColor =
       ComputeScrollbarthumbColor(aStyle, aElementState, aDocumentState);
   Rect thumbRect(aRect);
   thumbRect.Deflate(floorf(aRect.height / 4.0f));
   PaintRoundedRectWithRadius(aDrawTarget, thumbRect, thumbColor, sRGBColor(), 0,
                              thumbRect.height / 2.0f, 1);
 }
 
-void nsNativeBasicThemeGTK::PaintScrollbarthumbVertical(
-    DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-    const EventStates& aElementState, const EventStates& aDocumentState) {
-  sRGBColor thumbColor =
-      ComputeScrollbarthumbColor(aStyle, aElementState, aDocumentState);
-  Rect thumbRect(aRect);
-  thumbRect.Deflate(floorf(aRect.width / 4.0f));
-  PaintRoundedRectWithRadius(aDrawTarget, thumbRect, thumbColor, sRGBColor(), 0,
-                             thumbRect.width / 2.0f, 1);
-}
-
-void nsNativeBasicThemeGTK::PaintScrollbarHorizontal(
-    DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-    const EventStates& aDocumentState, bool aIsRoot) {
+void nsNativeBasicThemeGTK::PaintScrollbar(DrawTarget* aDrawTarget,
+                                           const Rect& aRect, bool aHorizontal,
+                                           const ComputedStyle& aStyle,
+                                           const EventStates& aDocumentState,
+                                           uint32_t aDpiRatio, bool aIsRoot) {
   sRGBColor trackColor = ComputeScrollbarColor(aStyle, aDocumentState, aIsRoot);
   aDrawTarget->FillRect(aRect, gfx::ColorPattern(ToDeviceColor(trackColor)));
 }
 
-void nsNativeBasicThemeGTK::PaintScrollbarVerticalAndCorner(
+void nsNativeBasicThemeGTK::PaintScrollCorner(
     DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
     const EventStates& aDocumentState, uint32_t aDpiRatio, bool aIsRoot) {
   sRGBColor trackColor = ComputeScrollbarColor(aStyle, aDocumentState, aIsRoot);
   aDrawTarget->FillRect(aRect, gfx::ColorPattern(ToDeviceColor(trackColor)));
 }
--- a/widget/gtk/nsNativeBasicThemeGTK.h
+++ b/widget/gtk/nsNativeBasicThemeGTK.h
@@ -15,32 +15,27 @@ class nsNativeBasicThemeGTK : public nsN
 
   NS_IMETHOD GetMinimumWidgetSize(nsPresContext* aPresContext, nsIFrame* aFrame,
                                   StyleAppearance aAppearance,
                                   mozilla::LayoutDeviceIntSize* aResult,
                                   bool* aIsOverridable) override;
 
   nsITheme::Transparency GetWidgetTransparency(
       nsIFrame* aFrame, StyleAppearance aAppearance) override;
-  void PaintScrollbarthumbHorizontal(
-      DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-      const EventStates& aElementState,
-      const EventStates& aDocumentState) override;
-  void PaintScrollbarthumbVertical(DrawTarget* aDrawTarget, const Rect& aRect,
-                                   const ComputedStyle& aStyle,
-                                   const EventStates& aElementState,
-                                   const EventStates& aDocumentState) override;
-  void PaintScrollbarHorizontal(DrawTarget* aDrawTarget, const Rect& aRect,
-                                const ComputedStyle& aStyle,
-                                const EventStates& aDocumentState,
-                                bool aIsRoot) override;
-  void PaintScrollbarVerticalAndCorner(DrawTarget* aDrawTarget,
-                                       const Rect& aRect,
-                                       const ComputedStyle& aStyle,
-                                       const EventStates& aDocumentState,
-                                       uint32_t aDpiRatio,
-                                       bool aIsRoot) override;
+  void PaintScrollbarThumb(DrawTarget* aDrawTarget, const Rect& aRect,
+                           bool aHorizontal, const ComputedStyle& aStyle,
+                           const EventStates& aElementState,
+                           const EventStates& aDocumentState,
+                           uint32_t aDpiRatio) override;
+  void PaintScrollbar(DrawTarget* aDrawTarget, const Rect& aRect,
+                      bool aHorizontal, const ComputedStyle& aStyle,
+                      const EventStates& aDocumentState, uint32_t aDpiRatio,
+                      bool aIsRoot) override;
+  void PaintScrollCorner(DrawTarget* aDrawTarget, const Rect& aRect,
+                         const ComputedStyle& aStyle,
+                         const EventStates& aDocumentState, uint32_t aDpiRatio,
+                         bool aIsRoot) override;
 
  protected:
   virtual ~nsNativeBasicThemeGTK() = default;
 };
 
 #endif
--- a/widget/nsNativeBasicTheme.cpp
+++ b/widget/nsNativeBasicTheme.cpp
@@ -915,71 +915,60 @@ sRGBColor nsNativeBasicTheme::ComputeScr
     // Root scrollbars must be opaque.
     nscolor bg = LookAndFeel::GetColor(LookAndFeel::ColorID::WindowBackground,
                                        NS_RGB(0xff, 0xff, 0xff));
     color = NS_ComposeColors(bg, color);
   }
   return gfx::sRGBColor::FromABGR(color);
 }
 
-void nsNativeBasicTheme::PaintScrollbarthumbHorizontal(
-    DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-    const EventStates& aElementState, const EventStates& aDocumentState) {
+void nsNativeBasicTheme::PaintScrollbarThumb(
+    DrawTarget* aDrawTarget, const Rect& aRect, bool aHorizontal,
+    const ComputedStyle& aStyle, const EventStates& aElementState,
+    const EventStates& aDocumentState, uint32_t aDpiRatio) {
   sRGBColor thumbColor =
       ComputeScrollbarthumbColor(aStyle, aElementState, aDocumentState);
   aDrawTarget->FillRect(aRect, ColorPattern(ToDeviceColor(thumbColor)));
 }
 
-void nsNativeBasicTheme::PaintScrollbarthumbVertical(
-    DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-    const EventStates& aElementState, const EventStates& aDocumentState) {
-  sRGBColor thumbColor =
-      ComputeScrollbarthumbColor(aStyle, aElementState, aDocumentState);
-  aDrawTarget->FillRect(aRect, ColorPattern(ToDeviceColor(thumbColor)));
-}
-
-void nsNativeBasicTheme::PaintScrollbarHorizontal(
-    DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-    const EventStates& aDocumentState, bool aIsRoot) {
+void nsNativeBasicTheme::PaintScrollbar(DrawTarget* aDrawTarget,
+                                        const Rect& aRect, bool aHorizontal,
+                                        const ComputedStyle& aStyle,
+                                        const EventStates& aDocumentState,
+                                        uint32_t aDpiRatio, bool aIsRoot) {
   sRGBColor scrollbarColor =
       ComputeScrollbarColor(aStyle, aDocumentState, aIsRoot);
   aDrawTarget->FillRect(aRect, ColorPattern(ToDeviceColor(scrollbarColor)));
   // FIXME(heycam): We should probably derive the border color when custom
   // scrollbar colors are in use too.  But for now, just skip painting it,
   // to avoid ugliness.
   if (aStyle.StyleUI()->mScrollbarColor.IsAuto()) {
     RefPtr<PathBuilder> builder = aDrawTarget->CreatePathBuilder();
-    builder->MoveTo(Point(aRect.x, aRect.y));
-    builder->LineTo(Point(aRect.x + aRect.width, aRect.y));
-    RefPtr<Path> path = builder->Finish();
-    aDrawTarget->Stroke(path,
-                        ColorPattern(ToDeviceColor(sScrollbarBorderColor)));
-  }
-}
-
-void nsNativeBasicTheme::PaintScrollbarVerticalAndCorner(
-    DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-    const EventStates& aDocumentState, uint32_t aDpiRatio, bool aIsRoot) {
-  sRGBColor scrollbarColor =
-      ComputeScrollbarColor(aStyle, aDocumentState, aIsRoot);
-  aDrawTarget->FillRect(aRect, ColorPattern(ToDeviceColor(scrollbarColor)));
-  // FIXME(heycam): We should probably derive the border color when custom
-  // scrollbar colors are in use too.  But for now, just skip painting it,
-  // to avoid ugliness.
-  if (aStyle.StyleUI()->mScrollbarColor.IsAuto()) {
-    RefPtr<PathBuilder> builder = aDrawTarget->CreatePathBuilder();
-    builder->MoveTo(Point(aRect.x, aRect.y));
-    builder->LineTo(Point(aRect.x, aRect.y + aRect.height));
+    Rect strokeRect(aRect);
+    strokeRect.Deflate(0.5f * aDpiRatio);
+    builder->MoveTo(Point(strokeRect.TopLeft()));
+    builder->LineTo(
+        Point(aHorizontal ? strokeRect.TopRight() : strokeRect.BottomLeft()));
     RefPtr<Path> path = builder->Finish();
     aDrawTarget->Stroke(path,
                         ColorPattern(ToDeviceColor(sScrollbarBorderColor)),
                         StrokeOptions(1.0f * aDpiRatio));
   }
 }
 
+void nsNativeBasicTheme::PaintScrollCorner(DrawTarget* aDrawTarget,
+                                           const Rect& aRect,
+                                           const ComputedStyle& aStyle,
+                                           const EventStates& aDocumentState,
+                                           uint32_t aDpiRatio, bool aIsRoot) {
+  sRGBColor scrollbarColor =
+      ComputeScrollbarColor(aStyle, aDocumentState, aIsRoot);
+  aDrawTarget->FillRect(aRect, ColorPattern(ToDeviceColor(scrollbarColor)));
+}
+
 void nsNativeBasicTheme::PaintScrollbarbutton(
     DrawTarget* aDrawTarget, StyleAppearance aAppearance, const Rect& aRect,
     const ComputedStyle& aStyle, const EventStates& aElementState,
     const EventStates& aDocumentState, uint32_t aDpiRatio) {
   bool isActive = aElementState.HasState(NS_EVENT_STATE_ACTIVE);
   bool isHovered = aElementState.HasState(NS_EVENT_STATE_HOVER);
 
   bool hasCustomColor = aStyle.StyleUI()->mScrollbarColor.IsColors();
@@ -1035,44 +1024,45 @@ void nsNativeBasicTheme::PaintScrollbarb
     // color.
     nscolor bg = buttonColor.ToABGR();
     bool darken = NS_GetLuminosity(bg) >= NS_MAX_LUMINOSITY / 2;
     if (isActive) {
       float c = darken ? 0.0f : 1.0f;
       arrowColor = sRGBColor(c, c, c);
     } else {
       uint8_t c = darken ? 0 : 255;
-      arrowColor = sRGBColor::FromABGR(NS_ComposeColors(bg, NS_RGBA(c, c, c, 160)));
+      arrowColor =
+          sRGBColor::FromABGR(NS_ComposeColors(bg, NS_RGBA(c, c, c, 160)));
     }
   } else if (isActive) {
     arrowColor = sScrollbarArrowColorActive;
   } else if (isHovered) {
     arrowColor = sScrollbarArrowColorHover;
   } else {
     arrowColor = sScrollbarArrowColor;
   }
   PaintArrow(aDrawTarget, aRect, arrowPolygonX, arrowPolygonY, arrowNumPoints,
-             arrowSize, arrowColor,
-             aDpiRatio);
+             arrowSize, arrowColor, aDpiRatio);
 
   // FIXME(heycam): We should probably derive the border color when custom
   // scrollbar colors are in use too.  But for now, just skip painting it,
   // to avoid ugliness.
   if (!hasCustomColor) {
     RefPtr<PathBuilder> builder = aDrawTarget->CreatePathBuilder();
     builder->MoveTo(Point(aRect.x, aRect.y));
     if (aAppearance == StyleAppearance::ScrollbarbuttonUp ||
         aAppearance == StyleAppearance::ScrollbarbuttonDown) {
       builder->LineTo(Point(aRect.x, aRect.y + aRect.height));
     } else {
       builder->LineTo(Point(aRect.x + aRect.width, aRect.y));
     }
 
     RefPtr<Path> path = builder->Finish();
-    aDrawTarget->Stroke(path, ColorPattern(ToDeviceColor(sScrollbarBorderColor)),
+    aDrawTarget->Stroke(path,
+                        ColorPattern(ToDeviceColor(sScrollbarBorderColor)),
                         StrokeOptions(1.0f * aDpiRatio));
   }
 }
 
 // Checks whether the frame is for a root <scrollbar> or <scrollcorner>, which
 // influences some platforms' scrollbar rendering.
 bool nsNativeBasicTheme::IsRootScrollbar(nsIFrame* aFrame) {
   return CheckBooleanAttr(aFrame, nsGkAtoms::root_) &&
@@ -1160,35 +1150,36 @@ nsNativeBasicTheme::DrawWidgetBackground
       break;
     case StyleAppearance::Meter:
       PaintMeter(dt, devPxRect, eventState, dpiRatio);
       break;
     case StyleAppearance::Meterchunk:
       PaintMeterchunk(aFrame, dt, devPxRect, eventState, dpiRatio);
       break;
     case StyleAppearance::ScrollbarthumbHorizontal:
-      PaintScrollbarthumbHorizontal(dt, devPxRect,
-                                    *nsLayoutUtils::StyleForScrollbar(aFrame),
-                                    eventState, docState);
+    case StyleAppearance::ScrollbarthumbVertical: {
+      bool isHorizontal =
+          aAppearance == StyleAppearance::ScrollbarthumbHorizontal;
+      PaintScrollbarThumb(dt, devPxRect, isHorizontal,
+                          *nsLayoutUtils::StyleForScrollbar(aFrame), eventState,
+                          docState, dpiRatio);
       break;
-    case StyleAppearance::ScrollbarthumbVertical:
-      PaintScrollbarthumbVertical(dt, devPxRect,
-                                  *nsLayoutUtils::StyleForScrollbar(aFrame),
-                                  eventState, docState);
-      break;
+    }
     case StyleAppearance::ScrollbarHorizontal:
-      PaintScrollbarHorizontal(dt, devPxRect,
-                               *nsLayoutUtils::StyleForScrollbar(aFrame),
-                               docState, IsRootScrollbar(aFrame));
+    case StyleAppearance::ScrollbarVertical: {
+      bool isHorizontal = aAppearance == StyleAppearance::ScrollbarHorizontal;
+      PaintScrollbar(dt, devPxRect, isHorizontal,
+                     *nsLayoutUtils::StyleForScrollbar(aFrame), docState,
+                     dpiRatio, IsRootScrollbar(aFrame));
       break;
-    case StyleAppearance::ScrollbarVertical:
+    }
     case StyleAppearance::Scrollcorner:
-      PaintScrollbarVerticalAndCorner(
-          dt, devPxRect, *nsLayoutUtils::StyleForScrollbar(aFrame), docState,
-          dpiRatio, IsRootScrollbar(aFrame));
+      PaintScrollCorner(dt, devPxRect,
+                        *nsLayoutUtils::StyleForScrollbar(aFrame), docState,
+                        dpiRatio, IsRootScrollbar(aFrame));
       break;
     case StyleAppearance::ScrollbarbuttonUp:
     case StyleAppearance::ScrollbarbuttonDown:
     case StyleAppearance::ScrollbarbuttonLeft:
     case StyleAppearance::ScrollbarbuttonRight:
       PaintScrollbarbutton(dt, aAppearance, devPxRect,
                            *nsLayoutUtils::StyleForScrollbar(aFrame),
                            eventState, docState, dpiRatio);
--- a/widget/nsNativeBasicTheme.h
+++ b/widget/nsNativeBasicTheme.h
@@ -299,33 +299,29 @@ class nsNativeBasicTheme : protected nsN
                          const EventStates& aState, uint32_t aDpiRatio);
   static void PaintMeterchunk(nsIFrame* aFrame, DrawTarget* aDrawTarget,
                               const Rect& aRect, const EventStates& aState,
                               uint32_t aDpiRatio);
   static void PaintButton(nsIFrame* aFrame, DrawTarget* aDrawTarget,
                           const Rect& aRect, const EventStates& aState,
                           uint32_t aDpiRatio);
 
-  virtual void PaintScrollbarthumbHorizontal(DrawTarget* aDrawTarget,
-                                             const Rect& aRect,
-                                             const ComputedStyle& aStyle,
-                                             const EventStates& aElementState,
-                                             const EventStates& aDocumentState);
-  virtual void PaintScrollbarthumbVertical(DrawTarget* aDrawTarget,
-                                           const Rect& aRect,
-                                           const ComputedStyle& aStyle,
-                                           const EventStates& aElementState,
-                                           const EventStates& aDocumentState);
-  virtual void PaintScrollbarHorizontal(DrawTarget* aDrawTarget,
-                                        const Rect& aRect,
-                                        const ComputedStyle& aStyle,
-                                        const EventStates& aDocumentState,
-                                        bool aIsRoot);
-  virtual void PaintScrollbarVerticalAndCorner(
-      DrawTarget* aDrawTarget, const Rect& aRect, const ComputedStyle& aStyle,
-      const EventStates& aDocumentState, uint32_t aDpiRatio, bool aIsRoot);
+  virtual void PaintScrollbarThumb(DrawTarget* aDrawTarget, const Rect& aRect,
+                                   bool aHorizontal,
+                                   const ComputedStyle& aStyle,
+                                   const EventStates& aElementState,
+                                   const EventStates& aDocumentState,
+                                   uint32_t aDpiRatio);
+  virtual void PaintScrollbar(DrawTarget* aDrawTarget, const Rect& aRect,
+                              bool aHorizontal, const ComputedStyle& aStyle,
+                              const EventStates& aDocumentState,
+                              uint32_t aDpiRatio, bool aIsRoot);
+  virtual void PaintScrollCorner(DrawTarget* aDrawTarget, const Rect& aRect,
+                                 const ComputedStyle& aStyle,
+                                 const EventStates& aDocumentState,
+                                 uint32_t aDpiRatio, bool aIsRoot);
   virtual void PaintScrollbarbutton(
       DrawTarget* aDrawTarget, StyleAppearance aAppearance, const Rect& aRect,
       const ComputedStyle& aStyle, const EventStates& aElementState,
       const EventStates& aDocumentState, uint32_t aDpiRatio);
 };
 
 #endif