Changeset 122647 in webkit

Timestamp:

Jul 13, 2012 5:15:43 PM (12 years ago)

Author:

pierre.rossi@gmail.com

Message:

[Qt] Improve the mobile theme slightly
​https://bugs.webkit.org/show_bug.cgi?id=90806

Reviewed by Kenneth Rohde Christiansen.

Improve drawing of the mobile theme's controls' background.

Ensure the focus ring never appears with the mobile theme, since it
looks bad in combination with the highlights.

No new tests. The painting code from the mobile theme is still
not covered specifically (it will when we revive pixel tests).

• platform/qt/RenderThemeQtMobile.cpp:

(WebCore):
(WebCore::addPointToOctants): Added. This is simply a helper to avoid

doing too much duplicate work in drawControlBackground.

(WebCore::drawControlBackground): Rely on the octant logic added above

and take the opportunity to increase the granularity.

(WebCore::borderPen):
(WebCore::StylePainterMobile::findLineEdit):
(WebCore::RenderThemeQtMobile::adjustTextFieldStyle):

• platform/qt/RenderThemeQtMobile.h:

(RenderThemeQtMobile):
(WebCore::RenderThemeQtMobile::supportsFocusRing):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/qt/RenderThemeQtMobile.cpp (modified) (7 diffs)
• platform/qt/RenderThemeQtMobile.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-13  Pierre Rossi  <pierre.rossi@gmail.com>
+
+        [Qt] Improve the mobile theme slightly
+        https://bugs.webkit.org/show_bug.cgi?id=90806
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Improve drawing of the mobile theme's controls' background.
+
+        Ensure the focus ring never appears with the mobile theme, since it
+        looks bad in combination with the highlights.
+
+        No new tests. The painting code from the mobile theme is still
+        not covered specifically (it will when we revive pixel tests).
+
+        * platform/qt/RenderThemeQtMobile.cpp:
+        (WebCore):
+        (WebCore::addPointToOctants): Added. This is simply a helper to avoid
+            doing too much duplicate work in drawControlBackground.
+        (WebCore::drawControlBackground): Rely on the octant logic added above
+            and take the opportunity to increase the granularity.
+        (WebCore::borderPen):
+        (WebCore::StylePainterMobile::findLineEdit):
+        (WebCore::RenderThemeQtMobile::adjustTextFieldStyle):
+        * platform/qt/RenderThemeQtMobile.h:
+        (RenderThemeQtMobile):
+        (WebCore::RenderThemeQtMobile::supportsFocusRing):
+
 2012-07-13  Julien Chaffraix  <jchaffraix@webkit.org>
 

trunk/Source/WebCore/platform/qt/RenderThemeQtMobile.cpp

@@ -67 +67 @@
 static const float buttonPaddingBottom = 3;
 static const float menuListPadding = 9;
-static const float textFieldPadding = 5;
+static const float textFieldPadding = 10;
 static const float radiusFactor = 0.36;
 static const float progressBarChunkPercentage = 0.2;
@@ -109 +109 @@
 }
 
+/*
+ * The octants' indices are identified below, for each point (x,y)
+ * in the first octant, we can populate the 7 others with the corresponding
+ * point.
+ *
+ *                                       index |   xpos   |   ypos
+ *                xd                    ---------------------------
+ *      4      |<--->| 3                    0  |  xd + x  |    y
+ *     __________________                   1  |  xd + y  |    x
+ *    /                  \                  2  |  xd + y  |   -x
+ * 5 |         .(c)       |  2              3  |  xd + x  |   -y
+ * 6 |                    |  1              4  | -xd - x  |   -y
+ *    \__________________/                  5  | -xd - y  |   -x
+ *                                          6  | -xd - y  |    x
+ *      7              0                    7  | -xd - x  |    y
+ *
+ **/
+
+static void addPointToOctants(QVector<QPainterPath>& octants, const QPointF& center, qreal x, qreal y , int xDelta = 0)
+{
+    ASSERT(octants.count() == 8);
+
+    for (short i = 0; i < 8; ++i) {
+        QPainterPath& octant = octants[i];
+        QPointF pos(center);
+        // The Gray code corresponding to the octant's index helps doing the math in a more generic way.
+        const short gray = (i >> 1) ^ i;
+        const qreal xOffset = xDelta + ((gray & 1) ? y : x);
+        pos.ry() += ((gray & 2)? -1 : 1) * ((gray & 1) ? x : y);
+        pos.rx() += (i < 4) ? xOffset : -xOffset;
+
+        if (octant.elementCount())
+            octant.lineTo(pos);
+        else // The path is empty. Initialize the start point.
+            octant.moveTo(pos);
+    }
+}
+
 static void drawControlBackground(QPainter* painter, const QPen& pen, const QRect& rect, const QBrush& brush)
 {
@@ -117 +155 @@
     painter->setBrush(brush);
 
-    const int line = 1;
-    const QRect paddedRect = rect.adjusted(line, line, -line, -line);
-
-    const int n = 3;
+    static const qreal line = 1.5;
+    const QRectF paddedRect = rect.adjusted(line, line, -line, -line);
+
+    static const int n = 3;
     const qreal invPow = 1 / double(n);
     ASSERT(paddedRect.width() >= paddedRect.height());
     const int radius = paddedRect.height() / 2;
     const int xDelta = paddedRect.width() / 2 - radius;
-    const QPoint center = paddedRect.topLeft() + QPoint(xDelta + radius, radius);
-    qreal x, y;
-    QPainterPath path;
-    path.moveTo(-xDelta, -radius);
-    for (y = -radius ; y <= radius; ++y) {
-        x = -xDelta - radius * pow(1 - pow(qAbs(y) / radius , n), invPow);
-        path.lineTo(x, y);
-    }
-    for (y = radius ; y >= -radius; --y) {
-        x =  xDelta + radius * pow(1 - pow(qAbs(y) / radius , n), invPow);
-        path.lineTo(x, y);
+    const QPointF center = paddedRect.center();
+    qreal x = 0;
+    qreal y;
+    QVector<QPainterPath> octants(8);
+    // Stay within reasonable distance from edge values, which can cause artifacts at certain zoom levels.
+    static const float epsilon = 0.02;
+    for (y = radius - epsilon; y - epsilon > x; y -= 0.5) {
+        x = radius * pow(1 - pow(qAbs(y) / radius , n), invPow);
+        addPointToOctants(octants, center, x, y, xDelta);
+    }
+
+    QPainterPath path = octants.first();
+    for (int i = 1; i < 8; ++i) {
+        // Due to the orientation of the arcs, we need to reverse the paths with odd indices.
+        QPainterPath subPath = (i % 2) ?  octants.at(i).toReversed() : octants.at(i);
+        path.connectPath(subPath);
     }
     path.closeSubpath();
-    path.translate(center);
 
     painter->drawPath(path);
@@ -153 +195 @@
 static inline QPen borderPen(QPainter* painter = 0)
 {
-    return QPen(darkColor, 0.4 * painterScale(painter), Qt::SolidLine, Qt::RoundCap, Qt::RoundJoin);
+    return QPen(darkColor, qMin(1.0, 0.4 * painterScale(painter)));
 }
 
@@ -384 +426 @@
     if (!findCachedControl(id, &result)) {
         const int focusFrame = painterScale(painter);
-        result = QPixmap(size + QSize(2 * focusFrame, 2 * focusFrame));
+        result = QPixmap(size);
         result.fill(Qt::transparent);
         const QRect rect = result.rect().adjusted(focusFrame, focusFrame, -focusFrame, -focusFrame);
@@ -391 +433 @@
 
         if (focused) {
-            QPen focusPen(highlightColor, focusFrame, Qt::SolidLine, Qt::RoundCap, Qt::RoundJoin);
+            QPen focusPen(highlightColor, 1.2 * painterScale(painter), Qt::SolidLine);
             drawControlBackground(&cachePainter, focusPen, rect, Qt::NoBrush);
         }
@@ -698 +740 @@
     style->setBackgroundColor(Color::transparent);
     style->resetBorder();
+    style->setBorderTopWidth(frameWidth);
+    style->setBorderRightWidth(frameWidth);
+    style->setBorderBottomWidth(frameWidth);
+    style->setBorderLeftWidth(frameWidth);
     style->resetPadding();
     computeSizeBasedOnStyle(style);

trunk/Source/WebCore/platform/qt/RenderThemeQtMobile.h

@@ -54 +54 @@
     virtual bool delegatesMenuListRendering() const { return true; }
 
-    // drawFocusRing() will return early if the color is invalid.
-    virtual Color platformFocusRingColor() const { return Color(); }
+    // We don't want the focus ring to be drawn by the graphics context so we
+    // always claim to support it in the theme.
+    // FIXME: This could be a usability problem in the case of contenteditable divs.
+    virtual bool supportsFocusRing(const RenderStyle*) const { return true; }
 
 protected: