sw/source/core/inc/cntfrm.hxx | 2 -
sw/source/core/inc/txtfrm.hxx | 2 -
sw/source/core/layout/calcmove.cxx | 8 +++---
sw/source/core/layout/unusedf.cxx | 2 -
sw/source/core/text/itrform2.cxx | 47 ++++++++++++++++++++++++++++++++++---
sw/source/core/text/itrform2.hxx | 2 -
sw/source/core/text/txtfrm.cxx | 43 ++++++++++++++++++++++++++++++---
sw/source/core/text/widorp.cxx | 29 ++++++++++++++++++++--
sw/source/core/text/widorp.hxx | 2 -
9 files changed, 118 insertions(+), 19 deletions(-)
New commits:
commit d31b30a6bb044bedd4e296c4cbb3e0fc9c2cb7b6
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Oct 14 16:59:42 2022 +0200
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Wed Feb 15 11:40:56 2023 +0000
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.st...@allotropia.de>
(cherry picked from commit 8c32cc17ce914188ea6783b0f79e19c5ddbf0b8d)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146698
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
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 1ba2c832081d..0ff2321bc375 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 c9d5622ba02d..184373585928 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 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 563ddd5c4c02..2401b40c337c 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 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 758eb0bd99e1..9c4ac5b0e546 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 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 )
