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>
(cherry picked from commit 8c32cc17ce914188ea6783b0f79e19c5ddbf0b8d)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146698
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
diff --git a/sw/source/core/inc/cntfrm.hxx b/sw/source/core/inc/cntfrm.hxx
index cf47396..b972a7a 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 1ba2c83..0ff2321 100644
--- a/sw/source/core/inc/txtfrm.hxx
+++ b/sw/source/core/inc/txtfrm.hxx
@@ -483,7 +483,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 c9d5622..1843735 100644
--- a/sw/source/core/layout/calcmove.cxx
+++ b/sw/source/core/layout/calcmove.cxx
@@ -1761,7 +1761,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);
@@ -2046,7 +2046,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 );
@@ -2054,7 +2054,7 @@ bool SwContentFrame::WouldFit_( SwTwips nSpace,
}
else
{
bRet = pFrame->WouldFit( nSpace, bSplit, false );
bRet = pFrame->WouldFit(nSpace, bSplit, false, true);
nSecondCheck = !bSplit ? 1 : 0;
}
@@ -2123,7 +2123,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 e8322c8..25de7617 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 563ddd5..2401b40 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>
@@ -401,7 +403,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()));
@@ -723,7 +756,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() )
@@ -1356,8 +1389,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 d5f0fa5..2d71fad 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 758eb0b..9c4ac5b 100644
--- a/sw/source/core/text/txtfrm.cxx
+++ b/sw/source/core/text/txtfrm.cxx
@@ -3237,7 +3237,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);
}
/**
@@ -3252,7 +3252,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" );
@@ -3335,7 +3335,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
@@ -3766,7 +3766,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 f6b9ba2..ceae9ee 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 9d526f9..193d27f 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 )
