From ce274e8eaf738aa62a15c6ddb3a6b9eebc6d5119 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Tue, 13 Apr 2021 20:13:51 +0200 Subject: sw: layout: if fly's anchor moves forward, move fly off SwPageFrame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The problem is that on Show Changes->Hide Changes->Show Changes in a 311 page document, the fly "Grafik1" was initially on page 203 but ends up on page 204, with a fly-sized gap on page 194. In a 25 page cut down version of the bugdoc, on layout action 659 the fly's anchor SwTextFrame moves from page 21 to page 22, but the fly remains in the SwPageFrame's m_pSortedObjs, because it's not the anchor frame itself that moves but a distant previous frame, and page 21 goes valid. 0 SwFlowFrame::PasteTree(SwFrame*, SwLayoutFrame*, SwFrame*, SwFrame*) (pStart=0x57c9e30, pParent=0xba15c50, pSibling=0x5a0f920, pOldParent=0xb057690) at sw/source/core/layout/flowfrm.cxx:586 1 SwFlowFrame::MoveSubTree(SwLayoutFrame*, SwFrame*) (this=0x57c9f48, pParent=0xba15c50, pSibling=0x5a0f920) at sw/source/core/layout/flowfrm.cxx:677 2 SwFlowFrame::MoveFwd(bool, bool, bool) (this=0x57c9f48, bMakePage=true, bPageBreak=false, bMoveAlways=false) at sw/source/core/layout/flowfrm.cxx:2061 3 SwContentFrame::MakeAll(OutputDevice*) (this=0x57c9e30) at sw/source/core/layout/calcmove.cxx:1831 4 SwFrame::OptPrepareMake() (this=0x57c9e30) at sw/source/core/layout/calcmove.cxx:399 5 SwFrame::OptCalc() const (this=0x57c9e30) at sw/source/core/inc/frame.hxx:1065 6 SwLayAction::FormatContent_(SwContentFrame const*, SwPageFrame const*) (this=0x7ffec0191b30, pContent=0x57c9e30, pPage=0xb9a1fd0) at sw/source/core/layout/layact.cxx:1833 In subsequent layout actions the anchor frame moves forward one page at a time, until in action 665, when things get really interesting. On page 24, the anchor text frame 582 is formatted for the first time, and it moves the fly to page 24, after positioning it on the page. 2 SwPageFrame::MoveFly(SwFlyFrame*, SwPageFrame*) (this=0xb9a1fd0, pToMove=0x641d310, pDest=0x9125bb0) at sw/source/core/layout/flylay.cxx:972 3 SwFlyAtContentFrame::RegisterAtCorrectPage() (this=0x641d310) at sw/source/core/layout/flycnt.cxx:1432 4 SwAnchoredObject::SetVertPosOrientFrame(SwLayoutFrame const&) (this=0x641d468, _rVertPosOrientFrame=...) at sw/source/core/layout/anchoredobject.cxx:195 5 SwFlyAtContentFrame::MakeObjPos() (this=0x641d310) at sw/source/core/layout/flycnt.cxx:1466 6 SwFlyFreeFrame::MakeAll(OutputDevice*) (this=0x641d310) at sw/source/core/layout/flylay.cxx:223 7 SwFlyAtContentFrame::MakeAll(OutputDevice*) (this=0x641d310, pRenderContext=0x55b1f00) at sw/source/core/layout/flycnt.cxx:384 8 SwFrame::PrepareMake(OutputDevice*) (this=0x641d310, pRenderContext=0x55b1f00) at sw/source/core/layout/calcmove.cxx:375 9 SwFrame::Calc(OutputDevice*) const (this=0x641d310, pRenderContext=0x55b1f00) at sw/source/core/layout/trvlfrm.cxx:1791 10 SwFlyFrame::Calc(OutputDevice*) const (this=0x641d310, pRenderContext=0x55b1f00) at sw/source/core/layout/fly.cxx:2874 11 SwLayAction::FormatLayoutFly(SwFlyFrame*) (this=0x7ffec0191b30, pFly=0x641d310) at sw/source/core/layout/layact.cxx:1455 12 SwObjectFormatter::FormatObj_(SwAnchoredObject&) (this=0xa5c0d10, _rAnchoredObj=...) at sw/source/core/layout/objectformatter.cxx:286 13 SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject&, bool) (this=0xa5c0d10, _rAnchoredObj=..., _bCheckForMovedFwd=false) at sw/source/core/layout/objectformattertxtfrm.cxx:135 14 SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame*) (this=0xa5c0d10, _pMasterTextFrame=0x0) at sw/source/core/layout/objectformatter.cxx:408 15 SwObjectFormatterTextFrame::DoFormatObjs() (this=0xa5c0d10) at sw/source/core/layout/objectformattertxtfrm.cxx:337 16 SwObjectFormatter::FormatObjsAtFrame(SwFrame&, SwPageFrame const&, SwLayAction*) (_rAnchorFrame=..., _rPageFrame=..., _pLayAction=0x7ffec0191b30) at sw/source/core/layout/objectformatter.cxx:160 17 SwLayAction::FormatContent(SwPageFrame const*) (this=0x7ffec0191b30, pPage=0x9125bb0) at sw/source/core/layout/layact.cxx:1675 18 SwLayAction::InternalAction(OutputDevice*) (this=0x7ffec0191b30, pRenderContext=0x55b1f00) at sw/source/core/layout/layact.cxx:771 Nothing invalidates page 21 now that the fly is gone, and formatting on page 24 is kind of pointless now because everything from page 21 on is wrongly positioned. (It's possible to skip out of the main layout action loop via SetNextCycle()/IsAgain(), but at this point it's in layact:771 in the layout-all-the-visible-pages loop that follows the main loop, and that one can't be cancelled.) Then DoFormatObjs() is called on frame 582, and this calls FormatAnchorFrameForCheckMoveFwd(), which formats previous frame 581, splitting it and moving its follow along with 582 to page 25. Here SwMovedFwdFramesByObjPos::Insert() is called, and now the anchor text frame 582 cannot move back to page 24 because it's prevented by SwMovedFwdFramesByObjPos::FrameMovedFwdByObjPos(), but that was all based on the wrong assumption that the pages before 24 were completely formatted (this happens in action 670). Something later formats page 21 again, and so at the end there is a fly-sized hole at the bottom of page 24, with frame 582 at the top of page 25. It won't help to detect a situation where the fly is on a page previous to the page it's anchor frame is on in DoFormatObjs() because it was actually moved to the same page in a previous formatting of the anchor frame, in the same layout action. To fix this, try to detect in SwLayAction::FormatContent() if the anchor frame of any fly on the page has moved forward, and move those flys off the page; this is enough for the 25 page document. The 311 page document still has a hole on page 194 though; apparently the last content frame on the page is never reformatted, so invalidate its size. Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114066 Tested-by: Jenkins Reviewed-by: Michael Stahl (cherry picked from commit eb85de8e6b61fb3fcb6c03ae0145f7fe5478bccf) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114519 Reviewed-by: Thorsten Behrens (cherry picked from commit 810f7e4e0b61ee7cb3a7d6a1b503782d7248a4b1) tdf#141945 sw: layout: check master frame when moving fly forward The problem is that in the finished layout the fly frames are positioned on the first page but are in SwPageFrame::m_pSortedObjs of the second page. Don't use FindPageFrameOfAnchor() because that looks up the follow-frame that contains the anchor position. This was unintentional; the idea was to get flys anchored in subsequent paragraphs out of the way. This situation where it's on a follow-frame of the same paragraph is more complicated and less obvious, so don't try to solve it now. (regression from eb85de8e6b61fb3fcb6c03ae0145f7fe5478bccf) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115100 Tested-by: Jenkins Reviewed-by: Michael Stahl (cherry picked from commit 30512746c182b52f37f9a818d4e206c98e715cb7) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115080 Reviewed-by: Caolán McNamara (cherry picked from commit 41f68b652eb1ccdd4941e2270426461da8e66417) Change-Id: I232c6b305e8593bfecd885c36058777f3980f82f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115763 Tested-by: Michael Stahl Reviewed-by: Michael Stahl --- sw/inc/anchoreddrawobject.hxx | 1 + sw/inc/anchoredobject.hxx | 5 +++- sw/source/core/inc/flyfrm.hxx | 1 + sw/source/core/inc/flyfrms.hxx | 1 + sw/source/core/inc/layact.hxx | 2 +- sw/source/core/layout/anchoreddrawobject.cxx | 14 +++++++-- sw/source/core/layout/fly.cxx | 5 ++++ sw/source/core/layout/flycnt.cxx | 18 +++++++++--- sw/source/core/layout/layact.cxx | 44 +++++++++++++++++++++++++++- 9 files changed, 81 insertions(+), 10 deletions(-) diff --git a/sw/inc/anchoreddrawobject.hxx b/sw/inc/anchoreddrawobject.hxx index e4f57f3e09ff..b1729b3eaca1 100644 --- a/sw/inc/anchoreddrawobject.hxx +++ b/sw/inc/anchoreddrawobject.hxx @@ -95,6 +95,7 @@ class SW_DLLPUBLIC SwAnchoredDrawObject : public SwAnchoredObject page frame */ virtual void RegisterAtCorrectPage() override; + virtual void RegisterAtPage(SwPageFrame &) override; virtual bool SetObjTop_( const SwTwips _nTop) override; virtual bool SetObjLeft_( const SwTwips _nLeft) override; diff --git a/sw/inc/anchoredobject.hxx b/sw/inc/anchoredobject.hxx index 2a5c192f2af5..746b9ca3b69a 100644 --- a/sw/inc/anchoredobject.hxx +++ b/sw/inc/anchoredobject.hxx @@ -46,7 +46,8 @@ class SW_DLLPUBLIC SwAnchoredObject private: // drawing object representing the anchored object in the drawing layer SdrObject* mpDrawObj; - // frame the object is anchored at + /// Frame the object is anchored at. + /// For at-char/at-para anchor, this is always the master SwTextFrame. SwFrame* mpAnchorFrame; // #i28701 - page frame the object is registered at // note: no page frame for as-character anchored objects @@ -295,6 +296,8 @@ class SW_DLLPUBLIC SwAnchoredObject /** method to invalidate position of the anchored object */ virtual void InvalidateObjPos() = 0; + virtual void RegisterAtPage(SwPageFrame &) = 0; + /** method to perform necessary invalidations for the positioning of objects, for whose the wrapping style influence has to be considered on the object positioning. diff --git a/sw/source/core/inc/flyfrm.hxx b/sw/source/core/inc/flyfrm.hxx index c44725f73a6b..a0fcdeb1eb57 100644 --- a/sw/source/core/inc/flyfrm.hxx +++ b/sw/source/core/inc/flyfrm.hxx @@ -134,6 +134,7 @@ protected: page frame */ virtual void RegisterAtCorrectPage() override; + virtual void RegisterAtPage(SwPageFrame &) override; virtual bool SetObjTop_( const SwTwips _nTop ) override; virtual bool SetObjLeft_( const SwTwips _nLeft ) override; diff --git a/sw/source/core/inc/flyfrms.hxx b/sw/source/core/inc/flyfrms.hxx index 8e167b9490f5..04b505ddd548 100644 --- a/sw/source/core/inc/flyfrms.hxx +++ b/sw/source/core/inc/flyfrms.hxx @@ -170,6 +170,7 @@ protected: #i28701# */ virtual void RegisterAtCorrectPage() override; + virtual void RegisterAtPage(SwPageFrame &) override; virtual void Modify( const SfxPoolItem*, const SfxPoolItem* ) override; public: diff --git a/sw/source/core/inc/layact.hxx b/sw/source/core/inc/layact.hxx index da2250e7fb49..2b1b17a4afd2 100644 --- a/sw/source/core/inc/layact.hxx +++ b/sw/source/core/inc/layact.hxx @@ -111,7 +111,7 @@ class SwLayAction bool FormatLayout( OutputDevice* pRenderContext, SwLayoutFrame *, bool bAddRect = true ); bool FormatLayoutTab( SwTabFrame *, bool bAddRect ); - bool FormatContent( const SwPageFrame* pPage ); + bool FormatContent(SwPageFrame * pPage); void FormatContent_( const SwContentFrame* pContent, const SwPageFrame* pPage ); bool IsShortCut( SwPageFrame *& ); diff --git a/sw/source/core/layout/anchoreddrawobject.cxx b/sw/source/core/layout/anchoreddrawobject.cxx index de969773d8b9..5660fa118c3d 100644 --- a/sw/source/core/layout/anchoreddrawobject.cxx +++ b/sw/source/core/layout/anchoreddrawobject.cxx @@ -840,10 +840,18 @@ void SwAnchoredDrawObject::RegisterAtCorrectPage() } if ( pPageFrame && GetPageFrame() != pPageFrame ) { - if ( GetPageFrame() ) - GetPageFrame()->RemoveDrawObjFromPage( *this ); - pPageFrame->AppendDrawObjToPage( *this ); + RegisterAtPage(*pPageFrame); } } +void SwAnchoredDrawObject::RegisterAtPage(SwPageFrame & rPageFrame) +{ + assert(GetPageFrame() != &rPageFrame); + if (GetPageFrame()) + { + GetPageFrame()->RemoveDrawObjFromPage( *this ); + } + rPageFrame.AppendDrawObjToPage( *this ); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/layout/fly.cxx b/sw/source/core/layout/fly.cxx index 1f566f373125..ef654ca8fc2b 100644 --- a/sw/source/core/layout/fly.cxx +++ b/sw/source/core/layout/fly.cxx @@ -2796,6 +2796,11 @@ void SwFlyFrame::RegisterAtCorrectPage() // default behaviour is to do nothing. } +void SwFlyFrame::RegisterAtPage(SwPageFrame &) +{ + // default behaviour is to do nothing. +} + /** method to determine, if a on the Writer fly frame is possible OD 2004-05-11 #i28701# diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx index f3c442ac8cfa..7ac4cf41740a 100644 --- a/sw/source/core/layout/flycnt.cxx +++ b/sw/source/core/layout/flycnt.cxx @@ -1396,10 +1396,20 @@ void SwFlyAtContentFrame::RegisterAtCorrectPage() } if ( pPageFrame && GetPageFrame() != pPageFrame ) { - if ( GetPageFrame() ) - GetPageFrame()->MoveFly( this, pPageFrame ); - else - pPageFrame->AppendFlyToPage( this ); + RegisterAtPage(*pPageFrame); + } +} + +void SwFlyAtContentFrame::RegisterAtPage(SwPageFrame & rPageFrame) +{ + assert(GetPageFrame() != &rPageFrame); + if (GetPageFrame()) + { + GetPageFrame()->MoveFly( this, &rPageFrame ); + } + else + { + rPageFrame.AppendFlyToPage( this ); } } diff --git a/sw/source/core/layout/layact.cxx b/sw/source/core/layout/layact.cxx index 50d5b1fa6b7a..74fe4e85bcfd 100644 --- a/sw/source/core/layout/layact.cxx +++ b/sw/source/core/layout/layact.cxx @@ -62,6 +62,8 @@ #include #include #include +#include +#include #include // Save some typing work to avoid accessing destroyed pages. @@ -1602,8 +1604,48 @@ bool SwLayAction::FormatLayoutTab( SwTabFrame *pTab, bool bAddRect ) return bChanged; } -bool SwLayAction::FormatContent( const SwPageFrame *pPage ) +bool SwLayAction::FormatContent(SwPageFrame *const pPage) { + ::comphelper::ScopeGuard g([this, pPage]() { + if (IsAgain()) + { + return; // pPage probably deleted + } + if (auto const* pObjs = pPage->GetSortedObjs()) + { + std::vector> moved; + for (auto const pObj : *pObjs) + { + assert(!pObj->AnchorFrame()->IsTextFrame() + || !static_cast(pObj->AnchorFrame())->IsFollow()); + SwPageFrame *const pAnchorPage(pObj->AnchorFrame()->FindPageFrame()); + assert(pAnchorPage); + if (pAnchorPage != pPage + && pPage->GetPhyPageNum() < pAnchorPage->GetPhyPageNum() + && pObj->GetFrameFormat().GetAnchor().GetAnchorId() + != RndStdIds::FLY_AS_CHAR) + { + moved.emplace_back(pObj, pAnchorPage); + } + } + for (auto const& [pObj, pAnchorPage] : moved) + { + SAL_INFO("sw.layout", "SwLayAction::FormatContent: move anchored " << pObj << " from " << pPage->GetPhyPageNum() << " to " << pAnchorPage->GetPhyPageNum()); + pObj->RegisterAtPage(*pAnchorPage); + ::Notify_Background(pObj->GetDrawObj(), pPage, + pObj->GetObjRect(), PREP_FLY_LEAVE, false); + } + if (!moved.empty()) + { + pPage->InvalidateFlyLayout(); + if (auto *const pContent = pPage->FindLastBodyContent()) + { + pContent->InvalidateSize(); + } + } + } + }); + const SwContentFrame *pContent = pPage->ContainsContent(); const SwViewShell *pSh = m_pRoot->GetCurrShell(); const bool bBrowse = pSh && pSh->GetViewOptions()->getBrowseMode(); -- cgit