[Libreoffice-commits] core.git: Branch 'libreoffice-7-2' - 5 commits - cui/source editeng/source sc/qa sc/source writerfilter/source

Vasily Melenchuk (via logerrit) logerrit at kemper.freedesktop.org
Thu Aug 19 12:32:50 UTC 2021


 cui/source/dialogs/tipofthedaydlg.cxx                    |   10 +
 cui/source/tabpages/numpages.cxx                         |    6 -
 editeng/source/items/numitem.cxx                         |    2 
 sc/qa/unit/data/ods/many_charts.ods                      |binary
 sc/qa/unit/subsequent_export-test2.cxx                   |   90 ++++++++++++++-
 sc/source/filter/excel/xeescher.cxx                      |   16 +-
 sc/source/filter/inc/xeescher.hxx                        |    3 
 sc/source/ui/inc/gridwin.hxx                             |    8 -
 sc/source/ui/view/gridwin4.cxx                           |   73 +++++++-----
 sc/source/ui/view/tabview5.cxx                           |    5 
 writerfilter/source/dmapper/DomainMapperTableManager.cxx |    2 
 writerfilter/source/dmapper/TableManager.hxx             |    2 
 12 files changed, 166 insertions(+), 51 deletions(-)

New commits:
commit e08fba90f9b593b1a0a7af44bae1d7da0404d3f1
Author:     Vasily Melenchuk <vasily.melenchuk at cib.de>
AuthorDate: Fri Aug 13 16:56:06 2021 +0300
Commit:     Vasily Melenchuk <vasily.melenchuk at cib.de>
CommitDate: Thu Aug 19 14:28:11 2021 +0200

    tdf#143858: sw: default value for nInclUpperLevels is 1
    
    SvxNumberFormat::nInclUpperLevels (matches text:display-levels in ODF)
    bit incorrect in its name: is counts total amount of levels to display,
    including current level. So value "0" seems have no sense: display 0
    levels in total?
    
    In UI you can't select less than 1 level and ODF standard (19.797)
    using 1 as a default. This looks plausable.
    
    Change-Id: I596386c7b3cc4370910cd0ff6e927e501179fbdf
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120458
    Tested-by: Jenkins
    Reviewed-by: Thorsten Behrens <thorsten.behrens at allotropia.de>
    (cherry picked from commit 37dd6e18f5cf498d230ffe8a0a395cfdf9625e0c)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120645
    Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>

diff --git a/cui/source/tabpages/numpages.cxx b/cui/source/tabpages/numpages.cxx
index 47542d147853..8c14c9607fe1 100644
--- a/cui/source/tabpages/numpages.cxx
+++ b/cui/source/tabpages/numpages.cxx
@@ -690,7 +690,7 @@ IMPL_LINK_NOARG(SvxNumPickTabPage, NumSelectHdl_Impl, ValueSet*, void)
         }
         else
         {
-            aFmt.SetIncludeUpperLevels(sal::static_int_cast< sal_uInt8 >(0 != nUpperLevelOrChar ? pActNum->GetLevelCount() : 0));
+            aFmt.SetIncludeUpperLevels(sal::static_int_cast< sal_uInt8 >(0 != nUpperLevelOrChar ? pActNum->GetLevelCount() : 1));
             aFmt.SetCharFormatName(sNumCharFmtName);
             aFmt.SetBulletRelSize(100);
             // #i93908#
@@ -1641,7 +1641,7 @@ IMPL_LINK(SvxNumOptionsTabPage, NumberTypeSelectHdl_Impl, weld::ComboBox&, rBox,
             if(SVX_NUM_BITMAP == (nNumberingType&(~LINK_TOKEN)))
             {
                 bBmp |= nullptr != aNumFmt.GetBrush();
-                aNumFmt.SetIncludeUpperLevels( 0 );
+                aNumFmt.SetIncludeUpperLevels( 1 );
                 aNumFmt.SetListFormat("", "", i);
                 if(!bBmp)
                     aNumFmt.SetGraphic("");
@@ -1651,7 +1651,7 @@ IMPL_LINK(SvxNumOptionsTabPage, NumberTypeSelectHdl_Impl, weld::ComboBox&, rBox,
             }
             else if( SVX_NUM_CHAR_SPECIAL == nNumberingType )
             {
-                aNumFmt.SetIncludeUpperLevels( 0 );
+                aNumFmt.SetIncludeUpperLevels( 1 );
                 aNumFmt.SetListFormat("", "", i);
                 if( !aNumFmt.GetBulletFont() )
                     aNumFmt.SetBulletFont(&aActBulletFont);
diff --git a/editeng/source/items/numitem.cxx b/editeng/source/items/numitem.cxx
index 9ce62c11dc16..ebd934a6742f 100644
--- a/editeng/source/items/numitem.cxx
+++ b/editeng/source/items/numitem.cxx
@@ -164,7 +164,7 @@ void SvxNumberType::dumpAsXml( xmlTextWriterPtr pWriter ) const
 SvxNumberFormat::SvxNumberFormat( SvxNumType eType )
     : SvxNumberType(eType),
       eNumAdjust(SvxAdjust::Left),
-      nInclUpperLevels(0),
+      nInclUpperLevels(1),
       nStart(1),
       cBullet(SVX_DEF_BULLET),
       nBulletRelSize(100),
commit 4b9836e37410f36093951080f057f8061a681e70
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Wed Aug 18 23:13:05 2021 +0300
Commit:     Michael Stahl <michael.stahl at allotropia.de>
CommitDate: Thu Aug 19 12:26:20 2021 +0200

    tdf#142264: make sure to load potentially unloaded objects when saving
    
    Commit 574eec9036c5f185b3572ba1e0ca9d111eb361dc happened to reveal
    a pre-existing problem that XLSX export only saved those OLE objects
    that were kept loaded in the OLE object cache, subject to thevalue of
    org.openoffice.Office.Common/Cache/DrawingEngine/OLE_Objects.
    
    Before that change, the imported charts were marked modified on load,
    and that prevented them from unloading in OLEObjCache::UnloadCheckHdl,
    because SdrOle2Obj::CanUnloadRunningObj returned false.
    
    After the mentioned change, the charts started to load without the
    wrong "modified" state, which allowed them to be properly managed by
    the cache, and the export filter implementation error surfaced. It's
    likely that commit 692878e3bb83c0fc104c5cca946c25ccf2d84ab2 tried to
    workaround the same underlying problem for charts that for some reason
    / at some point in time didn't get marked modified on load, and that
    commit converted an error shown in Excel into silently missing charts.
    
    This change makes sure that whenever a reference to chart document
    is requested from XclExpChartObj, it is actually loaded and ready
    for reading data.
    
    Possibly something could be done on the level of old reference that
    becomes non-functional (although valid) as the result of unloading,
    so that it would automatically reload on following use. That would
    make operating on the references robust. I didn't find an obvious
    way to do that.
    
    It is interesting to investigate, it the heizenbug related to images
    disappearing from documents, as users keep reporting without robust
    reproducers, might possibly be caused by a similar problem.
    
    Change-Id: I45fcdc98254157d805c7519340b5265526f27166
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120688
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
    (cherry picked from commit 420e834007ca654db9803030726edb32c3ba5710)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120640
    Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>

diff --git a/sc/qa/unit/data/ods/many_charts.ods b/sc/qa/unit/data/ods/many_charts.ods
new file mode 100644
index 000000000000..31acdf66e1ed
Binary files /dev/null and b/sc/qa/unit/data/ods/many_charts.ods differ
diff --git a/sc/qa/unit/subsequent_export-test2.cxx b/sc/qa/unit/subsequent_export-test2.cxx
index 8a1dc168fe26..a67648215744 100644
--- a/sc/qa/unit/subsequent_export-test2.cxx
+++ b/sc/qa/unit/subsequent_export-test2.cxx
@@ -72,7 +72,12 @@
 #include <svl/zformat.hxx>
 
 #include <test/xmltesttools.hxx>
-#include <com/sun/star/drawing/XDrawPageSupplier.hpp>
+#include <com/sun/star/chart2/XChartDocument.hpp>
+#include <com/sun/star/chart2/XChartTypeContainer.hpp>
+#include <com/sun/star/chart2/XCoordinateSystemContainer.hpp>
+#include <com/sun/star/drawing/XDrawPage.hpp>
+#include <com/sun/star/drawing/XDrawPages.hpp>
+#include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
 #include <com/sun/star/awt/XBitmap.hpp>
 #include <com/sun/star/frame/Desktop.hpp>
 #include <com/sun/star/graphic/GraphicType.hpp>
@@ -191,6 +196,7 @@ public:
     void testTdf140431();
     void testTdf142929_filterLessThanXLSX();
     void testTdf143220XLSX();
+    void testTdf142264ManyChartsToXLSX();
 
     CPPUNIT_TEST_SUITE(ScExportTest2);
 
@@ -290,6 +296,7 @@ public:
     CPPUNIT_TEST(testTdf140431);
     CPPUNIT_TEST(testTdf142929_filterLessThanXLSX);
     CPPUNIT_TEST(testTdf143220XLSX);
+    CPPUNIT_TEST(testTdf142264ManyChartsToXLSX);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -2370,6 +2377,87 @@ void ScExportTest2::testTdf143220XLSX()
     xDocSh->DoClose();
 }
 
+void ScExportTest2::testTdf142264ManyChartsToXLSX()
+{
+    // The cache size for the test should be small enough, to make sure that some charts get
+    // unloaded in the process, and then loaded on demand properly (default is currently 20)
+    CPPUNIT_ASSERT_LESS(sal_Int32(40),
+                        officecfg::Office::Common::Cache::DrawingEngine::OLE_Objects::get());
+
+    ScDocShellRef xDocSh = loadDoc(u"many_charts.", FORMAT_ODS);
+    CPPUNIT_ASSERT(xDocSh.is());
+    xDocSh = saveAndReload(xDocSh.get(), FORMAT_XLSX);
+    CPPUNIT_ASSERT(xDocSh.is());
+
+    auto xModel = xDocSh->GetModel();
+    css::uno::Reference<css::drawing::XDrawPagesSupplier> xSupplier(xModel,
+                                                                    css::uno::UNO_QUERY_THROW);
+    auto xDrawPages = xSupplier->getDrawPages();
+
+    // No charts (or other objects) on the first sheet, and resp. first draw page
+    css::uno::Reference<css::drawing::XDrawPage> xPage(xDrawPages->getByIndex(0),
+                                                       css::uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), xPage->getCount());
+
+    // 20 charts on the second sheet, and resp. second draw page
+    xPage.set(xDrawPages->getByIndex(1), css::uno::UNO_QUERY_THROW);
+    // Without the fix in place, this test would have failed with
+    // - Expected: 20
+    // - Actual : 0
+    // Because only the last 20 charts would get exported, all on the third sheet
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(20), xPage->getCount());
+    for (sal_Int32 i = 0; i < xPage->getCount(); ++i)
+    {
+        css::uno::Reference<css::beans::XPropertySet> xProps(xPage->getByIndex(i),
+                                                             css::uno::UNO_QUERY_THROW);
+        css::uno::Reference<css::chart2::XChartDocument> xChart(xProps->getPropertyValue("Model"),
+                                                                css::uno::UNO_QUERY_THROW);
+        const auto xDiagram = xChart->getFirstDiagram();
+        CPPUNIT_ASSERT(xDiagram);
+
+        css::uno::Reference<css::chart2::XCoordinateSystemContainer> xCooSysContainer(
+            xDiagram, uno::UNO_QUERY_THROW);
+
+        const auto xCooSysSeq = xCooSysContainer->getCoordinateSystems();
+        for (const auto& rCooSys : xCooSysSeq)
+        {
+            css::uno::Reference<css::chart2::XChartTypeContainer> xChartTypeCont(
+                rCooSys, uno::UNO_QUERY_THROW);
+            uno::Sequence<uno::Reference<chart2::XChartType>> xChartTypeSeq
+                = xChartTypeCont->getChartTypes();
+            CPPUNIT_ASSERT(xChartTypeSeq.hasElements());
+        }
+    }
+
+    // 20 charts on the third sheet, and resp. third draw page
+    xPage.set(xDrawPages->getByIndex(2), css::uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(20), xPage->getCount());
+    for (sal_Int32 i = 0; i < xPage->getCount(); ++i)
+    {
+        css::uno::Reference<css::beans::XPropertySet> xProps(xPage->getByIndex(i),
+                                                             css::uno::UNO_QUERY_THROW);
+        css::uno::Reference<css::chart2::XChartDocument> xChart(xProps->getPropertyValue("Model"),
+                                                                css::uno::UNO_QUERY_THROW);
+        const auto xDiagram = xChart->getFirstDiagram();
+        CPPUNIT_ASSERT(xDiagram);
+
+        css::uno::Reference<css::chart2::XCoordinateSystemContainer> xCooSysContainer(
+            xDiagram, uno::UNO_QUERY_THROW);
+
+        const auto xCooSysSeq = xCooSysContainer->getCoordinateSystems();
+        for (const auto& rCooSys : xCooSysSeq)
+        {
+            css::uno::Reference<css::chart2::XChartTypeContainer> xChartTypeCont(
+                rCooSys, uno::UNO_QUERY_THROW);
+            uno::Sequence<uno::Reference<chart2::XChartType>> xChartTypeSeq
+                = xChartTypeCont->getChartTypes();
+            CPPUNIT_ASSERT(xChartTypeSeq.hasElements());
+        }
+    }
+
+    xDocSh->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(ScExportTest2);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sc/source/filter/excel/xeescher.cxx b/sc/source/filter/excel/xeescher.cxx
index 1bc1a753acad..451242445d5d 100644
--- a/sc/source/filter/excel/xeescher.cxx
+++ b/sc/source/filter/excel/xeescher.cxx
@@ -1385,13 +1385,10 @@ XclExpChartObj::XclExpChartObj( XclExpObjectManager& rObjMgr, Reference< XShape
 
     // create the chart substream object
     ScfPropertySet aShapeProp( xShape );
-    Reference< XModel > xModel;
-    aShapeProp.GetProperty( xModel, "Model" );
-    mxChartDoc.set( xModel,UNO_QUERY );
     css::awt::Rectangle aBoundRect;
     aShapeProp.GetProperty( aBoundRect, "BoundRect" );
     tools::Rectangle aChartRect( Point( aBoundRect.X, aBoundRect.Y ), Size( aBoundRect.Width, aBoundRect.Height ) );
-    mxChart = std::make_shared<XclExpChart>( GetRoot(), xModel, aChartRect );
+    mxChart = std::make_shared<XclExpChart>(GetRoot(), GetChartDoc(), aChartRect);
 }
 
 XclExpChartObj::~XclExpChartObj()
@@ -1417,7 +1414,7 @@ void XclExpChartObj::SaveXml( XclExpXmlStream& rStrm )
     if (xPropSet.is())
     {
         XclObjAny::WriteFromTo( rStrm, mxShape, GetTab() );
-        ChartExport aChartExport(XML_xdr, pDrawing, mxChartDoc, &rStrm, drawingml::DOCUMENT_XLSX);
+        ChartExport aChartExport(XML_xdr, pDrawing, GetChartDoc(), &rStrm, drawingml::DOCUMENT_XLSX);
         auto pURLTransformer = std::make_shared<ScURLTransformer>(*mpDoc);
         aChartExport.SetURLTranslator(pURLTransformer);
         static sal_Int32 nChartCount = 0;
@@ -1434,9 +1431,14 @@ void XclExpChartObj::SaveXml( XclExpXmlStream& rStrm )
     pDrawing->endElement( FSNS( XML_xdr, XML_twoCellAnchor ) );
 }
 
-const css::uno::Reference<css::chart::XChartDocument>& XclExpChartObj::GetChartDoc() const
+css::uno::Reference<css::chart::XChartDocument> XclExpChartObj::GetChartDoc() const
 {
-    return mxChartDoc;
+    SdrObject* pObj = SdrObject::getSdrObjectFromXShape(mxShape);
+    if (!pObj || pObj->GetObjIdentifier() != OBJ_OLE2)
+        return {};
+    // May load here - makes sure that we are working with actually loaded OLE object
+    return css::uno::Reference<css::chart::XChartDocument>(
+        static_cast<SdrOle2Obj*>(pObj)->getXModel(), css::uno::UNO_QUERY);
 }
 
 XclExpNote::XclExpNote(const XclExpRoot& rRoot, const ScAddress& rScPos,
diff --git a/sc/source/filter/inc/xeescher.hxx b/sc/source/filter/inc/xeescher.hxx
index b6ff9e562f15..bd57edb13587 100644
--- a/sc/source/filter/inc/xeescher.hxx
+++ b/sc/source/filter/inc/xeescher.hxx
@@ -316,13 +316,12 @@ public:
     virtual void        Save( XclExpStream& rStrm ) override;
     virtual void        SaveXml( XclExpXmlStream& rStrm ) override;
 
-    const css::uno::Reference<css::chart::XChartDocument>& GetChartDoc() const;
+    css::uno::Reference<css::chart::XChartDocument> GetChartDoc() const;
 
 private:
     typedef std::shared_ptr< XclExpChart > XclExpChartRef;
     XclExpChartRef                                    mxChart;        /// The chart itself (BOF/EOF substream data).
     css::uno::Reference< css::drawing::XShape >       mxShape;
-    css::uno::Reference< css::chart::XChartDocument > mxChartDoc;
     ScDocument*                                       mpDoc;
 };
 
commit 7d81ad58fd6fccec02ddf2523f41716663e32dd0
Author:     Caolán McNamara <caolanm at redhat.com>
AuthorDate: Wed Aug 18 15:51:35 2021 +0100
Commit:     Michael Stahl <michael.stahl at allotropia.de>
CommitDate: Thu Aug 19 12:11:51 2021 +0200

    save LastTipOfTheDayShown when the dialog is shown not in its dtor
    
    Otherwise if a tip dialog is left open then its considered not to be
    have be shown and new ones will constantly open on every new
    document. Especially problematic in impress if you leave the first
    one open and select a template to open from the template dialog then
    you get two of them in the same frame.
    
    Change-Id: I94cd34aa031e133d8c229a0de78582fda1dbdf4a
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120638
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>

diff --git a/cui/source/dialogs/tipofthedaydlg.cxx b/cui/source/dialogs/tipofthedaydlg.cxx
index 8356f6f0e36f..c4d1f876f935 100644
--- a/cui/source/dialogs/tipofthedaydlg.cxx
+++ b/cui/source/dialogs/tipofthedaydlg.cxx
@@ -74,6 +74,15 @@ TipOfTheDayDialog::TipOfTheDayDialog(weld::Window* pParent)
 
     const auto t0 = std::chrono::system_clock::now().time_since_epoch();
     m_nDay = std::chrono::duration_cast<std::chrono::hours>(t0).count() / 24;
+
+    // save this time to the config now instead of in the dtor otherwise we
+    // end up with multiple copies of this dialog every time we open a new
+    // document if the first one isn't closed
+    std::shared_ptr<comphelper::ConfigurationChanges> xChanges(
+        comphelper::ConfigurationChanges::create());
+    officecfg::Office::Common::Misc::LastTipOfTheDayShown::set(m_nDay, xChanges);
+    xChanges->commit();
+
     if (m_nDay > officecfg::Office::Common::Misc::LastTipOfTheDayShown::get())
         m_nCurrentTip++;
 
@@ -90,7 +99,6 @@ TipOfTheDayDialog::~TipOfTheDayDialog()
 {
     std::shared_ptr<comphelper::ConfigurationChanges> xChanges(
         comphelper::ConfigurationChanges::create());
-    officecfg::Office::Common::Misc::LastTipOfTheDayShown::set(m_nDay, xChanges);
     officecfg::Office::Common::Misc::LastTipOfTheDayID::set(m_nCurrentTip, xChanges);
     officecfg::Office::Common::Misc::ShowTipOfTheDay::set(m_pShowTip->get_active(), xChanges);
     xChanges->commit();
commit 8c099c96936f136a1f76ac76259e14416e866980
Author:     Justin Luth <justin_luth at sil.org>
AuthorDate: Sat Aug 14 20:33:33 2021 +0200
Commit:     László Németh <nemeth at numbertext.org>
CommitDate: Thu Aug 19 12:09:45 2021 +0200

    related tdf#134569 writerfilter: negative means table end
    
    TableManager's EndParagraph uses mnTableDepthNew - mnTableDepth
    to identify that a positive is startLevel,
    and a negative is endLevel.
    So it doesn't make much sense to have this function
    return a huge unsigned int in case of a negative.
    
    As expected, an assert proves that LN_CT_TcPrBase_tcW
    can happen for both positive and negative,
    so the equivalent test is just a non-zero.
    An assert proves that startLevel always has
    a positive difference, so that clause can stay as is.
    
    Change-Id: I1b49dfae7087258e4ceed5fb45da0e62fd1f3b50
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120525
    Tested-by: Jenkins
    Reviewed-by: Justin Luth <justin_luth at sil.org>
    (cherry picked from commit 33d588ab553652637e90ecd543c1ffa6301c762b)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120588
    Reviewed-by: László Németh <nemeth at numbertext.org>

diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.cxx b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
index 35749d60bd59..e0f0ae18a160 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
@@ -331,7 +331,7 @@ bool DomainMapperTableManager::sprm(Sprm & rSprm)
                         else
                             // store the original value to limit rounding mistakes, if it's there in a recognized measure (twip)
                             getCurrentCellWidths()->push_back(pMeasureHandler->getMeasureValue() ? pMeasureHandler->getValue() : sal_Int32(0));
-                        if (getTableDepthDifference() > 0)
+                        if (getTableDepthDifference())
                             m_bPushCurrentWidth = true;
                     }
                 }
diff --git a/writerfilter/source/dmapper/TableManager.hxx b/writerfilter/source/dmapper/TableManager.hxx
index aa611e412b59..e6600d35793d 100644
--- a/writerfilter/source/dmapper/TableManager.hxx
+++ b/writerfilter/source/dmapper/TableManager.hxx
@@ -358,7 +358,7 @@ protected:
     /**
        Return the current table difference, i.e. 1 if we are in the first cell of a new table, etc.
      */
-    sal_uInt32 getTableDepthDifference() const { return mnTableDepthNew - mnTableDepth; }
+    sal_Int32 getTableDepthDifference() const { return mnTableDepthNew - mnTableDepth; }
 
     sal_uInt32 getTableDepth() const { return mnTableDepthNew; }
 
commit 3044912f5b1dba19c59e91901da0551c161f5fb4
Author:     Eike Rathke <erack at redhat.com>
AuthorDate: Wed Aug 18 23:42:45 2021 +0200
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Thu Aug 19 09:56:36 2021 +0200

    Do not count pages for initial page breaks, tdf#124983 follow-up
    
    Use a loaded page size or leave it. Otherwise the previous
    implementation could had lead to tremendous waiting time blocking
    everything on large data without. See source code comment.
    
    Also trigger updating page breaks only for one grid window, not
    multiple repeating everything all over.
    
    Remove bSetup parameter that does nothing but either doing something or
    nothing, check in caller instead.
    
    Move member variables to where they belong.
    
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120689
    Reviewed-by: Eike Rathke <erack at redhat.com>
    Tested-by: Jenkins
    (cherry picked from commit f58f35b2c8ca1efbacec642a8f3de5b0c499bc6b)
    
     Conflicts:
            sc/source/ui/inc/gridwin.hxx
            sc/source/ui/view/gridwin4.cxx
    
    Change-Id: I5efc321e5bc5af075a77631aa9d94b0c50ae6b6b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120691
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/sc/source/ui/inc/gridwin.hxx b/sc/source/ui/inc/gridwin.hxx
index daf6c35b652c..50109106aa5d 100644
--- a/sc/source/ui/inc/gridwin.hxx
+++ b/sc/source/ui/inc/gridwin.hxx
@@ -200,6 +200,8 @@ class SAL_DLLPUBLIC_RTTI ScGridWindow : public vcl::Window, public DropTargetHel
 
     RfCorner                aRFSelectedCorned;
 
+    Timer                   maShowPageBreaksTimer;
+
     bool                    bEEMouse:1;               // Edit Engine has mouse
     bool                    bDPMouse:1;               // DataPilot D&D (new Pivot table)
     bool                    bRFMouse:1;               // RangeFinder drag
@@ -210,6 +212,7 @@ class SAL_DLLPUBLIC_RTTI ScGridWindow : public vcl::Window, public DropTargetHel
     bool                    bNeedsRepaint:1;
     bool                    bAutoMarkVisible:1;
     bool                    bListValButton:1;
+    bool                    bInitialPageBreaks:1;
 
     DECL_LINK( PopupModeEndHdl, weld::Popover&, void );
     DECL_LINK( PopupSpellingHdl, SpellCallbackInfo&, void );
@@ -309,10 +312,9 @@ class SAL_DLLPUBLIC_RTTI ScGridWindow : public vcl::Window, public DropTargetHel
     void            InvalidateLOKViewCursor(const tools::Rectangle& rCursorRect,
                                             const Fraction aScaleX, const Fraction aScaleY);
 
-    Timer           maShowPageBreaksTimer;
-    bool            bInitialPageBreaks;
-    void            SetupInitialPageBreaks(ScDocument& rDoc, SCTAB nTab, bool bSetup);
+    void            SetupInitialPageBreaks(const ScDocument& rDoc, SCTAB nTab);
     DECL_LINK(InitiatePageBreaksTimer, Timer*, void);
+
 protected:
     virtual void    PrePaint(vcl::RenderContext& rRenderContext) override;
     virtual void    Paint(vcl::RenderContext& rRenderContext, const tools::Rectangle& rRect) override;
diff --git a/sc/source/ui/view/gridwin4.cxx b/sc/source/ui/view/gridwin4.cxx
index b17e34c8586c..239a9ef581ce 100644
--- a/sc/source/ui/view/gridwin4.cxx
+++ b/sc/source/ui/view/gridwin4.cxx
@@ -1271,29 +1271,27 @@ void ScGridWindow::DrawContent(OutputDevice &rDevice, const ScTableInfo& rTableI
     if (mpNoteMarker)
         mpNoteMarker->Draw(); // Above the cursor, in drawing map mode
 
-    SetupInitialPageBreaks(rDoc, nTab, bPage&& bInitialPageBreaks);
+    if (bPage && bInitialPageBreaks)
+        SetupInitialPageBreaks(rDoc, nTab);
 }
 
 
-void ScGridWindow::SetupInitialPageBreaks(ScDocument& rDoc, SCTAB nTab, bool bSetup)
+void ScGridWindow::SetupInitialPageBreaks(const ScDocument& rDoc, SCTAB nTab)
 {
     // tdf#124983, if option LibreOfficeDev Calc/View/Visual Aids/Page breaks
     // is enabled, breaks should be visible. If the document is opened the first
     // time, the breaks are not calculated yet, so for this initialization
     // a timer will be triggered here.
-    if (bSetup)
-    {
-        std::set<SCCOL> aColBreaks;
-        std::set<SCROW> aRowBreaks;
-        rDoc.GetAllColBreaks(aColBreaks, nTab, true, false);
-        rDoc.GetAllRowBreaks(aRowBreaks, nTab, true, false);
-        if (aColBreaks.size() == 0 || aRowBreaks.size() == 0)
-        {
-            maShowPageBreaksTimer.SetPriority(TaskPriority::DEFAULT_IDLE);
-            maShowPageBreaksTimer.Start();
-            bInitialPageBreaks = false;
-        }
+    std::set<SCCOL> aColBreaks;
+    std::set<SCROW> aRowBreaks;
+    rDoc.GetAllColBreaks(aColBreaks, nTab, true, false);
+    rDoc.GetAllRowBreaks(aRowBreaks, nTab, true, false);
+    if (aColBreaks.size() == 0 || aRowBreaks.size() == 0)
+    {
+        maShowPageBreaksTimer.SetPriority(TaskPriority::DEFAULT_IDLE);
+        maShowPageBreaksTimer.Start();
     }
+    bInitialPageBreaks = false;
 }
 
 namespace
@@ -2350,27 +2348,42 @@ IMPL_LINK(ScGridWindow, InitiatePageBreaksTimer, Timer*, pTimer, void)
 {
     if (pTimer == &maShowPageBreaksTimer)
     {
-        ScDocument& rDoc = mrViewData.GetDocument();
         const ScViewOptions& rOpts = mrViewData.GetOptions();
-        bool bPage = rOpts.GetOption(VOPT_PAGEBREAKS);
-        ScDocShell* pDocSh = mrViewData.GetDocShell();
-        bool bModified = pDocSh->IsModified();
-        // tdf#124983, if option LibreOfficeDev Calc/View/Visual Aids/Page breaks
-        // is enabled, breaks should be visible. If the document is opened the first
-        // time or a tab is activated the first time, the breaks are not calculated
-        // yet, so this initialization is done here.
+        const bool bPage = rOpts.GetOption(VOPT_PAGEBREAKS);
+        // tdf#124983, if option LibreOfficeDev Calc/View/Visual Aids/Page
+        // breaks is enabled, breaks should be visible. If the document is
+        // opened the first time or a tab is activated the first time, the
+        // breaks are not calculated yet, so this initialization is done here.
         if (bPage)
         {
-            SCTAB nCurrentTab = mrViewData.GetTabNo();
-            Size pagesize = rDoc.GetPageSize(nCurrentTab);
-            if (pagesize.IsEmpty())
+            const SCTAB nCurrentTab = mrViewData.GetTabNo();
+            ScDocument& rDoc = mrViewData.GetDocument();
+            const Size aPageSize = rDoc.GetPageSize(nCurrentTab);
+            // Do not attempt to calculate a page size here if it is empty if
+            // that involves counting pages.
+            // An earlier implementation did
+            //   ScPrintFunc(pDocSh, pDocSh->GetPrinter(), nCurrentTab);
+            //   rDoc.SetPageSize(nCurrentTab, rDoc.GetPageSize(nCurrentTab));
+            // which resulted in tremendous waiting times after having loaded
+            // larger documents i.e. imported from CSV, in which UI is entirely
+            // blocked. All time is spent under ScPrintFunc::CountPages() in
+            // ScTable::ExtendPrintArea() in the loop that calls
+            // MaybeAddExtraColumn() to do stuff for each text string content
+            // cell (each row in each column). Maybe that can be optimized, or
+            // obtaining page size without that overhead would be possible, but
+            // as is calling that from here is a no-no so this is a quick
+            // disable things.
+            if (!aPageSize.IsEmpty())
             {
-                ScPrintFunc(pDocSh, pDocSh->GetPrinter(), nCurrentTab);
-                rDoc.SetPageSize(nCurrentTab, rDoc.GetPageSize(nCurrentTab));
+                ScDocShell* pDocSh = mrViewData.GetDocShell();
+                const bool bModified = pDocSh->IsModified();
+                // Even setting the same size sets page size valid, so
+                // UpdatePageBreaks() actually does something.
+                rDoc.SetPageSize( nCurrentTab, aPageSize);
+                rDoc.UpdatePageBreaks(nCurrentTab);
+                pDocSh->PostPaint(0, 0, nCurrentTab, rDoc.MaxCol(), rDoc.MaxRow(), nCurrentTab, PaintPartFlags::Grid);
+                pDocSh->SetModified(bModified);
             }
-            rDoc.UpdatePageBreaks(nCurrentTab);
-            pDocSh->PostPaint(0, 0, nCurrentTab, rDoc.MaxCol(), rDoc.MaxRow(), nCurrentTab, PaintPartFlags::Grid);
-            pDocSh->SetModified(bModified);
         }
     }
 }
diff --git a/sc/source/ui/view/tabview5.cxx b/sc/source/ui/view/tabview5.cxx
index f0b6ed6c4f18..bfb2f6c82ed1 100644
--- a/sc/source/ui/view/tabview5.cxx
+++ b/sc/source/ui/view/tabview5.cxx
@@ -321,11 +321,14 @@ void ScTabView::TabChanged( bool bSameTabButMoved )
     }
 
     for (int i = 0; i < 4; i++)
+    {
         if (pGridWin[i])
         {
             pGridWin[i]->initiatePageBreaks();
+            // Trigger calculating page breaks only once.
+            break;
         }
-
+    }
 
     if (!comphelper::LibreOfficeKit::isActive())
         return;


More information about the Libreoffice-commits mailing list