Lok: improved tile invalidation compression

Handle corner cases better and eliminate
invalid rects and out-of-bounds coordinates.

Change-Id: Ib9247ae4f0306cf68937cd2678f6386fe7710eec
Reviewed-on: https://gerrit.libreoffice.org/31665
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>

1 files changed, 127 insertions, 56 deletions

diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index e3f517288150..79e284a2e7b8 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -348,7 +348,7 @@ struct RectangleAndPart
     int m_nPart;
 
     RectangleAndPart()
-        : m_nPart(-1)
+        : m_nPart(INT_MIN)  // -1 is reserved to mean "all parts".
     {
     }
 
@@ -356,17 +356,23 @@ struct RectangleAndPart
     {
         std::stringstream ss;
         ss << m_aRectangle.toString().getStr();
-        if (m_nPart != -1)
+        if (m_nPart >= -1)
             ss << ", " << m_nPart;
         return ss.str().c_str();
     }
 
-    /// Infinite Rectangle is when both dimensions are >= 2e7.
-    // ~2 billion twips is INT_MAX, which is full-area.
+    /// Infinite Rectangle is both sides are
+    /// equal or longer than SfxLokHelper::MaxTwips.
     bool isInfinite() const
     {
-        return m_aRectangle.GetWidth() >= 2e7 &&
-               m_aRectangle.GetHeight() >= 2e7;
+        return m_aRectangle.GetWidth() >= SfxLokHelper::MaxTwips &&
+               m_aRectangle.GetHeight() >= SfxLokHelper::MaxTwips;
+    }
+
+    /// Empty Rectangle is when it has zero dimensions.
+    bool isEmpty() const
+    {
+        return m_aRectangle.IsEmpty();
     }
 
     static RectangleAndPart Create(const std::string& rPayload)
@@ -374,7 +380,7 @@ struct RectangleAndPart
         RectangleAndPart aRet;
         if (rPayload.compare(0, 5, "EMPTY") == 0) // payload starts with "EMPTY"
         {
-            aRet.m_aRectangle = Rectangle(0, 0, INT_MAX, INT_MAX);
+            aRet.m_aRectangle = Rectangle(0, 0, SfxLokHelper::MaxTwips, SfxLokHelper::MaxTwips);
             if (comphelper::LibreOfficeKit::isPartInInvalidation())
                 aRet.m_nPart = std::stol(rPayload.substr(6));
 
@@ -382,15 +388,41 @@ struct RectangleAndPart
         }
 
         std::istringstream aStream(rPayload);
-        long nLeft, nTop, nRight, nBottom;
-        long nPart = -1;
+        long nLeft, nTop, nWidth, nHeight;
+        long nPart = INT_MIN;
         char nComma;
         if (comphelper::LibreOfficeKit::isPartInInvalidation())
-            aStream >> nLeft >> nComma >> nTop >> nComma >> nRight >> nComma >> nBottom >> nComma >> nPart;
+        {
+            aStream >> nLeft >> nComma >> nTop >> nComma >> nWidth >> nComma >> nHeight >> nComma >> nPart;
+        }
         else
-            aStream >> nLeft >> nComma >> nTop >> nComma >> nRight >> nComma >> nBottom;
+        {
+            aStream >> nLeft >> nComma >> nTop >> nComma >> nWidth >> nComma >> nHeight;
+        }
+
+        if (nWidth > 0 && nHeight > 0)
+        {
+            // The top-left corner starts at (0, 0).
+            // Anything negative is invalid.
+            if (nLeft < 0)
+            {
+                nWidth += nLeft;
+                nLeft = 0;
+            }
+
+            if (nTop < 0)
+            {
+                nHeight += nTop;
+                nTop = 0;
+            }
+
+            if (nWidth > 0 && nHeight > 0)
+            {
+                aRet.m_aRectangle = Rectangle(nLeft, nTop, nLeft + nWidth, nTop + nHeight);
+            }
+        }
+        // else leave empty rect.
 
-        aRet.m_aRectangle = Rectangle(nLeft, nTop, nLeft + nRight, nTop + nBottom);
         aRet.m_nPart = nPart;
         return aRet;
     }
@@ -607,6 +639,7 @@ void CallbackFlushHandler::queue(const int type, const char* data)
 {
     std::string payload(data ? data : "(nil)");
     //SAL_WARN("lok", "Queue: " << type << " : " << payload);
+
     if (m_bPartTilePainting)
     {
         // We drop notifications when this is set, except for important ones.
@@ -678,24 +711,6 @@ void CallbackFlushHandler::queue(const int type, const char* data)
         break;
     }
 
-    // if we have to invalidate all tiles, we can skip any new tile invalidation
-    if (type == LOK_CALLBACK_INVALIDATE_TILES)
-    {
-        const auto& pos = std::find_if(m_queue.rbegin(), m_queue.rend(),
-                [] (const queue_type::value_type& elem) { return (elem.first == LOK_CALLBACK_INVALIDATE_TILES); });
-
-        if (pos != m_queue.rend())
-        {
-            RectangleAndPart rcOld = RectangleAndPart::Create(pos->second);
-            RectangleAndPart rcNew = RectangleAndPart::Create(payload);
-            if (rcOld.isInfinite() && rcOld.m_nPart == rcNew.m_nPart)
-            {
-                SAL_WARN("lok", "Skipping queue [" << type << "]: [" << payload << "] since all tiles need to be invalidated.");
-                return;
-            }
-        }
-    }
-
     if (type == LOK_CALLBACK_TEXT_SELECTION && payload.empty())
     {
         const auto& posStart = std::find_if(m_queue.rbegin(), m_queue.rend(),
@@ -778,20 +793,50 @@ void CallbackFlushHandler::queue(const int type, const char* data)
             {
                 RectangleAndPart rcNew = RectangleAndPart::Create(payload);
                 //SAL_WARN("lok", "New: " << rcNew.toString());
+                if (rcNew.isEmpty())
+                {
+                    SAL_WARN("lok", "Skipping invalid event [" << type << "]: [" << payload << "].");
+                    return;
+                }
+
+                // If we have to invalidate all tiles, we can skip any new tile invalidation.
+                // Find the last INVALIDATE_TILES entry, if any to see if it's invalidate-all.
+                const auto& pos = std::find_if(m_queue.rbegin(), m_queue.rend(),
+                        [] (const queue_type::value_type& elem) { return (elem.first == LOK_CALLBACK_INVALIDATE_TILES); });
+                if (pos != m_queue.rend())
+                {
+                    RectangleAndPart rcOld = RectangleAndPart::Create(pos->second);
+                    if (rcOld.isInfinite() && (rcOld.m_nPart == -1 || rcOld.m_nPart == rcNew.m_nPart))
+                    {
+                        SAL_WARN("lok", "Skipping queue [" << type << "]: [" << payload << "] since all tiles need to be invalidated.");
+                        return;
+                    }
+
+                    if (rcOld.m_nPart == -1 || rcOld.m_nPart == rcNew.m_nPart)
+                    {
+                        // If fully overlapping.
+                        if (rcOld.m_aRectangle.IsInside(rcNew.m_aRectangle))
+                        {
+                            SAL_WARN("lok", "Skipping queue [" << type << "]: [" << payload << "] since overlaps existing all-parts.");
+                            return;
+                        }
+                    }
+                }
+
                 if (rcNew.isInfinite())
                 {
                     SAL_WARN("lok", "Have Empty [" << type << "]: [" << payload << "] so removing all with part " << rcNew.m_nPart << ".");
                     removeAll(
-                        [type, &rcNew] (const queue_type::value_type& elem) {
-                            if (elem.first == type)
+                        [&rcNew] (const queue_type::value_type& elem) {
+                            if (elem.first == LOK_CALLBACK_INVALIDATE_TILES)
                             {
+                                // Remove exiting if new is all-encompasing, or if of the same part.
                                 const RectangleAndPart rcOld = RectangleAndPart::Create(elem.second);
-                                return (rcOld.m_nPart == rcNew.m_nPart);
-                            }
-                            else
-                            {
-                                return false;
+                                return (rcNew.m_nPart == -1 || rcOld.m_nPart == rcNew.m_nPart);
                             }
+
+                            // Keep others.
+                            return false;
                         }
                     );
                 }
@@ -799,30 +844,55 @@ void CallbackFlushHandler::queue(const int type, const char* data)
                 {
                     const auto rcOrig = rcNew;
 
-                    //SAL_WARN("lok", "Have [" << type << "]: [" << payload << "] so merging overlapping.");
+                    SAL_WARN("lok", "Have [" << type << "]: [" << payload << "] so merging overlapping.");
                     removeAll(
-                        [type, &rcNew] (const queue_type::value_type& elem) {
-                            if (elem.first == type)
+                        [&rcNew] (const queue_type::value_type& elem) {
+                            if (elem.first == LOK_CALLBACK_INVALIDATE_TILES)
                             {
                                 const RectangleAndPart rcOld = RectangleAndPart::Create(elem.second);
-                                if (rcOld.m_nPart != rcNew.m_nPart)
+                                if (rcNew.m_nPart != -1 && rcOld.m_nPart != -1 && rcOld.m_nPart != rcNew.m_nPart)
+                                {
+                                    SAL_WARN("lok", "Nothing to merge between new: " << rcNew.toString() << ", and old: " << rcOld.toString());
                                     return false;
+                                }
 
-                                const Rectangle rcOverlap = rcNew.m_aRectangle.GetIntersection(rcOld.m_aRectangle);
-                                bool bOverlap = (rcOverlap.GetWidth() > 0 && rcOverlap.GetHeight() > 0);
-                                SAL_WARN("lok", "Merging " << rcNew.toString() << " & " << rcOld.toString() << " => " <<
-                                         rcOverlap.toString() << " Overlap: " << bOverlap);
-                                if (bOverlap)
+                                if (rcNew.m_nPart == -1)
                                 {
-                                    rcNew.m_aRectangle.Union(rcOld.m_aRectangle);
-                                    SAL_WARN("lok", "Merged: " << rcNew.toString());
+                                    // Don't merge unless fully overlaped.
+                                    SAL_WARN("lok", "New " << rcNew.toString() << " has " << rcOld.toString() << "?");
+                                    if (rcNew.m_aRectangle.IsInside(rcOld.m_aRectangle))
+                                    {
+                                        SAL_WARN("lok", "New " << rcNew.toString() << " engulfs old " << rcOld.toString() << ".");
+                                        return true;
+                                    }
+                                }
+                                else if (rcOld.m_nPart == -1)
+                                {
+                                    // Don't merge unless fully overlaped.
+                                    SAL_WARN("lok", "Old " << rcOld.toString() << " has " << rcNew.toString() << "?");
+                                    if (rcOld.m_aRectangle.IsInside(rcNew.m_aRectangle))
+                                    {
+                                        SAL_WARN("lok", "New " << rcNew.toString() << " engulfs old " << rcOld.toString() << ".");
+                                        return true;
+                                    }
+                                }
+                                else
+                                {
+                                    const Rectangle rcOverlap = rcNew.m_aRectangle.GetIntersection(rcOld.m_aRectangle);
+                                    const bool bOverlap = !rcOverlap.IsEmpty();
+                                    SAL_WARN("lok", "Merging " << rcNew.toString() << " & " << rcOld.toString() << " => " <<
+                                            rcOverlap.toString() << " Overlap: " << bOverlap);
+                                    if (bOverlap)
+                                    {
+                                        rcNew.m_aRectangle.Union(rcOld.m_aRectangle);
+                                        SAL_WARN("lok", "Merged: " << rcNew.toString());
+                                        return true;
+                                    }
                                 }
-                                return bOverlap;
-                            }
-                            else
-                            {
-                                return false;
                             }
+
+                            // Keep others.
+                            return false;
                         }
                     );
 
@@ -834,10 +904,10 @@ void CallbackFlushHandler::queue(const int type, const char* data)
                         {
                             SAL_WARN("lok", "Error: merged rect smaller.");
                         }
-
-                        payload = rcNew.toString().getStr();
                     }
                 }
+
+                payload = rcNew.toString().getStr();
             }
             break;
 
@@ -862,7 +932,8 @@ void CallbackFlushHandler::queue(const int type, const char* data)
     }
 
     m_queue.emplace_back(type, payload);
-    SAL_WARN("lok", "Queued [" << type << "]: [" << payload << "] to have " << m_queue.size() << " entries.");
+    SAL_WARN("lok", "Queued #" << (m_queue.size() - 1) <<
+             " [" << type << "]: [" << payload << "] to have " << m_queue.size() << " entries.");
 
     lock.unlock();
     if (!IsActive())