include/vcl/region.hxx             |    6 +-
 sw/CppunitTest_sw_uwriter.mk       |    1 
 sw/inc/swregion.hxx                |    3 -
 sw/qa/core/test_region.cxx         |  108 +++++++++++++++++++++++++++++++++++++
 sw/source/core/bastyp/swregion.cxx |   32 +++-------
 sw/source/core/view/viewsh.cxx     |    4 -
 6 files changed, 127 insertions(+), 27 deletions(-)

New commits:
commit 362d2271721dd19de7a7f6f2271a349272ce51d8
Author:     Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Wed Sep 22 16:41:45 2021 +0200
Commit:     Luboš Luňák <l.lu...@collabora.com>
CommitDate: Thu Sep 23 00:08:41 2021 +0200

    add some unittests for SwRegionRects
    
    And fix two small bugs in SwRegionRects::Compress().
    
    Change-Id: I8df7cc2e9d4d6ca78e3af711639e3fbae2e471ae
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122462
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lu...@collabora.com>

diff --git a/sw/CppunitTest_sw_uwriter.mk b/sw/CppunitTest_sw_uwriter.mk
index ef9951ca1f80..7626039ca23e 100644
--- a/sw/CppunitTest_sw_uwriter.mk
+++ b/sw/CppunitTest_sw_uwriter.mk
@@ -20,6 +20,7 @@ $(eval $(call 
gb_CppunitTest_add_exception_objects,sw_uwriter, \
     sw/qa/core/test_ToxLinkProcessor \
     sw/qa/core/test_ToxTextGenerator \
     sw/qa/core/test_ToxMiscTest \
+    sw/qa/core/test_region \
 ))
 
 $(eval $(call gb_CppunitTest_use_library_objects,sw_uwriter,sw))
diff --git a/sw/inc/swregion.hxx b/sw/inc/swregion.hxx
index 1aa190712a8f..338a30649c7a 100644
--- a/sw/inc/swregion.hxx
+++ b/sw/inc/swregion.hxx
@@ -55,8 +55,9 @@ public:
     // Ensures all rectangles are within the origin area.
     void LimitToOrigin();
 
+    enum CompressType { CompressExact, CompressFuzzy };
     // Combine adjacent rectangles.
-    void Compress();
+    void Compress( CompressType type );
 
     const SwRect &GetOrigin() const { return m_aOrigin; }
     void ChangeOrigin( const SwRect &rRect ) { m_aOrigin = rRect; }
diff --git a/sw/qa/core/test_region.cxx b/sw/qa/core/test_region.cxx
new file mode 100644
index 000000000000..15fa398c7e4a
--- /dev/null
+++ b/sw/qa/core/test_region.cxx
@@ -0,0 +1,108 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+
+#include <cppunit/TestAssert.h>
+#include <cppunit/TestFixture.h>
+#include <cppunit/extensions/HelperMacros.h>
+
+#include <swregion.hxx>
+#include <vcl/region.hxx>
+
+class RegionUnittest : public CppUnit::TestFixture
+{
+public:
+    void testCompress();
+    void testInvert();
+
+    CPPUNIT_TEST_SUITE(RegionUnittest);
+    CPPUNIT_TEST(testCompress);
+    CPPUNIT_TEST(testInvert);
+    CPPUNIT_TEST_SUITE_END();
+};
+
+void RegionUnittest::testCompress()
+{
+    SwRegionRects region;
+
+    // All inside each other, check it'll compress them to the largest one.
+    region = SwRegionRects();
+    region += SwRect(Point(10, 10), Size(10, 10));
+    region += SwRect(Point(10, 10), Size(20, 20));
+    region += SwRect(Point(10, 10), Size(100, 100));
+    region += SwRect(Point(10, 10), Size(50, 50));
+    region.Compress(SwRegionRects::CompressExact);
+    CPPUNIT_ASSERT_EQUAL(size_t(1), region.size());
+    CPPUNIT_ASSERT_EQUAL(SwRect(Point(10, 10), Size(100, 100)), region[0]);
+
+    // Check merging of adjacent rectangles. This will merge first two groups
+    // and then those two merged rects only in the next iteration.
+    region = SwRegionRects();
+    region += SwRect(Point(10, 10), Size(10000, 10000));
+    region += SwRect(Point(10010, 10), Size(10000, 10000));
+    region += SwRect(Point(10, 10010), Size(10000, 10000));
+    region += SwRect(Point(10010, 10010), Size(10000, 10000));
+    region.Compress(SwRegionRects::CompressExact);
+    CPPUNIT_ASSERT_EQUAL(size_t(1), region.size());
+    CPPUNIT_ASSERT_EQUAL(SwRect(Point(10, 10), Size(20000, 20000)), region[0]);
+
+    // Check fuzzy compress, two almost aligned rects will be compressed to 
one.
+    region = SwRegionRects();
+    region += SwRect(Point(10, 10), Size(100, 100));
+    region += SwRect(Point(110, 10), Size(100, 90));
+    region.Compress(SwRegionRects::CompressExact);
+    CPPUNIT_ASSERT_EQUAL(size_t(2), region.size());
+    region.Compress(SwRegionRects::CompressFuzzy);
+    CPPUNIT_ASSERT_EQUAL(size_t(1), region.size());
+    CPPUNIT_ASSERT_EQUAL(SwRect(Point(10, 10), Size(200, 100)), region[0]);
+
+    // Check it doesn't crash because of empty size.
+    region = SwRegionRects();
+    region += SwRect(Point(0, 0), Size(0, 0));
+    region += SwRect(Point(10, 10), Size(0, 0));
+    region += SwRect(Point(100, 100), Size(0, 0));
+    region.Compress(SwRegionRects::CompressExact);
+    region.Compress(SwRegionRects::CompressFuzzy);
+}
+
+void RegionUnittest::testInvert()
+{
+    // Check that punching holes and inverting has the same result as adding 
up rects.
+    const SwRect fullRect(Point(100, 100), Size(1000, 1000));
+    const SwRect rects[]
+        = { { Point(200, 200), Size(200, 200) },
+            { Point(0, 0), Size(200, 200) }, // this one is intentionally 
partially out of bounds
+            { Point(700, 760), Size(20, 150) },
+            { Point(100, 300), Size(200, 200) },
+            { Point(100, 800), Size(10, 10) }, // these two partially overlap
+            { Point(105, 805), Size(10, 10) } };
+    SwRegionRects region1(fullRect);
+    for (const SwRect& rect : rects)
+        region1 -= rect;
+    region1.Invert();
+    region1.Compress(SwRegionRects::CompressExact);
+    SwRegionRects region2;
+    region2.ChangeOrigin(fullRect);
+    for (const SwRect& rect : rects)
+        region2 += rect;
+    region2.LimitToOrigin();
+    region2.Compress(SwRegionRects::CompressExact);
+    // The regions should be the same area, but not necessarily the same 
representation.
+    // SwRegionRects cannot compare those easily, but vcl::Region can.
+    vcl::Region vclRegion1, vclRegion2;
+    for (const SwRect& rect : region1)
+        vclRegion1.Union(tools::Rectangle(rect.Pos(), rect.SSize()));
+    for (const SwRect& rect : region2)
+        vclRegion2.Union(tools::Rectangle(rect.Pos(), rect.SSize()));
+    CPPUNIT_ASSERT_EQUAL(vclRegion1, vclRegion2);
+}
+
+CPPUNIT_TEST_SUITE_REGISTRATION(RegionUnittest);
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/bastyp/swregion.cxx 
b/sw/source/core/bastyp/swregion.cxx
index 52dbd6a3a59a..967bc44b2cb2 100644
--- a/sw/source/core/bastyp/swregion.cxx
+++ b/sw/source/core/bastyp/swregion.cxx
@@ -152,7 +152,7 @@ void SwRegionRects::LimitToOrigin()
 }
 
 // combine all adjacent rectangles
-void SwRegionRects::Compress()
+void SwRegionRects::Compress( CompressType type )
 {
     bool bAgain;
     do
@@ -164,8 +164,8 @@ void SwRegionRects::Compress()
             // Rectangles are sorted by Y axis, so check only pairs of 
rectangles
             // that are possibly overlapping or adjacent or close enough to be 
grouped by the fuzzy
             // code below.
-            const tools::Long nFuzzy = 1361513;
-            const tools::Long yMax = (*this)[i].Bottom() + nFuzzy
+            const tools::Long nFuzzy = type == CompressFuzzy ? 1361513 : 0;
+            const tools::Long yMax = (*this)[i].Top() + (*this)[i].Height() + 
nFuzzy
                 / std::max<tools::Long>( 1, (*this)[i].Width());
             size_type j = i+1;
             while( j < size() && (*this)[j].Top() <= yMax )
@@ -188,21 +188,12 @@ void SwRegionRects::Compress()
                 }
                 else
                 {
-                    // TODO: I think the comment below and the code are 
partially incorrect.
-                    // An obvious mistake is the comment saying that one 
rectangle can be deleted,
-                    // while it's the union that gets used instead of the two 
rectangles.
-                    // I think this code is supposed to merge adjacent 
rectangles (possibly
-                    // overlapping), and such rectangles can be detected by 
their merged areas
-                    // being equal to the area of the union (which is 
obviously the case if they
-                    // share one side, and using the nFuzzy extra allow 
merging also rectangles
-                    // that do not quite cover the entire union but it's close 
enough).
-                    // So another mistake seems to be using '- CalcArea( 
aInter )',
-                    // it should be on the other side of the comparison to 
subtract shared area
-                    // counted twice. In practice it seems rectangles here do 
not share areas,
-                    // so the error is irrelevant.
-
-                    // If two rectangles have the same area of their union 
minus the
-                    // intersection then one of them can be deleted.
+                    // Merge adjacent rectangles (possibly overlapping), such 
rectangles can be
+                    // detected by their merged areas being equal to the area 
of the union
+                    // (which is obviously the case if they share one side, 
and using
+                    // the nFuzzy extra allows merging also rectangles that do 
not quite cover
+                    // the entire union but it's close enough).
+
                     // For combining as much as possible (and for having less 
single
                     // paints), the area of the union can be a little bit 
larger:
                     // ( 9622 * 141.5 = 1361513 ~= a quarter (1/4) centimeter 
wider
@@ -211,9 +202,8 @@ void SwRegionRects::Compress()
                     aUnion.Union( (*this)[j] );
                     SwRect aInter( (*this)[i] );
                     aInter.Intersection( (*this)[j] );
-                    if ( (::CalcArea( (*this)[i] ) +
-                          ::CalcArea( (*this)[j] ) + nFuzzy) >=
-                         (::CalcArea( aUnion ) - CalcArea( aInter )) )
+                    if ( CalcArea( (*this)[i] ) + CalcArea( (*this)[j] ) - 
CalcArea( aInter )
+                            + nFuzzy >= CalcArea( aUnion ) )
                     {
                         (*this)[i] = aUnion;
                         erase( begin() + j );
diff --git a/sw/source/core/view/viewsh.cxx b/sw/source/core/view/viewsh.cxx
index 758782b8e3ce..4d510e1cddd8 100644
--- a/sw/source/core/view/viewsh.cxx
+++ b/sw/source/core/view/viewsh.cxx
@@ -330,7 +330,7 @@ void SwViewShell::ImplEndAction( const bool bIdleEnd )
                 SwRootFrame* pCurrentLayout = GetLayout();
 
                 pRegion->LimitToOrigin();
-                pRegion->Compress();
+                pRegion->Compress( SwRegionRects::CompressFuzzy );
 
                 ScopedVclPtr<VirtualDevice> pVout;
                 while ( !pRegion->empty() )
@@ -1663,7 +1663,7 @@ bool SwViewShell::CheckInvalidForPaint( const SwRect 
&rRect )
         if ( pRegion )
         {
             pRegion->LimitToOrigin();
-            pRegion->Compress();
+            pRegion->Compress( SwRegionRects::CompressFuzzy );
             bRet = false;
             if ( !pRegion->empty() )
             {
commit 05f5c12da968bd53337e832805894c048aad2c6c
Author:     Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Wed Sep 22 16:38:12 2021 +0200
Commit:     Luboš Luňák <l.lu...@collabora.com>
CommitDate: Thu Sep 23 00:08:28 2021 +0200

    put vcl::Region operator<< in the vcl namespace
    
    Clang says it belongs there when vcl::Region is used
    in CPPUNIT_ASSERT_EQUAL.
    
    Change-Id: I457d7b35e9b2ca37909118467da8018d40ab18a0
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122461
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lu...@collabora.com>

diff --git a/include/vcl/region.hxx b/include/vcl/region.hxx
index 579d6e2c4fad..8d5004c18efd 100644
--- a/include/vcl/region.hxx
+++ b/include/vcl/region.hxx
@@ -132,11 +132,9 @@ public:
     static vcl::Region GetRegionFromPolyPolygon( const tools::PolyPolygon& 
rPolyPoly );
 };
 
-} /* namespace vcl */
-
 template< typename charT, typename traits >
 inline std::basic_ostream<charT, traits> & operator <<(
-    std::basic_ostream<charT, traits> & stream, const vcl::Region& rRegion)
+    std::basic_ostream<charT, traits> & stream, const Region& rRegion)
 {
     if (rRegion.IsEmpty())
         return stream << "EMPTY";
@@ -162,6 +160,8 @@ inline std::basic_ostream<charT, traits> & operator <<(
     return stream;
 }
 
+} /* namespace vcl */
+
 #endif // INCLUDED_VCL_REGION_HXX
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

Reply via email to