include/test/cppunitasserthelper.hxx | 12 +
sc/qa/unit/tiledrendering/data/cell-invalidations-200zoom-settings.ods |binary
sc/qa/unit/tiledrendering/tiledrendering.cxx | 88
++++++++++
sc/source/ui/inc/tabview.hxx | 2
sc/source/ui/unoobj/docuno.cxx | 2
sc/source/ui/view/tabview.cxx | 12 -
sc/source/ui/view/tabview3.cxx | 33
+--
test/Library_test.mk | 1
test/source/cppunitasserthelper.cxx | 60
++++++
9 files changed, 185 insertions(+), 25 deletions(-)
New commits:
commit 4dc98ca3f7ef7c3b331cce4594793e2d7a2a147d
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Wed Jan 17 21:09:41 2024 +0000
Commit: Michael Meeks <michael.me...@collabora.com>
CommitDate: Fri Jan 19 20:56:50 2024 +0100
calc doc with saved zoom in settings causes disjoint invalidations...
in multiple views, while the invalidations rectangles should be reported
at the same place from each view.
on load, ScTabView::SetZoom gets called with the zoom stored in the
document settings, which both calls sets Zoom on the ViewData, and then
calls ZoomChanged, which syncs the GridWindows MapMode from the ViewData
derived GetDrawMapMode().
Later lok sets zoom via setClientArea which leaves the GridWindows
MapMode untouched and out of sync with the newly changed ViewData
MapMode.
Typically then, on e.g. on deleting text in one view then
ScViewFunc::DeleteContents or similar is called which calls
ScTabView::UpdateCopySourceOverlay which calls
ScGridWindow::UpdateCopySourceOverlay and that sets the GridWindow
MapMode to the DrawMapMode but then *for lokit* returns early (among a
few other unlikely early return cases) while every other similar func
restores the orig GridWindow mode before returning. So the view which
is used to make the change ends up with GridWindows synced to the
ViewData MapMode, which looks like accident. While the other view
remains with GridWindows with MapModes unsynced with that views
ViewData MapMode.
So on invalidate, the view that was used to make the change has
GridWindows with MapModes that report the correct rectangle, while the
other unsynced view will report an incorrect rectangle, until something
happens in that view to get it to exec UpdateCopySourceOverlay and get
synced.
Here add the sync to ScModelObj::setClientZoom so the two MapModes
remain synced once that is called.
Change-Id: I2da59f50ae2b0e3ea6b7ef8b54debdab1ee98266
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162312
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Michael Meeks <michael.me...@collabora.com>
diff --git a/include/test/cppunitasserthelper.hxx
b/include/test/cppunitasserthelper.hxx
index b6703e255ce0..6d2f64ce23d8 100644
--- a/include/test/cppunitasserthelper.hxx
+++ b/include/test/cppunitasserthelper.hxx
@@ -10,12 +10,15 @@
#ifndef INCLUDED_TEST_CPPUNITASSERTHELPER_HXX
#define INCLUDED_TEST_CPPUNITASSERTHELPER_HXX
+#include <test/testdllapi.hxx>
+
#include <rtl/ustring.hxx>
#include <com/sun/star/awt/Point.hpp>
#include <com/sun/star/awt/Size.hpp>
#include <com/sun/star/table/CellAddress.hpp>
#include <com/sun/star/table/CellRangeAddress.hpp>
+#include <tools/gen.hxx>
#include <cppunit/TestAssert.h>
@@ -97,6 +100,15 @@ template <> struct
assertion_traits<css::table::CellRangeAddress>
}
};
+void OOO_DLLPUBLIC_TEST AssertRectEqualWithTolerance(std::string_view sInfo,
+ const tools::Rectangle&
rExpected,
+ const tools::Rectangle&
rActual,
+ const sal_Int32
nTolerance);
+
+void OOO_DLLPUBLIC_TEST AssertPointEqualWithTolerance(std::string_view sInfo,
const Point rExpected,
+ const Point rActual,
+ const sal_Int32
nTolerance);
+
CPPUNIT_NS_END
#endif // INCLUDED_TEST_CPPUNITASSERTHELPER_HXX
diff --git
a/sc/qa/unit/tiledrendering/data/cell-invalidations-200zoom-settings.ods
b/sc/qa/unit/tiledrendering/data/cell-invalidations-200zoom-settings.ods
new file mode 100644
index 000000000000..741441800e55
Binary files /dev/null and
b/sc/qa/unit/tiledrendering/data/cell-invalidations-200zoom-settings.ods differ
diff --git a/sc/qa/unit/tiledrendering/tiledrendering.cxx
b/sc/qa/unit/tiledrendering/tiledrendering.cxx
index b3813aa1c85c..55e72325f772 100644
--- a/sc/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sc/qa/unit/tiledrendering/tiledrendering.cxx
@@ -7,6 +7,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
+#include <test/cppunitasserthelper.hxx>
#include <test/unoapixml_test.hxx>
#include <test/helper/transferable.hxx>
#include <cppunit/tools/StringHelper.h>
@@ -165,6 +166,7 @@ public:
void testLongFirstColumnMouseClick();
void testSidebarLocale();
void testNoInvalidateOnSave();
+ void testCellInvalidationDocWithExistingZoom();
CPPUNIT_TEST_SUITE(ScTiledRenderingTest);
CPPUNIT_TEST(testRowColumnHeaders);
@@ -240,6 +242,7 @@ public:
CPPUNIT_TEST(testLongFirstColumnMouseClick);
CPPUNIT_TEST(testSidebarLocale);
CPPUNIT_TEST(testNoInvalidateOnSave);
+ CPPUNIT_TEST(testCellInvalidationDocWithExistingZoom);
CPPUNIT_TEST_SUITE_END();
private:
@@ -3688,6 +3691,91 @@ void ScTiledRenderingTest::testNoInvalidateOnSave()
CPPUNIT_ASSERT(!aView.m_bInvalidateTiles);
}
+void ScTiledRenderingTest::testCellInvalidationDocWithExistingZoom()
+{
+ ScAddress aB7(1, 6, 0);
+ ScopedVclPtrInstance<VirtualDevice> xDevice(DeviceFormat::DEFAULT);
+
+ ScModelObj* pModelObj =
createDoc("cell-invalidations-200zoom-settings.ods");
+ ScTabViewShell* pView =
dynamic_cast<ScTabViewShell*>(SfxViewShell::Current());
+ CPPUNIT_ASSERT(pModelObj && pView);
+
+ // Set View #1 to initial 100% and generate a paint
+ pModelObj->setClientVisibleArea(tools::Rectangle(0, 0, 19845, 6405));
+ pModelObj->setClientZoom(256, 256, 1536, 1536);
+ pModelObj->paintTile(*xDevice, 3328, 512, 0, 0, 19968, 3072);
+
+ Scheduler::ProcessEventsToIdle();
+
+ int nView1 = SfxLokHelper::getView();
+ // register to track View #1 invalidations
+ ViewCallback aView1;
+
+ // Create a View #2
+ SfxLokHelper::createView();
+ int nView2 = SfxLokHelper::getView();
+
pModelObj->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>());
+ // register to track View #1 invalidations
+ ViewCallback aView2;
+
+ // Set View #2 to initial 100% and generate a paint
+ pModelObj->setClientVisibleArea(tools::Rectangle(0, 0, 19845, 6405));
+ pModelObj->setClientZoom(256, 256, 1536, 1536);
+ pModelObj->paintTile(*xDevice, 3328, 512, 0, 0, 19968, 3072);
+
+ // Set View #1 to 50% zoom and generate a paint
+ SfxLokHelper::setView(nView1);
+ pModelObj->setClientVisibleArea(tools::Rectangle(0, 0, 41150, 13250));
+ pModelObj->setClientZoom(256, 256, 3185, 3185);
+ pModelObj->paintTile(*xDevice, 3328, 512, 0, 0, 41405, 6370);
+
+ Scheduler::ProcessEventsToIdle();
+
+ // Set View #2 to 200% zoom and generate a paint
+ SfxLokHelper::setView(nView2);
+ pModelObj->setClientVisibleArea(tools::Rectangle(0, 0, 9574, 3090));
+ pModelObj->setClientZoom(256, 256, 741, 741);
+ pModelObj->paintTile(*xDevice, 3328, 512, 0, 0, 19968, 3072);
+
+ Scheduler::ProcessEventsToIdle();
+ aView1.m_bInvalidateTiles = false;
+ aView1.m_aInvalidations.clear();
+ aView2.m_bInvalidateTiles = false;
+ aView2.m_aInvalidations.clear();
+
+ ScTabViewShell* pView2 =
dynamic_cast<ScTabViewShell*>(SfxViewShell::Current());
+ CPPUNIT_ASSERT(pView2);
+ pView2->SetCursor(aB7.Col(), aB7.Row());
+
+ pModelObj->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, awt::Key::DELETE);
+ pModelObj->postKeyEvent(LOK_KEYEVENT_KEYUP, 0, awt::Key::DELETE);
+ Scheduler::ProcessEventsToIdle();
+
+ // The problem tested for here is with two views at different zooms then a
+ // single cell invalidation resulted in the same rectangle reported as two
+ // different invalidations rectangles of different scales. While we should
+ // get the same invalidation rectangle reported.
+ //
+ // (B7 is a good choice to use in the real world to see the effect, to both
+ // avoid getting the two rects combined into one bigger one, or to have the
+ // two separated by so much space the 2nd is off-screen and not seen
+ CPPUNIT_ASSERT_EQUAL(size_t(1), aView1.m_aInvalidations.size());
+ CPPUNIT_ASSERT_EQUAL(size_t(1), aView2.m_aInvalidations.size());
+
+ // That they don't exactly match doesn't matter, we're not checking
rounding issues,
+ // what matters is that they are not utterly different rectangles
+ // Without fix result is originally:
+ // Comparing invalidation rects Left expected 278 actual 1213 Tolerance 50
+ CppUnit::AssertPointEqualWithTolerance("Comparing invalidations topleft",
+ aView1.m_aInvalidations[0].TopLeft(),
+ aView2.m_aInvalidations[0].TopLeft(),
+ 100);
+ CppUnit::AssertPointEqualWithTolerance("Comparing invalidations
bottomleft",
+
aView1.m_aInvalidations[0].BottomLeft(),
+
aView2.m_aInvalidations[0].BottomLeft(),
+ 100);
+}
+
}
CPPUNIT_TEST_SUITE_REGISTRATION(ScTiledRenderingTest);
diff --git a/sc/source/ui/inc/tabview.hxx b/sc/source/ui/inc/tabview.hxx
index 52686170030f..648c9189a3a0 100644
--- a/sc/source/ui/inc/tabview.hxx
+++ b/sc/source/ui/inc/tabview.hxx
@@ -624,6 +624,8 @@ public:
SCROW GetLOKEndHeaderRow() const { return mnLOKEndHeaderRow; }
SCCOL GetLOKStartHeaderCol() const { return mnLOKStartHeaderCol; }
SCCOL GetLOKEndHeaderCol() const { return mnLOKEndHeaderCol; }
+
+ void SyncGridWindowMapModeFromDrawMapMode();
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/ui/unoobj/docuno.cxx b/sc/source/ui/unoobj/docuno.cxx
index 75f869ea9a53..6b4b8b9734f0 100644
--- a/sc/source/ui/unoobj/docuno.cxx
+++ b/sc/source/ui/unoobj/docuno.cxx
@@ -1072,6 +1072,8 @@ void ScModelObj::setClientZoom(int nTilePixelWidth_, int
nTilePixelHeight_, int
return;
pViewData->SetZoom(newZoomX, newZoomY, true);
+ if (ScTabViewShell* pViewShell = pViewData->GetViewShell())
+ pViewShell->SyncGridWindowMapModeFromDrawMapMode();
// refresh our view's take on other view's cursors & selections
pViewData->GetActiveWin()->updateKitOtherCursors();
diff --git a/sc/source/ui/view/tabview.cxx b/sc/source/ui/view/tabview.cxx
index 206c637e648e..2dcc360a8bef 100644
--- a/sc/source/ui/view/tabview.cxx
+++ b/sc/source/ui/view/tabview.cxx
@@ -1659,9 +1659,7 @@ void ScTabView::DoHSplit(tools::Long nSplitPos)
// Form Layer needs to know the visible part of all windows
// that is why MapMode must already be correct here
- for (VclPtr<ScGridWindow> & pWin : pGridWin)
- if (pWin)
- pWin->SetMapMode( pWin->GetDrawMapMode() );
+ SyncGridWindowMapModeFromDrawMapMode();
SetNewVisArea();
PaintGrid();
@@ -1731,9 +1729,7 @@ void ScTabView::DoVSplit(tools::Long nSplitPos)
// Form Layer needs to know the visible part of all windows
// that is why MapMode must already be correct here
- for (VclPtr<ScGridWindow> & pWin : pGridWin)
- if (pWin)
- pWin->SetMapMode( pWin->GetDrawMapMode() );
+ SyncGridWindowMapModeFromDrawMapMode();
SetNewVisArea();
PaintGrid();
@@ -2151,9 +2147,7 @@ void ScTabView::FreezeSplitters( bool bFreeze,
SplitMethod eSplitMethod, SCCOLRO
// Form Layer needs to know the visible part of all windows
// that is why MapMode must already be correct here
- for (VclPtr<ScGridWindow> & p : pGridWin)
- if (p)
- p->SetMapMode( p->GetDrawMapMode() );
+ SyncGridWindowMapModeFromDrawMapMode();
SetNewVisArea();
RepeatResize(bUpdateFix);
diff --git a/sc/source/ui/view/tabview3.cxx b/sc/source/ui/view/tabview3.cxx
index 545c42e12b4d..8fdd5f5a3a33 100644
--- a/sc/source/ui/view/tabview3.cxx
+++ b/sc/source/ui/view/tabview3.cxx
@@ -2017,11 +2017,7 @@ void ScTabView::SetTabNo( SCTAB nTab, bool bNew, bool
bExtendSelection, bool bSa
// Form Layer must know the visible area of the new sheet
// that is why MapMode must already be correct here
- for (VclPtr<ScGridWindow> & pWin : pGridWin)
- {
- if (pWin)
- pWin->SetMapMode(pWin->GetDrawMapMode());
- }
+ SyncGridWindowMapModeFromDrawMapMode();
SetNewVisArea();
PaintGrid();
@@ -3062,6 +3058,20 @@ void ScTabView::UpdateInputLine()
SC_MOD()->InputEnterHandler();
}
+void ScTabView::SyncGridWindowMapModeFromDrawMapMode()
+{
+ // AW: Discussed with NN if there is a reason that new map mode was only
set for one window,
+ // but is not. Setting only on one window causes the first repaint to have
the old mapMode
+ // in three of four views, so the overlay will save the wrong content e.g.
when zooming out.
+ // Changing to setting map mode at all windows.
+ for (VclPtr<ScGridWindow> & pWin : pGridWin)
+ {
+ if (!pWin)
+ continue;
+ pWin->SetMapMode(pWin->GetDrawMapMode());
+ }
+}
+
void ScTabView::ZoomChanged()
{
ScInputHandler* pHdl = SC_MOD()->GetInputHdl(aViewData.GetViewShell());
@@ -3072,18 +3082,9 @@ void ScTabView::ZoomChanged()
UpdateScrollBars();
- // VisArea...
- // AW: Discussed with NN if there is a reason that new map mode was only
set for one window,
- // but is not. Setting only on one window causes the first repaint to have
the old mapMode
- // in three of four views, so the overlay will save the wrong content e.g.
when zooming out.
- // Changing to setting map mode at all windows.
-
- for (sal_uInt32 i = 0; i < 4; i++)
- {
- if (pGridWin[i])
- pGridWin[i]->SetMapMode(pGridWin[i]->GetDrawMapMode());
- }
+ SyncGridWindowMapModeFromDrawMapMode();
+ // VisArea...
SetNewVisArea();
InterpretVisible(); // have everything calculated before painting
diff --git a/test/Library_test.mk b/test/Library_test.mk
index 951ddf6edab2..31785285f4b1 100644
--- a/test/Library_test.mk
+++ b/test/Library_test.mk
@@ -57,6 +57,7 @@ $(eval $(call gb_Library_add_exception_objects,test,\
test/source/helper/form \
test/source/helper/shape \
test/source/helper/transferable \
+ test/source/cppunitasserthelper \
))
# vim: set noet sw=4 ts=4:
diff --git a/test/source/cppunitasserthelper.cxx
b/test/source/cppunitasserthelper.cxx
new file mode 100644
index 000000000000..b3119b5b26d8
--- /dev/null
+++ b/test/source/cppunitasserthelper.cxx
@@ -0,0 +1,60 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4;
fill-column: 100 -*- */
+/*
+ * 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 <test/cppunitasserthelper.hxx>
+
+CPPUNIT_NS_BEGIN
+
+void AssertRectEqualWithTolerance(std::string_view sInfo, const
tools::Rectangle& rExpected,
+ const tools::Rectangle& rActual, const
sal_Int32 nTolerance)
+{
+ // Left
+ OString sMsg = OString::Concat(sInfo) + " Left expected " +
OString::number(rExpected.Left())
+ + " actual " + OString::number(rActual.Left()) + "
Tolerance "
+ + OString::number(nTolerance);
+ CPPUNIT_ASSERT_MESSAGE(sMsg.getStr(),
+ std::abs(rExpected.Left() - rActual.Left()) <=
nTolerance);
+
+ // Top
+ sMsg = OString::Concat(sInfo) + " Top expected " +
OString::number(rExpected.Top()) + " actual "
+ + OString::number(rActual.Top()) + " Tolerance " +
OString::number(nTolerance);
+ CPPUNIT_ASSERT_MESSAGE(sMsg.getStr(), std::abs(rExpected.Top() -
rActual.Top()) <= nTolerance);
+
+ // Width
+ sMsg = OString::Concat(sInfo) + " Width expected " +
OString::number(rExpected.GetWidth())
+ + " actual " + OString::number(rActual.GetWidth()) + " Tolerance "
+ + OString::number(nTolerance);
+ CPPUNIT_ASSERT_MESSAGE(sMsg.getStr(),
+ std::abs(rExpected.GetWidth() - rActual.GetWidth())
<= nTolerance);
+
+ // Height
+ sMsg = OString::Concat(sInfo) + " Height expected " +
OString::number(rExpected.GetHeight())
+ + " actual " + OString::number(rActual.GetHeight()) + " Tolerance "
+ + OString::number(nTolerance);
+ CPPUNIT_ASSERT_MESSAGE(sMsg.getStr(),
+ std::abs(rExpected.GetHeight() -
rActual.GetHeight()) <= nTolerance);
+}
+
+void AssertPointEqualWithTolerance(std::string_view sInfo, const Point
rExpected,
+ const Point rActual, const sal_Int32
nTolerance)
+{
+ // X
+ OString sMsg = OString::Concat(sInfo) + " X expected " +
OString::number(rExpected.X())
+ + " actual " + OString::number(rActual.X()) + " Tolerance "
+ + OString::number(nTolerance);
+ CPPUNIT_ASSERT_MESSAGE(sMsg.getStr(), std::abs(rExpected.X() -
rActual.X()) <= nTolerance);
+ // Y
+ sMsg = OString::Concat(sInfo) + " Y expected " +
OString::number(rExpected.Y()) + " actual "
+ + OString::number(rActual.Y()) + " Tolerance " +
OString::number(nTolerance);
+ CPPUNIT_ASSERT_MESSAGE(sMsg.getStr(), std::abs(rExpected.Y() -
rActual.Y()) <= nTolerance);
+}
+
+CPPUNIT_NS_END
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
