include/test/cppunitasserthelper.hxx | 16 + sc/qa/unit/tiledrendering/data/cell-invalidations-200zoom-settings.ods |binary sc/qa/unit/tiledrendering/tiledrendering.cxx | 90 +++++++++- 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 +-- sw/source/filter/ww8/ww8par3.cxx | 6 test/Library_test.mk | 1 test/source/cppunitasserthelper.cxx | 60 ++++++ 10 files changed, 193 insertions(+), 29 deletions(-)
New commits: commit d63d416f345242e457eb1adebf572b0fb3000b1b Author: Caolán McNamara <[email protected]> AuthorDate: Fri Jan 19 21:11:37 2024 +0000 Commit: Caolán McNamara <[email protected]> CommitDate: Sat Jan 20 12:45:22 2024 +0100 ofz: Abrt from missing form component tweak to follow the typical default config option here Change-Id: I09b92458935ca0772c8593c4e50e59adc60a00f1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162325 Tested-by: Jenkins Reviewed-by: Caolán McNamara <[email protected]> diff --git a/sw/source/filter/ww8/ww8par3.cxx b/sw/source/filter/ww8/ww8par3.cxx index 17370bfad8b6..85563c7d1245 100644 --- a/sw/source/filter/ww8/ww8par3.cxx +++ b/sw/source/filter/ww8/ww8par3.cxx @@ -110,7 +110,7 @@ eF_ResT SwWW8ImplReader::Read_F_FormTextBox( WW8FieldDesc* pF, OUString& rStr ) text. */ - const bool bUseEnhFields = officecfg::Office::Common::Filter::Microsoft::Import::ImportWWFieldsAsEnhancedFields::get(); + const bool bUseEnhFields = m_bFuzzing || officecfg::Office::Common::Filter::Microsoft::Import::ImportWWFieldsAsEnhancedFields::get(); if (!bUseEnhFields) { @@ -191,7 +191,7 @@ eF_ResT SwWW8ImplReader::Read_F_FormCheckBox( WW8FieldDesc* pF, OUString& rStr ) if (rStr[pF->nLCode-1]==0x01) ImportFormulaControl(aFormula,pF->nSCode+pF->nLCode-1, WW8_CT_CHECKBOX); - const bool bUseEnhFields = officecfg::Office::Common::Filter::Microsoft::Import::ImportWWFieldsAsEnhancedFields::get(); + const bool bUseEnhFields = m_bFuzzing || officecfg::Office::Common::Filter::Microsoft::Import::ImportWWFieldsAsEnhancedFields::get(); if (!bUseEnhFields) { @@ -246,7 +246,7 @@ eF_ResT SwWW8ImplReader::Read_F_FormListBox( WW8FieldDesc* pF, OUString& rStr) if (pF->nLCode > 0 && rStr.getLength() >= pF->nLCode && rStr[pF->nLCode-1] == 0x01) ImportFormulaControl(aFormula,pF->nSCode+pF->nLCode-1, WW8_CT_DROPDOWN); - bool bUseEnhFields = officecfg::Office::Common::Filter::Microsoft::Import::ImportWWFieldsAsEnhancedFields::get(); + const bool bUseEnhFields = m_bFuzzing || officecfg::Office::Common::Filter::Microsoft::Import::ImportWWFieldsAsEnhancedFields::get(); if (!bUseEnhFields) { commit 7edfb1b85cef5e5435cd2bf46e1b91b68f1e6427 Author: Caolán McNamara <[email protected]> AuthorDate: Wed Jan 17 21:09:41 2024 +0000 Commit: Caolán McNamara <[email protected]> CommitDate: Sat Jan 20 12:45:13 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 <[email protected]> Reviewed-by: Michael Meeks <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162322 Tested-by: Caolán McNamara <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> diff --git a/include/test/cppunitasserthelper.hxx b/include/test/cppunitasserthelper.hxx index 0fd3806b65fe..a4bb9cd6a79c 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> @@ -74,6 +77,19 @@ inline std::string CPPUNIT_NS::assertion_traits<css::table::CellRangeAddress>::t return ost.str(); } +CPPUNIT_NS_BEGIN + +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 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ 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 44c15e18e1da..2123903fdfeb 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> @@ -98,7 +99,6 @@ public: virtual void setUp() override; virtual void tearDown() override; -protected: ScModelObj* createDoc(const char* pName); void setupLibreOfficeKitViewCallback(SfxViewShell* pViewShell); static void callback(int nType, const char* pPayload, void* pData); @@ -3354,6 +3354,94 @@ CPPUNIT_TEST_FIXTURE(ScTiledRenderingTest, testNoInvalidateOnSave) CPPUNIT_ASSERT(!aView.m_bInvalidateTiles); } +// That we don't end up with two views on different zooms that invalidate different +// rectangles, each should invalidate the same rectangle +CPPUNIT_TEST_FIXTURE(ScTiledRenderingTest, testCellInvalidationDocWithExistingZoom) +{ + ScAddress aB7(1, 6, 0); + ScopedVclPtrInstance<VirtualDevice> xDevice(DeviceFormat::WITHOUT_ALPHA); + + ScModelObj* pModelObj = createDoc("cell-invalidations-200zoom-settings.ods"); + CPPUNIT_ASSERT(pModelObj); + ScTabViewShell* pView = dynamic_cast<ScTabViewShell*>(SfxViewShell::Current()); + CPPUNIT_ASSERT(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_FIXTURE(ScTiledRenderingTest, testStatusBarLocale) { // Given 2 views, the second's locale is set to German: diff --git a/sc/source/ui/inc/tabview.hxx b/sc/source/ui/inc/tabview.hxx index d825eca1f1d8..9c6a2138e6d7 100644 --- a/sc/source/ui/inc/tabview.hxx +++ b/sc/source/ui/inc/tabview.hxx @@ -632,6 +632,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 be90caece8fe..7b8002bd194b 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 71a37ddce9da..1bf2b710ba01 100644 --- a/sc/source/ui/view/tabview.cxx +++ b/sc/source/ui/view/tabview.cxx @@ -1719,9 +1719,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(); @@ -1791,9 +1789,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(); @@ -2211,9 +2207,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 4a78aa38e710..444b78b20b64 100644 --- a/sc/source/ui/view/tabview3.cxx +++ b/sc/source/ui/view/tabview3.cxx @@ -2052,11 +2052,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(); @@ -3100,6 +3096,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()); @@ -3110,18 +3120,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 268a68744eaf..c51bd826e658 100644 --- a/test/Library_test.mk +++ b/test/Library_test.mk @@ -56,6 +56,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: */
