diff options
author | Michael Stahl <michael.stahl@allotropia.de> | 2022-10-14 16:59:42 +0200 |
---|---|---|
committer | Michael Stahl <michael.stahl@allotropia.de> | 2022-10-14 21:38:51 +0200 |
commit | 8c32cc17ce914188ea6783b0f79e19c5ddbf0b8d (patch) | |
tree | c313ab238961b4fd867c3abf9234e6d71e548207 /sw | |
parent | 720a093461aadff044ac0b1b7b45cf3d416b3785 (diff) |
tdf#146500 sw: try to fix some layout loops caused by fly in table cell
Reportedly regression from commit 1cb7e4899b5ada902e99a0c964ee047950c07044
but reverting that on master doesn't seem to help.
Inserting paragraphs with single letters into the first cell on page 2,
once we get to "s" and press Enter, the infinite loop happens as text
frame 56 is split at index 0 (where a fly is anchored), the new follow
moves to page 3, and very quickly frame 56 is formatted again and all
text in the follow moves back to page 2, where it doesn't fit with its
anchored fly.
The problem here is that the (Word format) file sets the
ConsiderObjWrapInfluenceOnObjPos mode, which disables any use of the
SwLayouter::FrameMovedFwdByObjPos() etc. functions.
Oddly enough the spec obj-pos-without-wrapping.sxw claims that in this
mode, "If a paragraph respectively one of the anchor characters moves to
the next page during its format in step (2) or (4), the process is
re-started with this paragraph, the complete paragraph is moved to the
next page and it is assured that the paragraph doesn't move back again."
What happens instead is that when a fly position is changed, its
mbRestartLayoutProcess flag is set.
So let's try to check this flag during SwTextFrame::Format(): stop at a
position where a fly is anchored in a follow frame that has the flag
set, via rInf.SetStop(true).
This is similar to a check in SwFlowFrame::MoveBwd() that prevents a
frame from moving back if it has an anchored fly with this flag set.
Now we get a different loop, where the text frame moves forward, then
back and then splits again.
This is because ShouldBwdMoved() sees that the first line of the text
frame fits into the cell of the previous page - but the first line
consists only of a SwFlyPortion and a SwMarginPortion, for the fly
anchored in the same text frame.
So change WidowsAndOrphans::WouldFit() to continue until the first line
that doesn't consist entirely of flys.
Then there's another loop where the follow flow row is joined and then
splits again - this is very similar to the previous problem, change
SwTextFrame::FirstLineHeight() (which is only called from table code) to
continue until a non-fly line as well.
Unforunately this is all not so clear because we can't actually tell
which text frame contains the anchor of a SwFlyPortion, so let's see
what's going to break with these changes.
Change-Id: Ia6e80f86f6ca3d2e3d91651b6a403c050d74ebfa
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141383
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
Diffstat (limited to 'sw')
-rw-r--r-- | sw/source/core/inc/cntfrm.hxx | 2 | ||||
-rw-r--r-- | sw/source/core/inc/txtfrm.hxx | 2 | ||||
-rw-r--r-- | sw/source/core/layout/calcmove.cxx | 8 | ||||
-rw-r--r-- | sw/source/core/layout/unusedf.cxx | 2 | ||||
-rw-r--r-- | sw/source/core/text/itrform2.cxx | 47 | ||||
-rw-r--r-- | sw/source/core/text/itrform2.hxx | 2 | ||||
-rw-r--r-- | sw/source/core/text/txtfrm.cxx | 43 | ||||
-rw-r--r-- | sw/source/core/text/widorp.cxx | 29 | ||||
-rw-r--r-- | sw/source/core/text/widorp.hxx | 2 |
9 files changed, 118 insertions, 19 deletions
diff --git a/sw/source/core/inc/cntfrm.hxx b/sw/source/core/inc/cntfrm.hxx index cf47396eb4d9..b972a7aaafd8 100644 --- a/sw/source/core/inc/cntfrm.hxx +++ b/sw/source/core/inc/cntfrm.hxx @@ -106,7 +106,7 @@ public: // nMaxHeight is the required height // bSplit indicates that the paragraph has to be split // bTst indicates that we are currently doing a test formatting - virtual bool WouldFit( SwTwips &nMaxHeight, bool &bSplit, bool bTst ); + virtual bool WouldFit(SwTwips &nMaxHeight, bool &bSplit, bool bTst, bool); bool MoveFootnoteCntFwd( bool, SwFootnoteBossFrame* ); // called by MoveFwd if content diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx index fd9c38336dcf..f24d889df35f 100644 --- a/sw/source/core/inc/txtfrm.hxx +++ b/sw/source/core/inc/txtfrm.hxx @@ -485,7 +485,7 @@ public: * bSplit indicates, that the paragraph has to be split * bTst indicates, that we are currently doing a test formatting */ - virtual bool WouldFit( SwTwips &nMaxHeight, bool &bSplit, bool bTst ) override; + virtual bool WouldFit(SwTwips &nMaxHeight, bool &bSplit, bool bTst, bool bMoveBwd) override; /** * The WouldFit equivalent for temporarily rewired TextFrames diff --git a/sw/source/core/layout/calcmove.cxx b/sw/source/core/layout/calcmove.cxx index 4894c8524cf3..acb384f2b759 100644 --- a/sw/source/core/layout/calcmove.cxx +++ b/sw/source/core/layout/calcmove.cxx @@ -1764,7 +1764,7 @@ void SwContentFrame::MakeAll(vcl::RenderContext* /*pRenderContext*/) SwTwips nTmp = aRectFnSet.GetHeight(GetUpper()->getFramePrintArea()) - aRectFnSet.GetTop(getFramePrintArea()); bool bSplit = !IsFwdMoveAllowed(); - if ( nTmp > 0 && WouldFit( nTmp, bSplit, false ) ) + if (nTmp > 0 && WouldFit(nTmp, bSplit, false, false)) { Prepare( PrepareHint::WidowsOrphans, nullptr, false ); setFrameAreaSizeValid(false); @@ -2049,7 +2049,7 @@ bool SwContentFrame::WouldFit_( SwTwips nSpace, bRet = static_cast<SwTextFrame*>(pFrame)->TestFormat( pTmpPrev, nSpace, bSplit ); } else - bRet = pFrame->WouldFit( nSpace, bSplit, false ); + bRet = pFrame->WouldFit(nSpace, bSplit, false, true); pTmpFrame->RemoveFromLayout(); pTmpFrame->InsertBefore( pUp, pOldNext ); @@ -2057,7 +2057,7 @@ bool SwContentFrame::WouldFit_( SwTwips nSpace, } else { - bRet = pFrame->WouldFit( nSpace, bSplit, false ); + bRet = pFrame->WouldFit(nSpace, bSplit, false, true); nSecondCheck = !bSplit ? 1 : 0; } @@ -2126,7 +2126,7 @@ bool SwContentFrame::WouldFit_( SwTwips nSpace, // We do a second check with the original remaining space // reduced by the required upper space: nOldSpace -= nSecondCheck; - const bool bSecondRet = nOldSpace >= 0 && pFrame->WouldFit( nOldSpace, bOldSplit, false ); + const bool bSecondRet = nOldSpace >= 0 && pFrame->WouldFit(nOldSpace, bOldSplit, false, true); if ( bSecondRet && bOldSplit && nOldSpace >= 0 ) { bRet = true; diff --git a/sw/source/core/layout/unusedf.cxx b/sw/source/core/layout/unusedf.cxx index e8322c896b12..25de7617c11d 100644 --- a/sw/source/core/layout/unusedf.cxx +++ b/sw/source/core/layout/unusedf.cxx @@ -32,7 +32,7 @@ void SwFrame::PaintSwFrame(vcl::RenderContext&, SwRect const&, SwPrintData const OSL_FAIL( "PaintSwFrame() of the base class called." ); } -bool SwContentFrame::WouldFit( SwTwips &, bool&, bool ) +bool SwContentFrame::WouldFit(SwTwips &, bool&, bool, bool) { OSL_FAIL( "WouldFit of ContentFrame called." ); return false; diff --git a/sw/source/core/text/itrform2.cxx b/sw/source/core/text/itrform2.cxx index 5d1d160d27bb..f925daaa8b97 100644 --- a/sw/source/core/text/itrform2.cxx +++ b/sw/source/core/text/itrform2.cxx @@ -45,6 +45,8 @@ #include "porhyph.hxx" #include "pordrop.hxx" #include "redlnitr.hxx" +#include <sortedobjs.hxx> +#include <fmtanchr.hxx> #include <pagefrm.hxx> #include <tgrditem.hxx> #include <doc.hxx> @@ -404,7 +406,38 @@ void SwTextFormatter::BuildPortions( SwTextFormatInfo &rInf ) rInf.SetFull(true); } - SwLinePortion *pPor = NewPortion( rInf ); + ::std::optional<TextFrameIndex> oMovedFlyIndex; + if (SwTextFrame const*const pFollow = GetTextFrame()->GetFollow()) + { + // flys are always on master! + if (GetTextFrame()->GetDrawObjs() && pFollow->GetUpper() != GetTextFrame()->GetUpper()) + { + for (SwAnchoredObject const*const pAnchoredObj : *GetTextFrame()->GetDrawObjs()) + { + // tdf#146500 try to stop where a fly is anchored in the follow + // that has recently been moved (presumably by splitting this + // frame); similar to check in SwFlowFrame::MoveBwd() + if (pAnchoredObj->RestartLayoutProcess() + && !pAnchoredObj->IsTmpConsiderWrapInfluence()) + { + SwFormatAnchor const& rAnchor(pAnchoredObj->GetFrameFormat().GetAnchor()); + assert(rAnchor.GetAnchorId() == RndStdIds::FLY_AT_CHAR || rAnchor.GetAnchorId() == RndStdIds::FLY_AT_PARA); + TextFrameIndex const nAnchor(GetTextFrame()->MapModelToViewPos(*rAnchor.GetContentAnchor())); + if (pFollow->GetOffset() <= nAnchor + && (pFollow->GetFollow() == nullptr + || nAnchor < pFollow->GetFollow()->GetOffset())) + { + if (!oMovedFlyIndex || nAnchor < *oMovedFlyIndex) + { + oMovedFlyIndex.emplace(nAnchor); + } + } + } + } + } + } + + SwLinePortion *pPor = NewPortion(rInf, oMovedFlyIndex); // Asian grid stuff SwTextGridItem const*const pGrid(GetGridItem(m_pFrame->FindPageFrame())); @@ -726,7 +759,7 @@ void SwTextFormatter::BuildPortions( SwTextFormatInfo &rInf ) { (void) rInf.CheckCurrentPosBookmark(); // bookmark was already created inside MultiPortion! } - pPor = NewPortion( rInf ); + pPor = NewPortion(rInf, oMovedFlyIndex); } if( !rInf.IsStop() ) @@ -1503,8 +1536,16 @@ static bool lcl_OldFieldRest( const SwLineLayout* pCurr ) * -> CalcFlyWidth emulates the width and return portion, if needed */ -SwLinePortion *SwTextFormatter::NewPortion( SwTextFormatInfo &rInf ) +SwLinePortion *SwTextFormatter::NewPortion(SwTextFormatInfo &rInf, + ::std::optional<TextFrameIndex> const oMovedFlyIndex) { + if (oMovedFlyIndex && *oMovedFlyIndex <= rInf.GetIdx()) + { + SAL_WARN_IF(*oMovedFlyIndex != rInf.GetIdx(), "sw.core", "stopping too late, no portion break at fly anchor?"); + rInf.SetStop(true); + return nullptr; + } + // Underflow takes precedence rInf.SetStopUnderflow( false ); if( rInf.GetUnderflow() ) diff --git a/sw/source/core/text/itrform2.hxx b/sw/source/core/text/itrform2.hxx index d5f0fa550b51..2d71fad34cd5 100644 --- a/sw/source/core/text/itrform2.hxx +++ b/sw/source/core/text/itrform2.hxx @@ -45,7 +45,7 @@ class SwTextFormatter : public SwTextPainter std::unique_ptr<sw::MergedAttrIterByEnd> m_pByEndIter; // HACK for TryNewNoLengthPortion SwLinePortion* m_pFirstOfBorderMerge; // The first text portion of a joined border (during portion building) - SwLinePortion *NewPortion( SwTextFormatInfo &rInf ); + SwLinePortion *NewPortion(SwTextFormatInfo &rInf, ::std::optional<TextFrameIndex>); SwTextPortion *NewTextPortion( SwTextFormatInfo &rInf ); SwLinePortion *NewExtraPortion( SwTextFormatInfo &rInf ); SwTabPortion *NewTabPortion( SwTextFormatInfo &rInf, bool bAuto ) const; diff --git a/sw/source/core/text/txtfrm.cxx b/sw/source/core/text/txtfrm.cxx index 166757f53d36..8c8fc987f57c 100644 --- a/sw/source/core/text/txtfrm.cxx +++ b/sw/source/core/text/txtfrm.cxx @@ -3241,7 +3241,7 @@ bool SwTextFrame::TestFormat( const SwFrame* pPrv, SwTwips &rMaxHeight, bool &bS SwTestFormat aSave( this, pPrv, rMaxHeight ); - return SwTextFrame::WouldFit( rMaxHeight, bSplit, true ); + return SwTextFrame::WouldFit(rMaxHeight, bSplit, true, false); } /** @@ -3256,7 +3256,7 @@ bool SwTextFrame::TestFormat( const SwFrame* pPrv, SwTwips &rMaxHeight, bool &bS * * @returns true if I can split */ -bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst ) +bool SwTextFrame::WouldFit(SwTwips &rMaxHeight, bool &bSplit, bool bTst, bool bMoveBwd) { OSL_ENSURE( ! IsVertical() || ! IsSwapped(), "SwTextFrame::WouldFit with swapped frame" ); @@ -3339,7 +3339,7 @@ bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst ) // is breaking necessary? bSplit = !aFrameBreak.IsInside( aLine ); if ( bSplit ) - bRet = !aFrameBreak.IsKeepAlways() && aFrameBreak.WouldFit( aLine, rMaxHeight, bTst ); + bRet = !aFrameBreak.IsKeepAlways() && aFrameBreak.WouldFit(aLine, rMaxHeight, bTst, bMoveBwd); else { // we need the total height including the current line @@ -3770,7 +3770,42 @@ sal_uInt16 SwTextFrame::FirstLineHeight() const if ( !pPara ) return USHRT_MAX; - return pPara->Height(); + // tdf#146500 Lines with only fly overlap cannot be "moved", so the idea + // here is to continue until there's some text. + // FIXME ideally we want to count a fly to the line in which it is anchored + // - it may even be anchored in some other paragraph! SwFlyPortion doesn't + // have a pointer sadly so no way to find out. + sal_uInt16 nHeight(0); + for (SwLineLayout const* pLine = pPara; pLine; pLine = pLine->GetNext()) + { + nHeight += pLine->Height(); + bool hasNonFly(false); + for (SwLinePortion const* pPortion = pLine->GetFirstPortion(); + pPortion; pPortion = pPortion->GetNextPortion()) + { + switch (pPortion->GetWhichPor()) + { + case PortionType::Fly: + case PortionType::Glue: + case PortionType::Margin: + break; + default: + { + hasNonFly = true; + break; + } + } + if (hasNonFly) + { + break; + } + } + if (hasNonFly) + { + break; + } + } + return nHeight; } sal_uInt16 SwTextFrame::GetLineCount(TextFrameIndex const nPos) diff --git a/sw/source/core/text/widorp.cxx b/sw/source/core/text/widorp.cxx index f6b9ba20184f..ceae9ee8d70c 100644 --- a/sw/source/core/text/widorp.cxx +++ b/sw/source/core/text/widorp.cxx @@ -525,7 +525,7 @@ bool WidowsAndOrphans::FindWidows( SwTextFrame *pFrame, SwTextMargin &rLine ) return true; } -bool WidowsAndOrphans::WouldFit( SwTextMargin &rLine, SwTwips &rMaxHeight, bool bTst ) +bool WidowsAndOrphans::WouldFit( SwTextMargin &rLine, SwTwips &rMaxHeight, bool bTst, bool bMoveBwd ) { // Here it does not matter, if pFrame is swapped or not. // IsInside() takes care of itself @@ -542,11 +542,34 @@ bool WidowsAndOrphans::WouldFit( SwTextMargin &rLine, SwTwips &rMaxHeight, bool rLine.Top(); SwTwips nLineSum = rLine.GetLineHeight(); - while( nMinLines > rLine.GetLineNr() ) + // tdf#146500 for MoveBwd(), want at least 1 line with non-fly + bool hasNonFly(!bMoveBwd); + while (nMinLines > rLine.GetLineNr() || !hasNonFly) { if( !rLine.NextLine() ) - return false; + { + if (nMinLines > rLine.GetLineNr()) + return false; + else + break; + } nLineSum += rLine.GetLineHeight(); + for (SwLinePortion const* pPortion = rLine.GetCurr()->GetFirstPortion(); + !hasNonFly && pPortion; pPortion = pPortion->GetNextPortion()) + { + switch (pPortion->GetWhichPor()) + { + case PortionType::Fly: + case PortionType::Glue: + case PortionType::Margin: + break; + default: + { + hasNonFly = true; + break; + } + } + } } // We do not fit diff --git a/sw/source/core/text/widorp.hxx b/sw/source/core/text/widorp.hxx index 9d526f9a805d..193d27f633fe 100644 --- a/sw/source/core/text/widorp.hxx +++ b/sw/source/core/text/widorp.hxx @@ -63,7 +63,7 @@ public: void ClrOrphLines(){ m_nOrphLines = 0; } bool FindBreak( SwTextFrame *pFrame, SwTextMargin &rLine, bool bHasToFit ); - bool WouldFit( SwTextMargin &rLine, SwTwips &rMaxHeight, bool bTest ); + bool WouldFit( SwTextMargin &rLine, SwTwips &rMaxHeight, bool bTest, bool bMoveBwd ); // i#16128 - This method is named this way to avoid confusion with // base class method <SwTextFrameBreak::IsBreakNow>, which isn't virtual. bool IsBreakNowWidAndOrp( SwTextMargin &rLine ) |