Quick: Move counters out of a bitfield in QQuickitem

Storing the anchor-loop-detection counters in a bitfield has the
disadvantage that the full field has to be read and masked in order to
use (increment) them. The same for the subsequent store. By putting them
in their own byte, this can be done a lot faster. Those bytes were
available, because they were needed for padding. By making them signed,
there is also no need for the compiler to insert overflow handling.

Change-Id: I3c250983c74de2ecfd33fe72ea8df04e24b8cd0c
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

2 files changed, 75 insertions, 71 deletions

diff --git a/src/quick/items/qquickanchors.cpp b/src/quick/items/qquickanchors.cpp
index b6978e534e..a069f1ece3 100644
--- a/src/quick/items/qquickanchors.cpp
+++ b/src/quick/items/qquickanchors.cpp
@@ -617,74 +617,75 @@ void QQuickAnchorsPrivate::updateVerticalAnchors()
     if (fill || centerIn || !isItemComplete())
         return;
 
-    if (updatingVerticalAnchor < 2) {
-        ++updatingVerticalAnchor;
-        if (usedAnchors & QQuickAnchors::TopAnchor) {
-            //Handle stretching
-            bool invalid = true;
+    if (Q_UNLIKELY(updatingVerticalAnchor > 1)) {
+        // ### Make this certain :)
+        qmlInfo(item) << QQuickAnchors::tr("Possible anchor loop detected on vertical anchor.");
+        return;
+    }
+
+    ++updatingVerticalAnchor;
+    if (usedAnchors & QQuickAnchors::TopAnchor) {
+        //Handle stretching
+        bool invalid = true;
+        qreal height = 0.0;
+        if (usedAnchors & QQuickAnchors::BottomAnchor) {
+            invalid = calcStretch(topAnchorItem, topAnchorLine,
+                                  bottomAnchorItem, bottomAnchorLine,
+                                  topMargin, -bottomMargin, QQuickAnchors::TopAnchor, height);
+        } else if (usedAnchors & QQuickAnchors::VCenterAnchor) {
+            invalid = calcStretch(topAnchorItem, topAnchorLine,
+                                  vCenterAnchorItem, vCenterAnchorLine,
+                                  topMargin, vCenterOffset, QQuickAnchors::TopAnchor, height);
+            height *= 2;
+        }
+        if (!invalid)
+            setItemHeight(height);
+
+        //Handle top
+        if (topAnchorItem == readParentItem(item)) {
+            setItemY(adjustedPosition(topAnchorItem, topAnchorLine) + topMargin);
+        } else if (readParentItem(topAnchorItem) == readParentItem(item)) {
+            setItemY(position(topAnchorItem, topAnchorLine) + topMargin);
+        }
+    } else if (usedAnchors & QQuickAnchors::BottomAnchor) {
+        //Handle stretching (top + bottom case is handled above)
+        if (usedAnchors & QQuickAnchors::VCenterAnchor) {
             qreal height = 0.0;
-            if (usedAnchors & QQuickAnchors::BottomAnchor) {
-                invalid = calcStretch(topAnchorItem, topAnchorLine,
-                                      bottomAnchorItem, bottomAnchorLine,
-                                      topMargin, -bottomMargin, QQuickAnchors::TopAnchor, height);
-            } else if (usedAnchors & QQuickAnchors::VCenterAnchor) {
-                invalid = calcStretch(topAnchorItem, topAnchorLine,
-                                      vCenterAnchorItem, vCenterAnchorLine,
-                                      topMargin, vCenterOffset, QQuickAnchors::TopAnchor, height);
-                height *= 2;
-            }
+            bool invalid = calcStretch(vCenterAnchorItem, vCenterAnchorLine,
+                                       bottomAnchorItem, bottomAnchorLine,
+                                       vCenterOffset, -bottomMargin, QQuickAnchors::TopAnchor,
+                                       height);
             if (!invalid)
-                setItemHeight(height);
-
-            //Handle top
-            if (topAnchorItem == readParentItem(item)) {
-                setItemY(adjustedPosition(topAnchorItem, topAnchorLine) + topMargin);
-            } else if (readParentItem(topAnchorItem) == readParentItem(item)) {
-                setItemY(position(topAnchorItem, topAnchorLine) + topMargin);
-            }
-        } else if (usedAnchors & QQuickAnchors::BottomAnchor) {
-            //Handle stretching (top + bottom case is handled above)
-            if (usedAnchors & QQuickAnchors::VCenterAnchor) {
-                qreal height = 0.0;
-                bool invalid = calcStretch(vCenterAnchorItem, vCenterAnchorLine,
-                                           bottomAnchorItem, bottomAnchorLine,
-                                           vCenterOffset, -bottomMargin, QQuickAnchors::TopAnchor,
-                                           height);
-                if (!invalid)
-                    setItemHeight(height*2);
-            }
+                setItemHeight(height*2);
+        }
 
-            //Handle bottom
-            if (bottomAnchorItem == readParentItem(item)) {
-                setItemY(adjustedPosition(bottomAnchorItem, bottomAnchorLine) - readHeight(item) - bottomMargin);
-            } else if (readParentItem(bottomAnchorItem) == readParentItem(item)) {
-                setItemY(position(bottomAnchorItem, bottomAnchorLine) - readHeight(item) - bottomMargin);
-            }
-        } else if (usedAnchors & QQuickAnchors::VCenterAnchor) {
-            //(stetching handled above)
-
-            //Handle vCenter
-            if (vCenterAnchorItem == readParentItem(item)) {
-                setItemY(adjustedPosition(vCenterAnchorItem, vCenterAnchorLine)
-                              - vcenter(item) + vCenterOffset);
-            } else if (readParentItem(vCenterAnchorItem) == readParentItem(item)) {
-                setItemY(position(vCenterAnchorItem, vCenterAnchorLine) - vcenter(item) + vCenterOffset);
-            }
-        } else if (usedAnchors & QQuickAnchors::BaselineAnchor) {
-            //Handle baseline
-            if (baselineAnchorItem == readParentItem(item)) {
-                setItemY(adjustedPosition(baselineAnchorItem, baselineAnchorLine)
-                         - readBaselineOffset(item) + baselineOffset);
-            } else if (readParentItem(baselineAnchorItem) == readParentItem(item)) {
-                setItemY(position(baselineAnchorItem, baselineAnchorLine)
-                         - readBaselineOffset(item) + baselineOffset);
-            }
+        //Handle bottom
+        if (bottomAnchorItem == readParentItem(item)) {
+            setItemY(adjustedPosition(bottomAnchorItem, bottomAnchorLine) - readHeight(item) - bottomMargin);
+        } else if (readParentItem(bottomAnchorItem) == readParentItem(item)) {
+            setItemY(position(bottomAnchorItem, bottomAnchorLine) - readHeight(item) - bottomMargin);
+        }
+    } else if (usedAnchors & QQuickAnchors::VCenterAnchor) {
+        //(stetching handled above)
+
+        //Handle vCenter
+        if (vCenterAnchorItem == readParentItem(item)) {
+            setItemY(adjustedPosition(vCenterAnchorItem, vCenterAnchorLine)
+                     - vcenter(item) + vCenterOffset);
+        } else if (readParentItem(vCenterAnchorItem) == readParentItem(item)) {
+            setItemY(position(vCenterAnchorItem, vCenterAnchorLine) - vcenter(item) + vCenterOffset);
+        }
+    } else if (usedAnchors & QQuickAnchors::BaselineAnchor) {
+        //Handle baseline
+        if (baselineAnchorItem == readParentItem(item)) {
+            setItemY(adjustedPosition(baselineAnchorItem, baselineAnchorLine)
+                     - readBaselineOffset(item) + baselineOffset);
+        } else if (readParentItem(baselineAnchorItem) == readParentItem(item)) {
+            setItemY(position(baselineAnchorItem, baselineAnchorLine)
+                     - readBaselineOffset(item) + baselineOffset);
         }
-        --updatingVerticalAnchor;
-    } else {
-        // ### Make this certain :)
-        qmlInfo(item) << QQuickAnchors::tr("Possible anchor loop detected on vertical anchor.");
     }
+    --updatingVerticalAnchor;
 }
 
 static inline QQuickAnchors::Anchor reverseAnchorLine(QQuickAnchors::Anchor anchorLine)
diff --git a/src/quick/items/qquickanchors_p_p.h b/src/quick/items/qquickanchors_p_p.h
index 3357e134bf..0373b1f824 100644
--- a/src/quick/items/qquickanchors_p_p.h
+++ b/src/quick/items/qquickanchors_p_p.h
@@ -113,12 +113,12 @@ public:
         , inDestructor(false)
         , baselineAnchorLine(QQuickAnchors::InvalidAnchor)
         , centerAligned(true)
+        , usedAnchors(QQuickAnchors::InvalidAnchor)
+        , componentComplete(true)
         , updatingFill(0)
         , updatingCenterIn(0)
         , updatingHorizontalAnchor(0)
         , updatingVerticalAnchor(0)
-        , componentComplete(true)
-        , usedAnchors(QQuickAnchors::InvalidAnchor)
     {
     }
 
@@ -198,13 +198,16 @@ public:
     uint inDestructor                        : 1;
     QQuickAnchors::Anchor baselineAnchorLine : 7;
     uint centerAligned                       : 1;
-    uint updatingFill                        : 2;
-    uint updatingCenterIn                    : 2;
-    uint updatingHorizontalAnchor            : 2;
-    uint updatingVerticalAnchor              : 2;
-
-    uint componentComplete                   : 1;
     uint usedAnchors                         : 7; // QQuickAnchors::Anchors
+    uint componentComplete                   : 1;
+
+    // Instead of using a mostly empty bit field, we can stretch the following fields up to be full
+    // bytes. The advantage is that incrementing/decrementing does not need any combining ands/ors.
+    qint8 updatingFill;
+    qint8 updatingCenterIn;
+    qint8 updatingHorizontalAnchor;
+    qint8 updatingVerticalAnchor;
+
 
     static inline QQuickAnchorsPrivate *get(QQuickAnchors *o) {
         return static_cast<QQuickAnchorsPrivate *>(QObjectPrivate::get(o));