summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Stahl <michael.stahl@allotropia.de>2022-10-14 16:59:42 +0200
committerXisco Fauli <xiscofauli@libreoffice.org>2023-02-15 11:40:56 +0000
commitd31b30a6bb044bedd4e296c4cbb3e0fc9c2cb7b6 (patch)
treecc54c295fc52022059560705443718f189f98069
parentdisable script dump (diff)
downloadcore-d31b30a6bb044bedd4e296c4cbb3e0fc9c2cb7b6.tar.gz
core-d31b30a6bb044bedd4e296c4cbb3e0fc9c2cb7b6.zip
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>
-rw-r--r--sw/source/core/inc/cntfrm.hxx2
-rw-r--r--sw/source/core/inc/txtfrm.hxx2
-rw-r--r--sw/source/core/layout/calcmove.cxx8
-rw-r--r--sw/source/core/layout/unusedf.cxx2
-rw-r--r--sw/source/core/text/itrform2.cxx47
-rw-r--r--sw/source/core/text/itrform2.hxx2
-rw-r--r--sw/source/core/text/txtfrm.cxx43
-rw-r--r--sw/source/core/text/widorp.cxx29
-rw-r--r--sw/source/core/text/widorp.hxx2
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 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 )