Fix o3tl::convert for Rectangle, to operate on right/bottom values

... instead of using confusing/ambiguous size having two interpretations.

This reverts some of the unit test changes made in commit
fa339b3adb53300ae68913bed87e18caf9f2e262.

Change-Id: Ic56417703e32c1d92bcee76ad8ff494824bd4a1f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120564
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

4 files changed, 50 insertions, 19 deletions

diff --git a/include/tools/gen.hxx b/include/tools/gen.hxx
index a9cf170952e6..e68b850b78cb 100644
--- a/include/tools/gen.hxx
+++ b/include/tools/gen.hxx
@@ -722,8 +722,15 @@ namespace o3tl
 
 constexpr tools::Rectangle convert(const tools::Rectangle& rRectangle, o3tl::Length eFrom, o3tl::Length eTo)
 {
-    return tools::Rectangle(o3tl::convert(rRectangle.TopLeft(), eFrom, eTo),
-                            o3tl::convert(rRectangle.GetSize(), eFrom, eTo));
+    // 1. Create an empty rectangle with correct left and top
+    tools::Rectangle aRect(o3tl::convert(rRectangle.Left(), eFrom, eTo),
+                           o3tl::convert(rRectangle.Top(), eFrom, eTo));
+    // 2. If source has width/heigth, set respective right and bottom
+    if (!rRectangle.IsWidthEmpty())
+        aRect.SetRight(o3tl::convert(rRectangle.Right(), eFrom, eTo));
+    if (!rRectangle.IsHeightEmpty())
+        aRect.SetBottom(o3tl::convert(rRectangle.Bottom(), eFrom, eTo));
+    return aRect;
 }
 
 } // end o3tl
diff --git a/sc/qa/unit/tiledrendering/tiledrendering.cxx b/sc/qa/unit/tiledrendering/tiledrendering.cxx
index 18a01a051a17..bc9259b9dc84 100644
--- a/sc/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sc/qa/unit/tiledrendering/tiledrendering.cxx
@@ -1225,7 +1225,7 @@ void ScTiledRenderingTest::testInvalidateOnInserRowCol()
     Scheduler::ProcessEventsToIdle();
     CPPUNIT_ASSERT(aView.m_bInvalidateTiles);
     CPPUNIT_ASSERT_EQUAL(size_t(2), aView.m_aInvalidations.size());
-    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(-75, 50985, 32212230, 63989), aView.m_aInvalidations[0]);
+    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(-75, 50985, 32212230, 63990), aView.m_aInvalidations[0]);
 
     // move on the right
     for (int i = 0; i < 200; ++i)
@@ -1242,7 +1242,7 @@ void ScTiledRenderingTest::testInvalidateOnInserRowCol()
     Scheduler::ProcessEventsToIdle();
     CPPUNIT_ASSERT(aView.m_bInvalidateTiles);
     CPPUNIT_ASSERT_EQUAL(size_t(2), aView.m_aInvalidations.size());
-    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(253650, -15, 32212230, 63989), aView.m_aInvalidations[0]);
+    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(253650, -15, 32212230, 63990), aView.m_aInvalidations[0]);
 }
 
 void ScTiledRenderingTest::testCommentCallback()
@@ -1963,7 +1963,7 @@ void ScTiledRenderingTest::testSheetChangeInvalidation()
     Scheduler::ProcessEventsToIdle();
     CPPUNIT_ASSERT(aView1.m_bInvalidateTiles);
     CPPUNIT_ASSERT_EQUAL(size_t(2), aView1.m_aInvalidations.size());
-    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(0, 0, 1310719, 268435455), aView1.m_aInvalidations[0]);
+    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(0, 0, 1310720, 268435456), aView1.m_aInvalidations[0]);
     CPPUNIT_ASSERT_EQUAL(tools::Rectangle(0, 0, 1000000000, 1000000000), aView1.m_aInvalidations[1]);
     CPPUNIT_ASSERT_EQUAL(size_t(1), aView1.m_aInvalidationsParts.size());
     CPPUNIT_ASSERT_EQUAL(pModelObj->getPart(), aView1.m_aInvalidationsParts[0]);
diff --git a/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx b/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx
index dd661553b425..762faaac1b56 100644
--- a/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx
+++ b/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx
@@ -347,9 +347,9 @@ void LOKitSearchTest::testSearchInPDF()
     CPPUNIT_ASSERT_EQUAL(1, mpCallbackRecorder->m_nSearchResultCount);
 
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultSelection.size());
-    CPPUNIT_ASSERT_EQUAL(OString("3763, 1331, 1432, 482"),
+    CPPUNIT_ASSERT_EQUAL(OString("3763, 1331, 1432, 483"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
-    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(Point(3763, 1331), Size(1433, 483)),
+    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(Point(3763, 1331), Size(1433, 484)),
                          mpCallbackRecorder->m_aSelection[0]);
 
     // Search again - same result
@@ -359,9 +359,9 @@ void LOKitSearchTest::testSearchInPDF()
     CPPUNIT_ASSERT_EQUAL(2, mpCallbackRecorder->m_nSearchResultCount);
 
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultSelection.size());
-    CPPUNIT_ASSERT_EQUAL(OString("3763, 1331, 1432, 482"),
+    CPPUNIT_ASSERT_EQUAL(OString("3763, 1331, 1432, 483"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
-    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(Point(3763, 1331), Size(1433, 483)),
+    CPPUNIT_ASSERT_EQUAL(tools::Rectangle(Point(3763, 1331), Size(1433, 484)),
                          mpCallbackRecorder->m_aSelection[0]);
 }
 
@@ -444,7 +444,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePages()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(0, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("9463, 3382, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("9463, 3382, 1099, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him"
@@ -457,7 +457,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePages()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(0, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("5592, 5038, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("5592, 5038, 1100, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him"
@@ -470,7 +470,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePages()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(1, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("9463, 1308, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("9463, 1308, 1099, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him"
@@ -483,7 +483,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePages()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(1, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("5592, 2964, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("5592, 2964, 1100, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him" - back to start
@@ -496,7 +496,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePages()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(0, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("9463, 3382, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("9463, 3382, 1099, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 }
 
@@ -547,7 +547,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePagesBackwards()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(0, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("5592, 5038, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("5592, 5038, 1100, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him"
@@ -560,7 +560,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePagesBackwards()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(0, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("9463, 3382, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("9463, 3382, 1099, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him"
@@ -573,7 +573,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePagesBackwards()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(1, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("5592, 2964, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("5592, 2964, 1100, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him"
@@ -586,7 +586,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePagesBackwards()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(1, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("9463, 1308, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("9463, 1308, 1099, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 
     // Search for "him" - back to start
@@ -599,7 +599,7 @@ void LOKitSearchTest::testSearchInPDFInMultiplePagesBackwards()
     CPPUNIT_ASSERT_EQUAL(size_t(1), mpCallbackRecorder->m_aSearchResultPart.size());
 
     CPPUNIT_ASSERT_EQUAL(0, mpCallbackRecorder->m_aSearchResultPart[0]);
-    CPPUNIT_ASSERT_EQUAL(OString("5592, 5038, 1099, 498"),
+    CPPUNIT_ASSERT_EQUAL(OString("5592, 5038, 1100, 499"),
                          mpCallbackRecorder->m_aSearchResultSelection[0]);
 }
 
diff --git a/tools/qa/cppunit/test_rectangle.cxx b/tools/qa/cppunit/test_rectangle.cxx
index 29aa6cde94df..e213ed28daf8 100644
--- a/tools/qa/cppunit/test_rectangle.cxx
+++ b/tools/qa/cppunit/test_rectangle.cxx
@@ -68,6 +68,30 @@ void Test::test_rectangle()
         aRect2.SetSize(Size(-1, -2));
         CPPUNIT_ASSERT_EQUAL(aRect, aRect2);
     }
+
+    {
+        constexpr tools::Rectangle aRectTwip(100, 100, 100, 100);
+        constexpr tools::Rectangle aRectMm100(
+            o3tl::convert(aRectTwip, o3tl::Length::twip, o3tl::Length::mm100));
+        static_assert(!aRectMm100.IsEmpty());
+        // Make sure that we use coordinates for conversion, not width/height:
+        // the latter is ambiguous, and e.g. GetWidth(aRectTwip) gives 1, which
+        // would had been converted to 2, resulting in different LR coordinates
+        static_assert(aRectMm100.Left() == aRectMm100.Right());
+        static_assert(aRectMm100.Top() == aRectMm100.Bottom());
+    }
+
+    {
+        constexpr tools::Rectangle aRectTwip(1, 1);
+        constexpr tools::Rectangle aRectMm100(
+            o3tl::convert(aRectTwip, o3tl::Length::twip, o3tl::Length::mm100));
+        // Make sure that result keeps the empty flag
+        static_assert(aRectMm100.IsEmpty());
+        static_assert(aRectMm100.IsWidthEmpty());
+        static_assert(aRectMm100.IsHeightEmpty());
+        static_assert(aRectMm100.GetWidth() == 0);
+        static_assert(aRectMm100.GetHeight() == 0);
+    }
 }
 
 CPPUNIT_TEST_SUITE_REGISTRATION(Test);