Improve dirty region calculation in software renderer

The software renderers dirty region calculation is done in integral logical
coordinates, which leads to quite some trouble when handling scene graph
elements with fractional coordinates (used by WebEngine for example).

The optimal solution would probably be to either use integral physical
coordinates or floating point logical coordinates, however this would seem to
require substantial changes to QPainter and QBackingStore and so on. So, this
patch instead changes the calculation to use something like interval arithmetic:
instead of just rounding each logical coordinate to the nearest integer the
renderer now uses (very carefully) both the upper and lower boundaries to make
sure that the dirty regions always err on the side of caution.

I expect this change to make rendering slower but only in situations where
previously there would be rendering errors.

Task-number: QTBUG-60393
Change-Id: I7f8e7d2739c810328c841130df9c1c7332439447
Reviewed-by: Andy Nichols <andy.nichols@qt.io>

3 files changed, 55 insertions, 36 deletions

diff --git a/src/quick/scenegraph/adaptations/software/qsgabstractsoftwarerenderer.cpp b/src/quick/scenegraph/adaptations/software/qsgabstractsoftwarerenderer.cpp
index 2ff180ea99..02cf8209d1 100644
--- a/src/quick/scenegraph/adaptations/software/qsgabstractsoftwarerenderer.cpp
+++ b/src/quick/scenegraph/adaptations/software/qsgabstractsoftwarerenderer.cpp
@@ -165,12 +165,12 @@ QRegion QSGAbstractSoftwareRenderer::optimizeRenderList()
 
         // Keep up with obscured regions
         if (node->isOpaque()) {
-            m_obscuredRegion += QRegion(node->boundingRect());
+            m_obscuredRegion += node->boundingRectMin();
         }
 
         if (node->isDirty()) {
             // Don't paint things outside of the rendering area
-            if (!m_background->rect().toRect().contains(node->boundingRect(), /*proper*/ true)) {
+            if (!m_background->rect().toRect().contains(node->boundingRectMax(), /*proper*/ true)) {
                 // Some part(s) of node is(are) outside of the rendering area
                 QRegion renderArea(m_background->rect().toRect());
                 QRegion outsideRegions = node->dirtyRegion().subtracted(renderArea);
@@ -181,7 +181,7 @@ QRegion QSGAbstractSoftwareRenderer::optimizeRenderList()
             // Get the dirty region's to pass to the next nodes
             if (node->isOpaque()) {
                 // if isOpaque, subtract node's dirty rect from m_dirtyRegion
-                m_dirtyRegion -= node->dirtyRegion();
+                m_dirtyRegion -= node->boundingRectMin();
             } else {
                 // if isAlpha, add node's dirty rect to m_dirtyRegion
                 m_dirtyRegion += node->dirtyRegion();
@@ -264,7 +264,7 @@ void QSGAbstractSoftwareRenderer::nodeRemoved(QSGNode *node)
         // Need to mark this region dirty in the other nodes
         QRegion dirtyRegion = renderable->previousDirtyRegion(true);
         if (dirtyRegion.isEmpty())
-            dirtyRegion = renderable->boundingRect();
+            dirtyRegion = renderable->boundingRectMax();
         m_dirtyRegion += dirtyRegion;
         m_nodes.remove(node);
         delete renderable;
diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode.cpp b/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode.cpp
index 59c47db0c4..ce08d18057 100644
--- a/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode.cpp
+++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode.cpp
@@ -54,10 +54,28 @@
 #include <private/qsgrendernode_p.h>
 #include <private/qsgtexture_p.h>
 
+#include <qmath.h>
+
 Q_LOGGING_CATEGORY(lcRenderable, "qt.scenegraph.softwarecontext.renderable")
 
 QT_BEGIN_NAMESPACE
 
+// Largest subrectangle with integer coordinates
+inline QRect toRectMin(const QRectF & r)
+{
+    int x1 = qCeil(r.left());
+    int x2 = qFloor(r.right());
+    int y1 = qCeil(r.top());
+    int y2 = qFloor(r.bottom());
+    return QRect(x1, y1, x2 - x1, y2 - y1);
+}
+
+// Smallest superrectangle with integer coordinates
+inline QRect toRectMax(const QRectF & r)
+{
+    return r.toAlignedRect();
+}
+
 QSGSoftwareRenderableNode::QSGSoftwareRenderableNode(NodeType type, QSGNode *node)
     : m_nodeType(type)
     , m_isOpaque(true)
@@ -117,7 +135,7 @@ void QSGSoftwareRenderableNode::update()
     // Update the Node properties
     m_isDirty = true;
 
-    QRect boundingRect;
+    QRectF boundingRect;
 
     switch (m_nodeType) {
     case QSGSoftwareRenderableNode::SimpleRect:
@@ -126,7 +144,7 @@ void QSGSoftwareRenderableNode::update()
         else
             m_isOpaque = false;
 
-        boundingRect = m_handle.simpleRectNode->rect().toRect();
+        boundingRect = m_handle.simpleRectNode->rect();
         break;
     case QSGSoftwareRenderableNode::SimpleTexture:
         if (!m_handle.simpleTextureNode->texture()->hasAlphaChannel() && !m_transform.isRotating())
@@ -134,7 +152,7 @@ void QSGSoftwareRenderableNode::update()
         else
             m_isOpaque = false;
 
-        boundingRect = m_handle.simpleTextureNode->rect().toRect();
+        boundingRect = m_handle.simpleTextureNode->rect();
         break;
     case QSGSoftwareRenderableNode::Image:
         // There isn't a way to tell, so assume it's not
@@ -148,7 +166,7 @@ void QSGSoftwareRenderableNode::update()
         else
             m_isOpaque = false;
 
-        boundingRect = QRect(0, 0, m_handle.painterNode->size().width(), m_handle.painterNode->size().height());
+        boundingRect = QRectF(0, 0, m_handle.painterNode->size().width(), m_handle.painterNode->size().height());
         break;
     case QSGSoftwareRenderableNode::Rectangle:
         if (m_handle.rectangleNode->isOpaque() && !m_transform.isRotating())
@@ -156,19 +174,19 @@ void QSGSoftwareRenderableNode::update()
         else
             m_isOpaque = false;
 
-        boundingRect = m_handle.rectangleNode->rect().toRect();
+        boundingRect = m_handle.rectangleNode->rect();
         break;
     case QSGSoftwareRenderableNode::Glyph:
         // Always has alpha
         m_isOpaque = false;
 
-        boundingRect = m_handle.glpyhNode->boundingRect().toAlignedRect();
+        boundingRect = m_handle.glpyhNode->boundingRect();
         break;
     case QSGSoftwareRenderableNode::NinePatch:
         // Difficult to tell, assume non-opaque
         m_isOpaque = false;
 
-        boundingRect = m_handle.ninePatchNode->bounds().toRect();
+        boundingRect = m_handle.ninePatchNode->bounds();
         break;
     case QSGSoftwareRenderableNode::SimpleRectangle:
         if (m_handle.simpleRectangleNode->color().alpha() == 255 && !m_transform.isRotating())
@@ -176,7 +194,7 @@ void QSGSoftwareRenderableNode::update()
         else
             m_isOpaque = false;
 
-        boundingRect = m_handle.simpleRectangleNode->rect().toRect();
+        boundingRect = m_handle.simpleRectangleNode->rect();
         break;
     case QSGSoftwareRenderableNode::SimpleImage:
         if (!m_handle.simpleImageNode->texture()->hasAlphaChannel() && !m_transform.isRotating())
@@ -184,12 +202,12 @@ void QSGSoftwareRenderableNode::update()
         else
             m_isOpaque = false;
 
-        boundingRect = m_handle.simpleImageNode->rect().toRect();
+        boundingRect = m_handle.simpleImageNode->rect();
         break;
 #if QT_CONFIG(quick_sprite)
     case QSGSoftwareRenderableNode::SpriteNode:
         m_isOpaque = m_handle.spriteNode->isOpaque();
-        boundingRect = m_handle.spriteNode->rect().toRect();
+        boundingRect = m_handle.spriteNode->rect();
         break;
 #endif
     case QSGSoftwareRenderableNode::RenderNode:
@@ -198,27 +216,32 @@ void QSGSoftwareRenderableNode::update()
         else
             m_isOpaque = false;
 
-        boundingRect = m_handle.renderNode->rect().toRect();
+        boundingRect = m_handle.renderNode->rect();
         break;
     default:
         break;
     }
 
-    m_boundingRect = m_transform.mapRect(boundingRect);
+    const QRectF transformedRect = m_transform.mapRect(boundingRect);
+    m_boundingRectMin = toRectMin(transformedRect);
+    m_boundingRectMax = toRectMax(transformedRect);
 
     if (m_hasClipRegion && m_clipRegion.rectCount() <= 1) {
         // If there is a clipRegion, and it is empty, the item wont be rendered
-        if (m_clipRegion.isEmpty())
-            m_boundingRect = QRect();
-        else
-            m_boundingRect = m_boundingRect.intersected(m_clipRegion.rects().first());
+        if (m_clipRegion.isEmpty()) {
+            m_boundingRectMin = QRect();
+            m_boundingRectMax = QRect();
+        } else {
+            m_boundingRectMin = m_boundingRectMin.intersected(m_clipRegion.rects().first());
+            m_boundingRectMax = m_boundingRectMax.intersected(m_clipRegion.rects().first());
+        }
     }
 
     // Overrides
     if (m_opacity < 1.0f)
         m_isOpaque = false;
 
-    m_dirtyRegion = QRegion(m_boundingRect);
+    m_dirtyRegion = QRegion(m_boundingRectMax);
 }
 
 struct RenderNodeState : public QSGRenderNode::RenderState
@@ -258,7 +281,7 @@ QRegion QSGSoftwareRenderableNode::renderNode(QPainter *painter, bool forceOpaqu
             rs.cr = m_clipRegion;
 
             const QRect br = m_handle.renderNode->flags().testFlag(QSGRenderNode::BoundedRectRendering)
-                ? m_boundingRect :
+                ? m_boundingRectMax :
                 QRect(0, 0, painter->device()->width(), painter->device()->height());
 
             painter->save();
@@ -335,19 +358,13 @@ QRegion QSGSoftwareRenderableNode::renderNode(QPainter *painter, bool forceOpaqu
     painter->restore();
 
     QRegion areaToBeFlushed = m_dirtyRegion;
-    m_previousDirtyRegion = QRegion(m_boundingRect);
+    m_previousDirtyRegion = QRegion(m_boundingRectMax);
     m_isDirty = false;
     m_dirtyRegion = QRegion();
 
     return areaToBeFlushed;
 }
 
-QRect QSGSoftwareRenderableNode::boundingRect() const
-{
-    // This returns the bounding area of a renderable node in world coordinates
-    return m_boundingRect;
-}
-
 bool QSGSoftwareRenderableNode::isDirtyRegionEmpty() const
 {
     return m_dirtyRegion.isEmpty();
@@ -392,12 +409,12 @@ void QSGSoftwareRenderableNode::markMaterialDirty()
 
 void QSGSoftwareRenderableNode::addDirtyRegion(const QRegion &dirtyRegion, bool forceDirty)
 {
-    // Check if the dirty region applys to this node
+    // Check if the dirty region applies to this node
     QRegion prev = m_dirtyRegion;
-    if (dirtyRegion.intersects(boundingRect())) {
+    if (dirtyRegion.intersects(m_boundingRectMax)) {
         if (forceDirty)
             m_isDirty = true;
-        m_dirtyRegion += dirtyRegion.intersected(boundingRect());
+        m_dirtyRegion += dirtyRegion.intersected(m_boundingRectMax);
     }
     qCDebug(lcRenderable) << "addDirtyRegion: " << dirtyRegion << "old dirtyRegion: " << prev << "new dirtyRegion: " << m_dirtyRegion;
 }
@@ -407,7 +424,7 @@ void QSGSoftwareRenderableNode::subtractDirtyRegion(const QRegion &dirtyRegion)
     QRegion prev = m_dirtyRegion;
     if (m_isDirty) {
         // Check if this rect concerns us
-        if (dirtyRegion.intersects(QRegion(boundingRect()))) {
+        if (dirtyRegion.intersects(m_boundingRectMax)) {
             m_dirtyRegion -= dirtyRegion;
             if (m_dirtyRegion.isEmpty())
                 m_isDirty = false;
@@ -423,7 +440,7 @@ QRegion QSGSoftwareRenderableNode::previousDirtyRegion(bool wasRemoved) const
     if (wasRemoved)
         return m_previousDirtyRegion;
 
-    return m_previousDirtyRegion.subtracted(QRegion(m_boundingRect));
+    return m_previousDirtyRegion.subtracted(QRegion(m_boundingRectMax));
 }
 
 QRegion QSGSoftwareRenderableNode::dirtyRegion() const
diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode_p.h b/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode_p.h
index 473578c185..8fc87db179 100644
--- a/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode_p.h
+++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarerenderablenode_p.h
@@ -98,7 +98,8 @@ public:
     void update();
 
     QRegion renderNode(QPainter *painter, bool forceOpaquePainting = false);
-    QRect boundingRect() const;
+    QRect boundingRectMin() const { return m_boundingRectMin; }
+    QRect boundingRectMax() const { return m_boundingRectMax; }
     NodeType type() const { return m_nodeType; }
     bool isOpaque() const { return m_isOpaque; }
     bool isDirty() const { return m_isDirty; }
@@ -149,7 +150,8 @@ private:
     bool m_hasClipRegion;
     float m_opacity;
 
-    QRect m_boundingRect;
+    QRect m_boundingRectMin;
+    QRect m_boundingRectMax;
 };
 
 QT_END_NAMESPACE