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: */