include/tools/gen.hxx | 11 ++++++++--
sc/qa/unit/tiledrendering/tiledrendering.cxx | 6 ++---
sd/qa/unit/tiledrendering/LOKitSearchTest.cxx | 28 +++++++++++++-------------
tools/qa/cppunit/test_rectangle.cxx | 24 ++++++++++++++++++++++
4 files changed, 50 insertions(+), 19 deletions(-)
New commits:
commit 17f524a37c81a05e6d448a7186df858a69ada635
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Aug 17 11:18:39 2021 +0300
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Tue Aug 17 22:32:22 2021 +0200
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.kagan...@collabora.com>
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);
