sw/qa/extras/layout/data/tdf169399.fodt | 47 +++++++++++++++++++++++++ sw/qa/extras/layout/layout4.cxx | 9 ++++ sw/source/core/doc/DocumentLayoutManager.cxx | 11 +++++ sw/source/core/inc/DocumentLayoutManager.hxx | 2 + sw/source/core/inc/layouter.hxx | 2 + sw/source/core/inc/movedfwdfrmsbyobjpos.hxx | 4 ++ sw/source/core/layout/layact.cxx | 17 ++++++--- sw/source/core/layout/layouter.cxx | 21 +++++++++++ sw/source/core/layout/movedfwdfrmsbyobjpos.cxx | 15 +++++++ 9 files changed, 123 insertions(+), 5 deletions(-)
New commits: commit cca30ce7a043cdeab43ad2cf3b6bf92c574346c4 Author: Mike Kaganski <[email protected]> AuthorDate: Thu Nov 27 16:44:53 2025 +0500 Commit: Xisco Fauli <[email protected]> CommitDate: Fri Nov 28 17:10:49 2025 +0100 tdf#169399: try again when frames were moved forward by objects The problem in the bug document was caused by the extra call to SwFrame::OptCalc in SwLayAction::FormatLayout. It was there since initial import, as a shortcut; but its conditions were changed in commits 610c6f02b11b4b4c555a78b0feb2a1eb35159e39 (tdf#156724 tdf#156722 tdf#156745 sw: layout: partially remove IsPaintLocked(), 2023-08-14), 61a78a523a6131ff98b5d846368e5626fe58d99c (tdf#156724 tdf#156722 sw: layout: remove IsPaintLocked() check, 2023-08-15). Commit c303981cfd95ce1c3881366023d5495ae2edce97 (tdf#156724 sw: layout: fix tables not splitting due to footnotes differently, 2023-08-24) had dropped that code completely, but it was restored in commit 397d72e582c725d162c7e0b819dc6c0bb62e42b0 (Related: tdf#158986 sw floattable: fix unexpected page break with sections, 2024-02-23), with more strict conditions. Its conditions were restricted in commit 607fcac441c7f3a7d3c169c19039e581d707f2bb (tdf#160067 sw floattable: fix missing move bwd of paras in split section frame, 2024-04-08). There, it was noted, that "probably a cleaner way would be to completely stop calculating content frames in SwLayAction::FormatLayout() and only do that in FormatContent()", which emphasizes the problematic nature of that code. I looked into restricting the conditions there more for the bug document, but failed to come with the working condition. So the change drops that code again, and tries to keep the bugs fixed by re-introduction of that code fixed. My understanding of the mechanism causing tdf#158986 is following: Document's SwLayouter keeps track of the nodes, which frames were moved forward by objects. This data will disallow those frames from moving backwards (see uses of SwLayouter::FrameMovedFwdByObjPos). The problem in the bug document comes from the fly attached to a paragraph inside a multicolumn section. During the iterative layout of the section, there is a stage where the height of the section is too small for the object, and its paragraph gets moved forward to a new page (together with some previous paragraphs). When layout finishes, the height of the section is enough, but the paragraph cannot move back, keeping the extra page. I tried hard to solve it in many less-bruteforce ways. The ideas included prevention of the wrong too small section height during layout (e.g., in lcl_FormatContentOfLayoutFrame); attempts to find a place where a specific "moved forward" record becomes obsolete and should be removed (tried to modify fly.cxx' CalcContent and SwObjectFormatterTextFrame::DoFormatObj); and modifications to let page body's resize code (called when a section's height is changed) to re-position the objects. But all these attempts produced layout loops. This change introduces a hack. In SwLayAction::Action, when the first call to InternalAction increases the number of frames that were moved forward, it clears the layouter entries, and tries once more. I hope that at some point, a better fix will be possible. Change-Id: I47315473982cb1fe5c8d77f7603cd24de08afa89 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194712 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194759 diff --git a/sw/qa/extras/layout/data/tdf169399.fodt b/sw/qa/extras/layout/data/tdf169399.fodt new file mode 100644 index 000000000000..29a375ca84a8 --- /dev/null +++ b/sw/qa/extras/layout/data/tdf169399.fodt @@ -0,0 +1,47 @@ +<?xml version="1.0" encoding="UTF-8"?> + +<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0" xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0" office:version="1.4" office:mimetype="application/vnd.oasis.opendocument.text"> + <office:styles> + <style:style style:name="Frame" style:family="graphic"/> + </office:styles> + <office:automatic-styles> + <style:style style:name="P1" style:family="paragraph"> + <style:paragraph-properties fo:line-height="115%"/> + </style:style> + <style:style style:name="fr1" style:family="graphic" style:parent-style-name="Frame"> + <style:graphic-properties style:wrap="none" style:vertical-pos="from-top" style:vertical-rel="paragraph" style:horizontal-pos="from-left" style:horizontal-rel="paragraph-content" fo:border="0.06pt solid #000000"/> + </style:style> + <style:page-layout style:name="pm1"> + <style:page-layout-properties fo:page-width="210mm" fo:page-height="297mm" style:print-orientation="portrait" fo:margin-top="15mm" fo:margin-bottom="1cm" fo:margin-left="2cm" fo:margin-right="2cm"/> + </style:page-layout> + </office:automatic-styles> + <office:master-styles> + <style:master-page style:name="Standard" style:page-layout-name="pm1"/> + </office:master-styles> + <office:body> + <office:text> + <text:p>p1</text:p> + <text:p><draw:frame draw:style-name="fr1" draw:name="Frame1" text:anchor-type="paragraph" svg:x="0" svg:y="0" svg:width="5cm" svg:height="78mm"> + <draw:text-box/> + </draw:frame>p2</text:p> + <text:section text:name="Section1"> + <text:p>p3</text:p> + </text:section> + <text:p>p4</text:p> + <text:section text:name="Section2"> + <text:p>p5</text:p> + </text:section> + <text:section text:name="Section3"> + <text:p>p6</text:p> + </text:section> + <text:p>p7</text:p> + <text:section text:name="Section4"> + <text:p text:style-name="P1">p8<text:line-break/><text:line-break/><text:line-break/><text:line-break/><text:line-break/><text:line-break/></text:p> + <text:section text:name="Section5"> + <text:p>p9 must be on page 1</text:p> + <text:p>p10</text:p> + </text:section> + </text:section> + </office:text> + </office:body> +</office:document> \ No newline at end of file diff --git a/sw/qa/extras/layout/layout4.cxx b/sw/qa/extras/layout/layout4.cxx index 669d2a484100..97958ebe25ad 100644 --- a/sw/qa/extras/layout/layout4.cxx +++ b/sw/qa/extras/layout/layout4.cxx @@ -1943,6 +1943,15 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter4, TestTdf163285) CPPUNIT_ASSERT(topText3.startsWith("pg_3")); } +CPPUNIT_TEST_FIXTURE(SwLayoutWriter4, testTdf169399) +{ + createSwDoc("tdf169399.fodt"); + + xmlDocUniquePtr pXmlDoc = parseLayoutDump(); + // Without the fix, this failed, because there were two pages: + assertXPath(pXmlDoc, "/root/page", 1); +} + CPPUNIT_TEST_FIXTURE(SwLayoutWriter4, TestTdf152839_firstRows) { createSwDoc("tdf152839_firstrows.rtf"); diff --git a/sw/source/core/doc/DocumentLayoutManager.cxx b/sw/source/core/doc/DocumentLayoutManager.cxx index 0c89f480bece..e15df6de772b 100644 --- a/sw/source/core/doc/DocumentLayoutManager.cxx +++ b/sw/source/core/doc/DocumentLayoutManager.cxx @@ -492,6 +492,17 @@ void DocumentLayoutManager::ClearSwLayouterEntries() SwLayouter::ClearMoveBwdLayoutInfo( m_rDoc ); } +size_t DocumentLayoutManager::GetMovedFwdFramesCount() const +{ + return SwLayouter::GetMovedFwdFramesCount(m_rDoc); +} + +void DocumentLayoutManager::ClearSwLayouterEntriesWithInvalidation() +{ + SwLayouter::InvalidateMovedFwdFrames(m_rDoc); + ClearSwLayouterEntries(); +} + DocumentLayoutManager::~DocumentLayoutManager() { } diff --git a/sw/source/core/inc/DocumentLayoutManager.hxx b/sw/source/core/inc/DocumentLayoutManager.hxx index 8db5cbe328fe..42315aeb6d55 100644 --- a/sw/source/core/inc/DocumentLayoutManager.hxx +++ b/sw/source/core/inc/DocumentLayoutManager.hxx @@ -54,6 +54,8 @@ public: //Non Interface methods void ClearSwLayouterEntries(); + size_t GetMovedFwdFramesCount() const; + void ClearSwLayouterEntriesWithInvalidation(); virtual ~DocumentLayoutManager() override; diff --git a/sw/source/core/inc/layouter.hxx b/sw/source/core/inc/layouter.hxx index 2a5aac558387..bebe9d56ce93 100644 --- a/sw/source/core/inc/layouter.hxx +++ b/sw/source/core/inc/layouter.hxx @@ -116,6 +116,8 @@ public: static bool FrameMovedFwdByObjPos( const SwDoc& _rDoc, const SwTextFrame& _rTextFrame, sal_uInt32& _ornToPageNum ); + static size_t GetMovedFwdFramesCount(const SwDoc& _rDoc); + static void InvalidateMovedFwdFrames(const SwDoc& _rDoc); // --> #i40155# - unmark given frame as to be moved forward. static void RemoveMovedFwdFrame( const SwDoc& _rDoc, const SwTextFrame& _rTextFrame ); diff --git a/sw/source/core/inc/movedfwdfrmsbyobjpos.hxx b/sw/source/core/inc/movedfwdfrmsbyobjpos.hxx index c42944060ced..c1f6a639201f 100644 --- a/sw/source/core/inc/movedfwdfrmsbyobjpos.hxx +++ b/sw/source/core/inc/movedfwdfrmsbyobjpos.hxx @@ -51,6 +51,10 @@ class SwMovedFwdFramesByObjPos bool DoesRowContainMovedFwdFrame( const SwRowFrame& _rRowFrame ) const; void Clear() { maMovedFwdFrames.clear(); }; + + size_t Count() const { return maMovedFwdFrames.size(); } + + void InvalidateAll(); }; #endif diff --git a/sw/source/core/layout/layact.cxx b/sw/source/core/layout/layact.cxx index ad59d390d86d..5439efae5bcf 100644 --- a/sw/source/core/layout/layact.cxx +++ b/sw/source/core/layout/layact.cxx @@ -37,6 +37,7 @@ #include <IDocumentDrawModelAccess.hxx> #include <IDocumentStatistics.hxx> #include <IDocumentLayoutAccess.hxx> +#include <DocumentLayoutManager.hxx> #include <sfx2/event.hxx> @@ -395,9 +396,20 @@ void SwLayAction::Action(OutputDevice* pRenderContext) if ( IsCalcLayout() ) SetCheckPages( false ); + // tdf#169399: workaround for frames unable to move backward after moved forward by objects + // in incomplete layout + auto& rLayoutManager = m_pRoot->GetFormat()->GetDoc().GetDocumentLayoutManager(); + const auto nOldMovedCount = rLayoutManager.GetMovedFwdFramesCount(); + InternalAction(pRenderContext); if (RemoveEmptyBrowserPages()) SetAgain(true); + if (nOldMovedCount < rLayoutManager.GetMovedFwdFramesCount()) + { + // Only do it once + rLayoutManager.ClearSwLayouterEntriesWithInvalidation(); + SetAgain(true); + } while ( IsAgain() ) { SetAgain(false); @@ -1500,11 +1512,6 @@ bool SwLayAction::FormatLayout( OutputDevice *pRenderContext, SwLayoutFrame *pLa PopFormatLayout(); } } - else if (pLay->IsSctFrame() && pLay->GetNext() && pLay->GetNext()->IsSctFrame() && pLow->IsTextFrame() && pLow == pLay->GetLastLower()) - { - // else: only calc the last text lower of sections, followed by sections - pLow->OptCalc(); - } if ( IsAgain() ) return false; diff --git a/sw/source/core/layout/layouter.cxx b/sw/source/core/layout/layouter.cxx index 45bf3608c0c2..f4fb8426047b 100644 --- a/sw/source/core/layout/layouter.cxx +++ b/sw/source/core/layout/layouter.cxx @@ -370,6 +370,27 @@ bool SwLayouter::FrameMovedFwdByObjPos( const SwDoc& _rDoc, } } +// static +size_t SwLayouter::GetMovedFwdFramesCount(const SwDoc& _rDoc) +{ + if (_rDoc.getIDocumentLayoutAccess().GetLayouter() + && _rDoc.getIDocumentLayoutAccess().GetLayouter()->mpMovedFwdFrames) + { + return _rDoc.getIDocumentLayoutAccess().GetLayouter()->mpMovedFwdFrames->Count(); + } + return 0; +} + +// static +void SwLayouter::InvalidateMovedFwdFrames(const SwDoc& _rDoc) +{ + if (_rDoc.getIDocumentLayoutAccess().GetLayouter() + && _rDoc.getIDocumentLayoutAccess().GetLayouter()->mpMovedFwdFrames) + { + _rDoc.getIDocumentLayoutAccess().GetLayouter()->mpMovedFwdFrames->InvalidateAll(); + } +} + // #i26945# bool SwLayouter::DoesRowContainMovedFwdFrame( const SwDoc& _rDoc, const SwRowFrame& _rRowFrame ) diff --git a/sw/source/core/layout/movedfwdfrmsbyobjpos.cxx b/sw/source/core/layout/movedfwdfrmsbyobjpos.cxx index 7aba4b74a37b..a99e35110324 100644 --- a/sw/source/core/layout/movedfwdfrmsbyobjpos.cxx +++ b/sw/source/core/layout/movedfwdfrmsbyobjpos.cxx @@ -87,4 +87,19 @@ bool SwMovedFwdFramesByObjPos::DoesRowContainMovedFwdFrame( const SwRowFrame& _r return bDoesRowContainMovedFwdFrame; } +void SwMovedFwdFramesByObjPos::InvalidateAll() +{ + for (const auto& rEntry : maMovedFwdFrames) + { + SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aFrameIter( + *rEntry.first); + for (SwTextFrame* pTextFrame = aFrameIter.First(); pTextFrame; + pTextFrame = aFrameIter.Next()) + { + pTextFrame->InvalidatePos(); + pTextFrame->InvalidatePage(); + } + } +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
