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 b6bcc7ac278e0db22df02707789730ec123bf934
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Fri Jun 6 02:05:21 2025 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Sat Jun 7 11:47:47 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 <mike.kagan...@collabora.com>
Tested-by: Jenkins
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 e0ca92bd4524..075c1a58417d 100644
--- a/sw/qa/extras/layout/layout5.cxx
+++ b/sw/qa/extras/layout/layout5.cxx
@@ -1751,6 +1751,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 19bca2e1b238..fedcbf31e53a 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 a3f94a56daf1..4c1df8a6b286 100644
--- a/sw/source/core/layout/laycache.cxx
+++ b/sw/source/core/layout/laycache.cxx
@@ -528,8 +528,6 @@ SwLayHelper::SwLayHelper( SwDoc& rDoc, SwFrame* &rpF,
SwFrame* &rpP, SwPageFrame
, mrpActualSection( rpA )
, mbBreakAfter(false)
, mrDoc(rDoc)
- , mnMaxParaPerPage( 25 )
- , mnParagraphCnt( bCache ? 0 : USHRT_MAX )
, mnFlyIdx( 0 )
, mbFirst( bCache )
{
@@ -541,7 +539,6 @@ SwLayHelper::SwLayHelper( SwDoc& rDoc, 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 )
{
@@ -942,8 +873,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 356caa4c14d1..91f8e259b93e 100644
--- a/sw/source/core/layout/layhelp.hxx
+++ b/sw/source/core/layout/layhelp.hxx
@@ -114,8 +114,6 @@ class SwLayHelper
bool mbBreakAfter;
SwDoc& mrDoc;
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 f98444f7d410..c3fffa454f47 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -1124,6 +1124,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); };
