sw/qa/extras/layout/data/many-paragraphs-per-page-in-6-columns.odt |binary sw/qa/extras/layout/layout5.cxx | 34 +++ sw/qa/extras/ooxmlexport/ooxmlexport12.cxx | 2 sw/source/core/layout/laycache.cxx | 101 +--------- sw/source/core/layout/layhelp.hxx | 2 sw/source/core/layout/tabfrm.cxx | 4 6 files changed, 56 insertions(+), 87 deletions(-)
New commits: commit 9ec73c650091e03e84eba8bc6ed90ebccc68d0bb Author: Mike Kaganski <[email protected]> AuthorDate: Fri Jun 6 02:05:21 2025 +0500 Commit: Xisco Fauli <[email protected]> CommitDate: Tue Jun 10 08:50:19 2025 +0200 tdf#166871: drop mnMaxParaPerPage hack It was there since commit 84a3db80b4fd66c6854b3135b5f69b61fd828e62 (initial import, 2000-09-18); and its idea was to use some heuristics to predict the likely number of pages (when there was no metadata, to use that number for progress indicator), and, most importantly, to predict the page breaks (where a paragraph must be moved to the next page, just because there are many paragraphs on this page already; it likely was considered an optimization). But that hack, in fact, creates page breaks in random places, and then requires moving stuff back; in some cases, the resulting layout was instable, and layout loop control triggered. And that broke the most straightforward way of arranging pages, from start to end. This change removes that hack. The speedup that the hack could potentially provide would be negligible, if at all; and the damage is real. I don't know if the following part of documentation comment for the constructor of SwLayHelper is still relevant after this change: "If there's no layout cache, the distribution to the pages is more a guess, but a guess with statistical background." testObjectCrossReference needed a fields update, for unclear reason. SwTabFrame::Split needed to be improvedto fix failing TestTdf150606, where a zero-height column body tried to layout its table; in this case, in SwTabFrame::Split pRow was pointing to Lower(), nRowCount was 0, nRepeat was 0, and bSplitRowAllowed was false -> it chose "bKeepNextRow" path, moved pRow to its next, split it, and a wrong single-cell table appeared on page 3. This seems to pre-exist, and only happened to be manifested in a test by the change. Change-Id: I2d203055e67b0155bcd719efb1f8a8b6a82b1156 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186243 Reviewed-by: Mike Kaganski <[email protected]> Tested-by: Jenkins Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186289 diff --git a/sw/qa/extras/layout/data/many-paragraphs-per-page-in-6-columns.odt b/sw/qa/extras/layout/data/many-paragraphs-per-page-in-6-columns.odt new file mode 100644 index 000000000000..83f8943e1db9 Binary files /dev/null and b/sw/qa/extras/layout/data/many-paragraphs-per-page-in-6-columns.odt differ diff --git a/sw/qa/extras/layout/layout5.cxx b/sw/qa/extras/layout/layout5.cxx index 91456defc00e..5de4f1d29932 100644 --- a/sw/qa/extras/layout/layout5.cxx +++ b/sw/qa/extras/layout/layout5.cxx @@ -1740,6 +1740,40 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter5, testTdf166691) assertXPath(pXmlDoc, "//page", 1); } +CPPUNIT_TEST_FIXTURE(SwLayoutWriter5, testTdf166871) +{ + // Given a document with 6-column-layout pages, having hundreds of small paragraphs per page + // (normal and headings); its metadata has the number of pages, paragraphs, etc.: + createSwDoc("many-paragraphs-per-page-in-6-columns.odt"); + auto pXmlDoc = parseLayoutDump(); + + // Check the layout - i.e., the count of paragraphs per column per page. + // The metadata in the document must not prevent the correct layout. + + assertXPath(pXmlDoc, "//page", 12); + + // The first 11 pages have the same paragraph count in each column + for (int page = 1; page <= 11; ++page) + { + OString Xpath1 = "//page[" + OString::number(page) + "]/body/column"; + assertXPath(pXmlDoc, Xpath1, 6); + for (int column = 1; column <= 6; ++column) + { + OString Xpath2 = Xpath1 + "[" + OString::number(column) + "]/body/txt"; + assertXPath(pXmlDoc, Xpath2, 47); + } + } + + // Check the last page + assertXPath(pXmlDoc, "//page[12]/body/column", 6); + assertXPath(pXmlDoc, "//page[12]/body/column[1]/body/txt", 14); + assertXPath(pXmlDoc, "//page[12]/body/column[2]/body/txt", 0); + assertXPath(pXmlDoc, "//page[12]/body/column[3]/body/txt", 0); + assertXPath(pXmlDoc, "//page[12]/body/column[4]/body/txt", 0); + assertXPath(pXmlDoc, "//page[12]/body/column[5]/body/txt", 0); + assertXPath(pXmlDoc, "//page[12]/body/column[6]/body/txt", 0); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx index af32d3ebf216..2423e22e93a7 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx @@ -18,6 +18,7 @@ #include <com/sun/star/text/XTextTablesSupplier.hpp> #include <com/sun/star/text/XTextTable.hpp> #include <com/sun/star/document/XViewDataSupplier.hpp> +#include <com/sun/star/util/XRefreshable.hpp> #include <o3tl/string_view.hxx> class Test : public SwModelTestBase @@ -468,6 +469,7 @@ CPPUNIT_TEST_FIXTURE(Test, testObjectCrossReference) uno::Reference<text::XTextFieldsSupplier> xTextFieldsSupplier(mxComponent, uno::UNO_QUERY); uno::Reference<container::XEnumerationAccess> xFieldsAccess( xTextFieldsSupplier->getTextFields()); + xFieldsAccess.queryThrow<util::XRefreshable>()->refresh(); uno::Reference<container::XEnumeration> xFields(xFieldsAccess->createEnumeration()); CPPUNIT_ASSERT(xFields->hasMoreElements()); diff --git a/sw/source/core/layout/laycache.cxx b/sw/source/core/layout/laycache.cxx index 75ccf1ac4686..79be4e2147af 100644 --- a/sw/source/core/layout/laycache.cxx +++ b/sw/source/core/layout/laycache.cxx @@ -528,8 +528,6 @@ SwLayHelper::SwLayHelper( SwDoc *pD, SwFrame* &rpF, SwFrame* &rpP, SwPageFrame* , mrpActualSection( rpA ) , mbBreakAfter(false) , mpDoc(pD) - , mnMaxParaPerPage( 25 ) - , mnParagraphCnt( bCache ? 0 : USHRT_MAX ) , mnFlyIdx( 0 ) , mbFirst( bCache ) { @@ -541,7 +539,6 @@ SwLayHelper::SwLayHelper( SwDoc *pD, SwFrame* &rpF, SwFrame* &rpP, SwPageFrame* { mnIndex = 0; mnStartOfContent = rNodes.GetEndOfContent().StartOfSectionNode()->GetIndex(); - mnMaxParaPerPage = 1000; } else { @@ -600,28 +597,8 @@ sal_uLong SwLayHelper::CalcPageCount() if ( nTmp > SwNodeOffset(0) ) nNdCount = sal_Int32(nTmp); } - if ( nNdCount > 100 ) // no estimation below this value - { - if ( nPgCount > 0 ) - { // tdf#129529 avoid 0... - mnMaxParaPerPage = std::max<sal_uLong>(3, nNdCount / nPgCount); - } - else - { - mnMaxParaPerPage = std::max( sal_uLong(20), - sal_uLong(20 + nNdCount / 1000 * 3) ); - const sal_uLong nMax = 53; - mnMaxParaPerPage = std::min( mnMaxParaPerPage, nMax ); - nPgCount = nNdCount / mnMaxParaPerPage; - } - if ( nNdCount < 1000 ) - nPgCount = 0;// no progress bar for small documents - SwViewShell *pSh = nullptr; - if( mrpLay && mrpLay->getRootFrame() ) - pSh = mrpLay->getRootFrame()->GetCurrShell(); - if( pSh && pSh->GetViewOptions()->getBrowseMode() ) - mnMaxParaPerPage *= 6; - } + if ( nNdCount < 1000 ) + nPgCount = 0;// no progress bar for small documents } return nPgCount; } @@ -721,13 +698,10 @@ bool SwLayHelper::CheckInsertPage( bool SwLayHelper::CheckInsert( SwNodeOffset nNodeIndex ) { bool bRet = false; - bool bLongTab = false; - sal_uLong nMaxRowPerPage( 0 ); nNodeIndex -= mnStartOfContent; sal_uInt16 nRows( 0 ); if( mrpFrame->IsTabFrame() ) { - //Inside a table counts every row as a paragraph SwFrame *pLow = static_cast<SwTabFrame*>(mrpFrame)->Lower(); nRows = 0; do @@ -735,89 +709,47 @@ bool SwLayHelper::CheckInsert( SwNodeOffset nNodeIndex ) ++nRows; pLow = pLow->GetNext(); } while ( pLow ); - mnParagraphCnt += nRows; - if( !mpImpl && mnParagraphCnt > mnMaxParaPerPage + 10 ) - { - // OD 09.04.2003 #108698# - improve heuristics: - // Assume that a table, which has more than three times the quantity - // of maximal paragraphs per page rows, consists of rows, which have - // the height of a normal paragraph. Thus, allow as much rows per page - // as much paragraphs are allowed. - if ( nRows > ( 3*mnMaxParaPerPage ) ) - { - nMaxRowPerPage = mnMaxParaPerPage; - } - else - { - SwFrame *pTmp = static_cast<SwTabFrame*>(mrpFrame)->Lower(); - if( pTmp->GetNext() ) - pTmp = pTmp->GetNext(); - pTmp = static_cast<SwRowFrame*>(pTmp)->Lower(); - sal_uInt16 nCnt = 0; - do - { - ++nCnt; - pTmp = pTmp->GetNext(); - } while( pTmp ); - nMaxRowPerPage = std::max( sal_uLong(2), mnMaxParaPerPage / nCnt ); - } - bLongTab = true; - } } - else - ++mnParagraphCnt; if( mbFirst && mpImpl && mnIndex < mpImpl->size() && mpImpl->GetBreakIndex( mnIndex ) == nNodeIndex && ( mpImpl->GetBreakOfst( mnIndex ) < COMPLETE_STRING || ( ++mnIndex < mpImpl->size() && mpImpl->GetBreakIndex( mnIndex ) == nNodeIndex ) ) ) mbFirst = false; - // OD 09.04.2003 #108698# - always split a big tables. - if ( !mbFirst || - ( mrpFrame->IsTabFrame() && bLongTab ) - ) + if (!mbFirst) { sal_Int32 nRowCount = 0; do { - if( mpImpl || bLongTab ) + if (mpImpl) { sal_Int32 nOfst = COMPLETE_STRING; sal_uInt16 nType = SW_LAYCACHE_IO_REC_PAGES; - if( bLongTab ) + while( mnIndex < mpImpl->size() && + mpImpl->GetBreakIndex(mnIndex) < nNodeIndex) + ++mnIndex; + if( mnIndex < mpImpl->size() && + mpImpl->GetBreakIndex(mnIndex) == nNodeIndex ) { + nType = mpImpl->GetBreakType( mnIndex ); + nOfst = mpImpl->GetBreakOfst( mnIndex++ ); mbBreakAfter = true; - nOfst = static_cast<sal_Int32>(nRowCount + nMaxRowPerPage); - } - else - { - while( mnIndex < mpImpl->size() && - mpImpl->GetBreakIndex(mnIndex) < nNodeIndex) - ++mnIndex; - if( mnIndex < mpImpl->size() && - mpImpl->GetBreakIndex(mnIndex) == nNodeIndex ) - { - nType = mpImpl->GetBreakType( mnIndex ); - nOfst = mpImpl->GetBreakOfst( mnIndex++ ); - mbBreakAfter = true; - } } if( nOfst < COMPLETE_STRING ) { bool bSplit = false; sal_uInt16 nRepeat( 0 ); - if( !bLongTab && mrpFrame->IsTextFrame() && + if( mrpFrame->IsTextFrame() && SW_LAYCACHE_IO_REC_PARA == nType && nOfst < static_cast<SwTextFrame*>(mrpFrame)->GetText().getLength()) bSplit = true; else if( mrpFrame->IsTabFrame() && nRowCount < nOfst && - ( bLongTab || SW_LAYCACHE_IO_REC_TABLE == nType ) ) + ( SW_LAYCACHE_IO_REC_TABLE == nType ) ) { nRepeat = static_cast<SwTabFrame*>(mrpFrame)-> GetTable()->GetRowsToRepeat(); bSplit = nOfst < nRows && nRowCount + nRepeat < nOfst; - bLongTab = bLongTab && bSplit; } if( bSplit ) { @@ -891,7 +823,7 @@ bool SwLayHelper::CheckInsert( SwNodeOffset nNodeIndex ) } SwPageFrame* pLastPage = mrpPage; - if (CheckInsertPage(mrpPage, mrpLay, mrpFrame, mbBreakAfter, mnMaxParaPerPage < mnParagraphCnt)) + if (CheckInsertPage(mrpPage, mrpLay, mrpFrame, mbBreakAfter, false)) { CheckFlyCache_( pLastPage ); if( mrpPrv && mrpPrv->IsTextFrame() && !mrpPrv->isFrameAreaSizeValid() ) @@ -902,7 +834,6 @@ bool SwLayHelper::CheckInsert( SwNodeOffset nNodeIndex ) bRet = true; mrpPrv = nullptr; - mnParagraphCnt = 0; if ( mrpActualSection ) { @@ -941,8 +872,8 @@ bool SwLayHelper::CheckInsert( SwNodeOffset nNodeIndex ) mrpLay = mrpLay->GetNextLayoutLeaf(); } } - } while( bLongTab || ( mpImpl && mnIndex < mpImpl->size() && - mpImpl->GetBreakIndex( mnIndex ) == nNodeIndex ) ); + } while( mpImpl && mnIndex < mpImpl->size() && + mpImpl->GetBreakIndex( mnIndex ) == nNodeIndex ); } mbFirst = false; return bRet; diff --git a/sw/source/core/layout/layhelp.hxx b/sw/source/core/layout/layhelp.hxx index 1ac325cfac82..a53f3ca3de09 100644 --- a/sw/source/core/layout/layhelp.hxx +++ b/sw/source/core/layout/layhelp.hxx @@ -114,8 +114,6 @@ class SwLayHelper bool mbBreakAfter; SwDoc* mpDoc; SwLayCacheImpl* mpImpl; - sal_uLong mnMaxParaPerPage; - sal_uLong mnParagraphCnt; SwNodeOffset mnStartOfContent; size_t mnIndex; ///< the index in the page break array size_t mnFlyIdx; ///< the index in the fly cache array diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index d4f970f0932c..20505e00baf5 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -1119,6 +1119,10 @@ bool SwTabFrame::Split(const SwTwips nCutPos, bool bTryToSplit, // and compare with the available height SwTwips nRemainingSpaceForLastRow = aRectFnSet.YDiff(nCutPos, aRectFnSet.GetPrtTop(*this)); nRemainingSpaceForLastRow -= aRectFnSet.GetBottomMargin(*this); + + if (nRemainingSpaceForLastRow <= 0) + return false; // upper has no space for this table at all + auto getRemainingAfter = [aRectFnSet, nAvailable = nRemainingSpaceForLastRow, nFirstRowTop = aRectFnSet.GetTop(pRow->getFrameArea())](SwFrame* p) { return nAvailable + (p ? aRectFnSet.BottomDist(p->getFrameArea(), nFirstRowTop) : 0); };
