[Libreoffice-commits] core.git: sw/source
Michael Stahl (via logerrit)
logerrit at kemper.freedesktop.org
Thu Apr 22 16:44:10 UTC 2021
sw/source/core/layout/flycnt.cxx | 18 ++-
sw/source/core/layout/objectformattertxtfrm.cxx | 142 ++++++++++++++++++++++--
sw/source/core/layout/objectformattertxtfrm.hxx | 9 +
3 files changed, 152 insertions(+), 17 deletions(-)
New commits:
commit c799de145f7e289f31e3669646e5bd12814e6c5e
Author: Michael Stahl <michael.stahl at allotropia.de>
AuthorDate: Thu Apr 22 13:43:07 2021 +0200
Commit: Michael Stahl <michael.stahl at allotropia.de>
CommitDate: Thu Apr 22 18:43:30 2021 +0200
tdf#138518 sw: layout: avoid moving flys forward prematurely
(regression from b9ef71476fd70bc13f50ebe80390e0730d1b7afb
"tdf#134298 sw: layout: remove left-over page frame without content")
When updating the 3rd ToX, the change to remove empty page frames
causes one page frame to be deleted.
On the subsequent layout, things generally move backward, but there are
some fly-related hiccups; the first problem is visible on page 7.
Commit eb85de8e6b61fb3fcb6c03ae0145f7fe5478bccf
"sw: layout: if fly's anchor moves forward, move fly off SwPageFrame"
helps quite a bit, but not completely; now the first problem happens on
page 54, when SwTextFrame 1261 and its fly 3132 are formatted.
Frame 1261 moves forward to page 55, and then
SwObjectFormatterTextFrame::CheckMovedFwdCondition() returns true and so
SwMovedFwdFramesByObjPos::Insert() is called to prevent frame 1261 from
moving back to page 54.
But the reason why it moved forward is that there are 3 flys on page 54
that are anchored on frames in the next-chain of 1261, namely 1275,
1283 and 1284; if these flys weren't on the page then 1261 would fit.
While the move forward cannot be easily prevented in the situation, it's
possible to avoid the entry into the SwMovedFwdFramesByObjPos map,
by detecting that there are flys on the page that would should forward
*before* the current one does.
With this fix and the above mentioned commit to get the flys off the
page, frame 1261 will eventually move back to page 54 again.
Change-Id: I83e44d65a0b889a49a313b0cd8b08efce4c3afa7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114478
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>
diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx
index 728112f2f684..474e51697785 100644
--- a/sw/source/core/layout/flycnt.cxx
+++ b/sw/source/core/layout/flycnt.cxx
@@ -416,9 +416,11 @@ void SwFlyAtContentFrame::MakeAll(vcl::RenderContext* pRenderContext)
// <SwObjectFormatterTextFrame::CheckMovedFwdCondition(..)>
sal_uInt32 nToPageNum( 0 );
bool bDummy( false );
+ bool bPageHasFlysAnchoredBelowThis(false);
if ( SwObjectFormatterTextFrame::CheckMovedFwdCondition(
*this, GetPageFrame()->GetPhyPageNum(),
- bAnchoredAtMaster, nToPageNum, bDummy ) )
+ bAnchoredAtMaster, nToPageNum, bDummy,
+ bPageHasFlysAnchoredBelowThis) )
{
bConsiderWrapInfluenceDueToMovedFwdAnchor = true;
// mark anchor text frame
@@ -431,14 +433,22 @@ void SwFlyAtContentFrame::MakeAll(vcl::RenderContext* pRenderContext)
rDoc, *pAnchorTextFrame, nAnchorFrameToPageNum ) )
{
if ( nAnchorFrameToPageNum < nToPageNum )
- SwLayouter::RemoveMovedFwdFrame( rDoc, *pAnchorTextFrame );
+ {
+ if (!bPageHasFlysAnchoredBelowThis)
+ {
+ SwLayouter::RemoveMovedFwdFrame(rDoc, *pAnchorTextFrame);
+ }
+ }
else
bInsert = false;
}
if ( bInsert )
{
- SwLayouter::InsertMovedFwdFrame( rDoc, *pAnchorTextFrame,
- nToPageNum );
+ if (!bPageHasFlysAnchoredBelowThis)
+ {
+ SwLayouter::InsertMovedFwdFrame(rDoc, *pAnchorTextFrame,
+ nToPageNum);
+ }
}
}
}
diff --git a/sw/source/core/layout/objectformattertxtfrm.cxx b/sw/source/core/layout/objectformattertxtfrm.cxx
index 1036ea3f05c3..5f482039a378 100644
--- a/sw/source/core/layout/objectformattertxtfrm.cxx
+++ b/sw/source/core/layout/objectformattertxtfrm.cxx
@@ -30,6 +30,7 @@
#include <fmtwrapinfluenceonobjpos.hxx>
#include <fmtfollowtextflow.hxx>
#include <layact.hxx>
+#include <flyfrm.hxx>
#include <ftnfrm.hxx>
#include <fmtornt.hxx>
#include <textboxhelper.hxx>
@@ -232,11 +233,13 @@ bool SwObjectFormatterTextFrame::DoFormatObj( SwAnchoredObject& _rAnchoredObj,
sal_uInt32 nToPageNum( 0 );
// #i43913#
bool bDummy( false );
+ bool bPageHasFlysAnchoredBelowThis(false);
// #i58182# - consider new method signature
if ( SwObjectFormatterTextFrame::CheckMovedFwdCondition( *GetCollectedObj( nIdx ),
GetPgNumOfCollected( nIdx ),
IsCollectedAnchoredAtMaster( nIdx ),
- nToPageNum, bDummy ) )
+ nToPageNum, bDummy,
+ bPageHasFlysAnchoredBelowThis))
{
// #i49987# - consider, that anchor frame
// could already been marked to move forward.
@@ -247,7 +250,12 @@ bool SwObjectFormatterTextFrame::DoFormatObj( SwAnchoredObject& _rAnchoredObj,
rDoc, mrAnchorTextFrame, nMovedFwdToPageNum ) )
{
if ( nMovedFwdToPageNum < nToPageNum )
- SwLayouter::RemoveMovedFwdFrame( rDoc, mrAnchorTextFrame );
+ {
+ if (!bPageHasFlysAnchoredBelowThis)
+ {
+ SwLayouter::RemoveMovedFwdFrame(rDoc, mrAnchorTextFrame);
+ }
+ }
else
bInsert = false;
}
@@ -255,8 +263,11 @@ bool SwObjectFormatterTextFrame::DoFormatObj( SwAnchoredObject& _rAnchoredObj,
{
// Indicate that anchor text frame has to move forward and
// invalidate its position to force a re-format.
- SwLayouter::InsertMovedFwdFrame( rDoc, mrAnchorTextFrame,
- nToPageNum );
+ if (!bPageHasFlysAnchoredBelowThis)
+ {
+ SwLayouter::InsertMovedFwdFrame(rDoc,
+ mrAnchorTextFrame, nToPageNum);
+ }
mrAnchorTextFrame.InvalidatePos();
// Indicate restart of the layout process
@@ -358,13 +369,14 @@ bool SwObjectFormatterTextFrame::DoFormatObjs()
sal_uInt32 nToPageNum( 0 );
// #i43913#
bool bInFollow( false );
+ bool bPageHasFlysAnchoredBelowThis(false);
SwAnchoredObject* pObj = nullptr;
if ( !mrAnchorTextFrame.IsFollow() )
{
pObj = GetFirstObjWithMovedFwdAnchor(
// #i35017# - constant name has changed
text::WrapInfluenceOnPosition::ONCE_CONCURRENT,
- nToPageNum, bInFollow );
+ nToPageNum, bInFollow, bPageHasFlysAnchoredBelowThis );
}
// #i35911#
if ( pObj && pObj->HasClearedEnvironment() )
@@ -385,14 +397,22 @@ bool SwObjectFormatterTextFrame::DoFormatObjs()
rDoc, mrAnchorTextFrame, nTmpToPageNum ) )
{
if ( nTmpToPageNum < pAnchorPageFrame->GetPhyPageNum() )
- SwLayouter::RemoveMovedFwdFrame( rDoc, mrAnchorTextFrame );
+ {
+ if (!bPageHasFlysAnchoredBelowThis)
+ {
+ SwLayouter::RemoveMovedFwdFrame(rDoc, mrAnchorTextFrame);
+ }
+ }
else
bInsert = false;
}
if ( bInsert )
{
- SwLayouter::InsertMovedFwdFrame( rDoc, mrAnchorTextFrame,
- pAnchorPageFrame->GetPhyPageNum() );
+ if (bPageHasFlysAnchoredBelowThis)
+ {
+ SwLayouter::InsertMovedFwdFrame(rDoc, mrAnchorTextFrame,
+ pAnchorPageFrame->GetPhyPageNum());
+ }
mrAnchorTextFrame.InvalidatePos();
bSuccess = false;
InvalidatePrevObjs( *pObj );
@@ -511,7 +531,8 @@ void SwObjectFormatterTextFrame::InvalidateFollowObjs( SwAnchoredObject& _rAncho
SwAnchoredObject* SwObjectFormatterTextFrame::GetFirstObjWithMovedFwdAnchor(
const sal_Int16 _nWrapInfluenceOnPosition,
sal_uInt32& _noToPageNum,
- bool& _boInFollow )
+ bool& _boInFollow,
+ bool& o_rbPageHasFlysAnchoredBelowThis)
{
// #i35017# - constant names have changed
OSL_ENSURE( _nWrapInfluenceOnPosition == text::WrapInfluenceOnPosition::ONCE_SUCCESSIVE ||
@@ -535,7 +556,8 @@ SwAnchoredObject* SwObjectFormatterTextFrame::GetFirstObjWithMovedFwdAnchor(
if ( SwObjectFormatterTextFrame::CheckMovedFwdCondition( *GetCollectedObj( i ),
GetPgNumOfCollected( i ),
IsCollectedAnchoredAtMaster( i ),
- _noToPageNum, _boInFollow ) )
+ _noToPageNum, _boInFollow,
+ o_rbPageHasFlysAnchoredBelowThis) )
{
pRetAnchoredObj = pAnchoredObj;
break;
@@ -546,6 +568,38 @@ SwAnchoredObject* SwObjectFormatterTextFrame::GetFirstObjWithMovedFwdAnchor(
return pRetAnchoredObj;
}
+static SwRowFrame const* FindTopLevelRowFrame(SwFrame const*const pFrame)
+{
+ SwRowFrame * pRow = const_cast<SwFrame*>(pFrame)->FindRowFrame();
+ // looks like SwTabFrame has mbInfTab = true so go up 2 levels
+ while (pRow->GetUpper()->GetUpper()->IsInTab())
+ {
+ pRow = pRow->GetUpper()->GetUpper()->FindRowFrame();
+ }
+ return pRow;
+}
+
+static SwContentFrame const* FindFrameInBody(SwAnchoredObject const& rAnchored)
+{
+ SwFrame const*const pAnchor(rAnchored.GetAnchorFrame());
+ assert(pAnchor);
+ if (pAnchor->IsPageFrame() || pAnchor->FindFooterOrHeader())
+ {
+ return nullptr;
+ }
+ if (pAnchor->IsInFly())
+ {
+ return FindFrameInBody(*pAnchor->FindFlyFrame());
+ }
+ if (pAnchor->IsInFootnote())
+ {
+ return pAnchor->FindFootnoteFrame()->GetRef();
+ }
+ assert(pAnchor->IsInDocBody());
+ assert(pAnchor->IsContentFrame());
+ return static_cast<SwContentFrame const*>(pAnchor);
+}
+
// #i58182#
// - replace private method by corresponding static public method
bool SwObjectFormatterTextFrame::CheckMovedFwdCondition(
@@ -553,7 +607,8 @@ bool SwObjectFormatterTextFrame::CheckMovedFwdCondition(
const sal_uInt32 _nFromPageNum,
const bool _bAnchoredAtMasterBeforeFormatAnchor,
sal_uInt32& _noToPageNum,
- bool& _boInFollow )
+ bool& _boInFollow,
+ bool& o_rbPageHasFlysAnchoredBelowThis)
{
bool bAnchorIsMovedForward( false );
@@ -629,6 +684,71 @@ bool SwObjectFormatterTextFrame::CheckMovedFwdCondition(
}
}
+ if (bAnchorIsMovedForward)
+ {
+ // tdf#138518 try to determine if there is a fly on page _nFromPageNum
+ // which is anchored in a frame that is "below" the anchor frame
+ // of _rAnchoredObj, such that it should move to the next page before
+ // _rAnchoredObj does
+ SwPageFrame const& rAnchoredObjPage(*_rAnchoredObj.GetPageFrame());
+ assert(rAnchoredObjPage.GetPhyPageNum() == _nFromPageNum);
+ if (auto * pObjs = rAnchoredObjPage.GetSortedObjs())
+ {
+ for (SwAnchoredObject *const pObj : *pObjs)
+ {
+ SwPageFrame const*const pObjAnchorPage(pObj->FindPageFrameOfAnchor());
+ assert(pObjAnchorPage);
+ if ((pObjAnchorPage == &rAnchoredObjPage
+ ? _boInFollow // same-page but will move forward
+ : rAnchoredObjPage.GetPhyPageNum() < pObjAnchorPage->GetPhyPageNum())
+ && pObj->GetFrameFormat().GetAnchor().GetAnchorId()
+ != RndStdIds::FLY_AS_CHAR)
+ {
+ if (pPageFrameOfAnchor->GetPhyPageNum() < pObjAnchorPage->GetPhyPageNum())
+ {
+ SAL_INFO("sw.layout", "SwObjectFormatterTextFrame::CheckMovedFwdCondition(): o_rbPageHasFlysAnchoredBelowThis because next page");
+ o_rbPageHasFlysAnchoredBelowThis = true;
+ break;
+ }
+ // on same page: check if it's in next-chain in the document body
+ // (in case both are in the same fly the flag must not be
+ // set because the whole fly moves at once)
+ SwContentFrame const*const pInBodyFrameObj(FindFrameInBody(*pObj));
+ SwContentFrame const*const pInBodyFrameAnchoredObj(FindFrameInBody(_rAnchoredObj));
+ if (pInBodyFrameObj && pInBodyFrameAnchoredObj)
+ {
+ bool isBreakMore(false);
+ // currently this ignores index of at-char flys
+ for (SwContentFrame const* pContentFrame = pInBodyFrameAnchoredObj->FindNextCnt();
+ pContentFrame;
+ pContentFrame = pContentFrame->FindNextCnt())
+ {
+ if (pInBodyFrameObj == pContentFrame)
+ {
+ // subsequent cells in a row are not automatically
+ // "below" and the row could potentially be split
+ // TODO refine check if needed
+ if (!pInBodyFrameAnchoredObj->IsInTab()
+ || FindTopLevelRowFrame(pInBodyFrameAnchoredObj)
+ != FindTopLevelRowFrame(pInBodyFrameAnchoredObj))
+ { // anchored in next chain on same page
+ SAL_INFO("sw.layout", "SwObjectFormatterTextFrame::CheckMovedFwdCondition(): o_rbPageHasFlysAnchoredBelowThis because next chain on same page");
+ o_rbPageHasFlysAnchoredBelowThis = true;
+ isBreakMore = true;
+ }
+ break;
+ }
+ }
+ if (isBreakMore)
+ {
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
+
return bAnchorIsMovedForward;
}
diff --git a/sw/source/core/layout/objectformattertxtfrm.hxx b/sw/source/core/layout/objectformattertxtfrm.hxx
index 15667b9ead60..1fc6160507c6 100644
--- a/sw/source/core/layout/objectformattertxtfrm.hxx
+++ b/sw/source/core/layout/objectformattertxtfrm.hxx
@@ -96,7 +96,8 @@ class SwObjectFormatterTextFrame : public SwObjectFormatter
SwAnchoredObject* GetFirstObjWithMovedFwdAnchor(
const sal_Int16 _nWrapInfluenceOnPosition,
sal_uInt32& _noToPageNum,
- bool& _boInFollow );
+ bool& _boInFollow,
+ bool& o_rbPageHasFlysAnchoredBelowThis);
/** method to format the anchor frame for checking of the move forward condition
@@ -169,6 +170,9 @@ class SwObjectFormatterTextFrame : public SwObjectFormatter
output parameter - boolean, indicating that anchor text frame is
currently on the same page, but it's a follow of in a follow row,
which will move forward. value only relevant, if method return <true>.
+ @param o_rbPageHasFlysAnchoredBelowThis
+ output parameter - indicates that the page has flys anchored
+ somewhere below the anchor of the passed _rAnchoredObj
@return boolean
indicating, if 'anchor is moved forward'
@@ -177,7 +181,8 @@ class SwObjectFormatterTextFrame : public SwObjectFormatter
const sal_uInt32 _nFromPageNum,
const bool _bAnchoredAtMasterBeforeFormatAnchor,
sal_uInt32& _noToPageNum,
- bool& _boInFollow );
+ bool& _boInFollow,
+ bool& o_rbPageHasFlysAnchoredBelowThis);
};
#endif
More information about the Libreoffice-commits
mailing list