[Libreoffice-commits] core.git: Branch 'private/mst/sw_redlinehide_2' - 2 commits - sw/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Fri Aug 3 17:21:30 UTC 2018


Rebased ref, commits from common ancestor:
commit cf39722ea5f8fbd58e4fad1999a1bc5724911120
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Fri Aug 3 18:48:10 2018 +0200
Commit:     Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Fri Aug 3 18:48:10 2018 +0200

    sw_redlinehide_2: try to minimise invalidation on show/hide
    
    InvalidateAllContent(Size) will basically format every paragraph
    in the document from scratch; let's try to optimise this a bit
    by invalidating only the SwTextFrames that actually contain redlines
    and the SwPageFrames they're on, and also invalidate the position
    of all flys anchored at these frames as a precautionary measure.
    
    Change-Id: I22ed683dc2225992ee48faa6084f277ef8733e8b

diff --git a/sw/source/core/layout/wsfrm.cxx b/sw/source/core/layout/wsfrm.cxx
index 239da7288749..e3684accaeae 100644
--- a/sw/source/core/layout/wsfrm.cxx
+++ b/sw/source/core/layout/wsfrm.cxx
@@ -4207,6 +4207,20 @@ static void UnHideRedlines(SwRootFrame & rLayout,
                             pFrame->SetMergedPara(std::move(pMerged));
                         }
                         auto const pMerged(pFrame->GetMergedPara());
+                        if (pMerged)
+                        {
+                            // invalidate SwInvalidateFlags::Size
+                            pFrame->Prepare(PREP_CLEAR, nullptr, false);
+                            pFrame->InvalidatePage();
+                            if (auto const pObjs = pFrame->GetDrawObjs())
+                            {   // also invalidate position of existing flys
+                                // because they may need to be moved
+                                for (auto const pObject : *pObjs)
+                                {
+                                    pObject->InvalidateObjPos();
+                                }
+                            }
+                        }
                         if (pMerged
                             // do this only *once*, for the *last* frame
                             // otherwise AppendObj would create multiple frames for fly-frames!
@@ -4252,6 +4266,16 @@ static void UnHideRedlines(SwRootFrame & rLayout,
                 {
                     if (auto const& pMergedPara = pFrame->GetMergedPara())
                     {
+                        // invalidate SwInvalidateFlags::Size
+                        pFrame->Prepare(PREP_CLEAR, nullptr, false);
+                        pFrame->InvalidatePage();
+                        if (auto const pObjs = pFrame->GetDrawObjs())
+                        {   // also invalidate position of existing flys
+                            for (auto const pObject : *pObjs)
+                            {
+                                pObject->InvalidateObjPos();
+                            }
+                        }
                         // SwFlyAtContentFrame::Modify() always appends to
                         // the master frame, so do the same here.
                         // (RemoveFootnotesForNode must be called at least once)
@@ -4434,7 +4458,7 @@ void SwRootFrame::SetHideRedlines(bool const bHideRedlines)
     UnHideRedlinesExtras(*this, rNodes, rNodes.GetEndOfAutotext());
     UnHideRedlines(*this, rNodes, rNodes.GetEndOfContent());
 
-    InvalidateAllContent(SwInvalidateFlags::Size); // ??? TODO what to invalidate?
+//    InvalidateAllContent(SwInvalidateFlags::Size); // ??? TODO what to invalidate?  this is the big hammer
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
commit fc346b71c215587069156a1f49d0d2b11935d5e2
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Thu Aug 2 17:09:53 2018 +0200
Commit:     Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Fri Aug 3 18:47:19 2018 +0200

    sw_redlinehide_2: show/hide flys in redlines
    
    The old implementation would not actually hide at-char flys but move
    the anchor, which is clearly not ideal; better to hide them.
    
    Change-Id: If21d0360e04857752a2c84f5329eadfad7818ac8

diff --git a/sw/source/core/inc/frmtool.hxx b/sw/source/core/inc/frmtool.hxx
index 0ffe4a75c0f6..e692bb900ef6 100644
--- a/sw/source/core/inc/frmtool.hxx
+++ b/sw/source/core/inc/frmtool.hxx
@@ -45,6 +45,8 @@ class GraphicAttr;
 class SwPageDesc;
 class SwFrameFormats;
 class SwRegionRects;
+class SwTextNode;
+namespace sw { struct Extent; }
 
 #define FAR_AWAY (SAL_MAX_INT32 - 20000)  // initial position of a Fly
 #define BROWSE_HEIGHT (56700L * 10L) // 10 Meters
@@ -55,6 +57,15 @@ class SwRegionRects;
 void AppendObjs( const SwFrameFormats *pTable, sal_uLong nIndex,
                        SwFrame *pFrame, SwPageFrame *pPage, SwDoc* doc );
 
+void AppendObjsOfNode(SwFrameFormats const* pTable, sal_uLong nIndex,
+        SwFrame * pFrame, SwPageFrame * pPage, SwDoc * pDoc,
+        std::vector<sw::Extent>::const_iterator * pIter,
+        std::vector<sw::Extent>::const_iterator const* pEnd);
+
+void RemoveHiddenObjsOfNode(SwTextNode const& rNode,
+        std::vector<sw::Extent>::const_iterator * pIter,
+        std::vector<sw::Extent>::const_iterator const* pEnd);
+
 // draw background with brush or graphics
 // The 6th parameter indicates that the method should consider background
 // transparency, saved in the color of the brush item.
diff --git a/sw/source/core/layout/frmtool.cxx b/sw/source/core/layout/frmtool.cxx
index ad8d07e2cc2f..757af06f176a 100644
--- a/sw/source/core/layout/frmtool.cxx
+++ b/sw/source/core/layout/frmtool.cxx
@@ -1039,32 +1039,63 @@ static bool IsShown(sal_uLong const nIndex,
     std::vector<sw::Extent>::const_iterator const*const pEnd)
 {
     SwPosition const& rAnchor(*rAnch.GetContentAnchor());
+    if (rAnchor.nNode.GetIndex() != nIndex)
+    {
+        return false;
+    }
     if (pIter && rAnch.GetAnchorId() != RndStdIds::FLY_AT_PARA)
     {
-        // TODO are frames sorted by anchor positions perhaps?
+        // note: frames are not sorted by anchor position.
         assert(pEnd);
         assert(rAnch.GetAnchorId() != RndStdIds::FLY_AT_FLY);
-        for ( ; *pIter != *pEnd; ++*pIter)
+        for (auto iter = *pIter; iter != *pEnd; ++iter)
         {
-            assert((**pIter).pNode->GetIndex() == nIndex);
-            if ((**pIter).nStart <= rAnchor.nContent.GetIndex())
+            assert(iter->pNode->GetIndex() == nIndex);
+            if (rAnchor.nContent.GetIndex() < iter->nStart)
             {
-                // TODO off by one? need < for AS_CHAR but what for AT_CHAR?
-                if (rAnchor.nContent.GetIndex() < (**pIter).nEnd)
-                {
-                    return true;
-                }
-                else
-                {
-                    return false;
-                }
+                return false;
+            }
+            // for AS_CHAR obviously must be <
+            // for AT_CHAR it is questionable whether < or <= should be used
+            // and there is the additional corner case of Len() to consider
+            // prefer < for now for symmetry (and inverted usage with
+            // "hidden") and handle special case explicitly
+            if (rAnchor.nContent.GetIndex() < iter->nEnd
+                || iter->nEnd == iter->pNode->Len())
+            {
+                return true;
             }
         }
         return false;
     }
     else
     {
-        return rAnch.GetContentAnchor()->nNode.GetIndex() == nIndex;
+        return true;
+    }
+}
+
+void RemoveHiddenObjsOfNode(SwTextNode const& rNode,
+    std::vector<sw::Extent>::const_iterator *const pIter,
+    std::vector<sw::Extent>::const_iterator const*const pEnd)
+{
+    std::vector<SwFrameFormat*> const*const pFlys(rNode.GetAnchoredFlys());
+    if (!pFlys)
+    {
+        return;
+    }
+    for (SwFrameFormat * pFrameFormat : *pFlys)
+    {
+        SwFormatAnchor const& rAnchor = pFrameFormat->GetAnchor();
+        if (rAnchor.GetAnchorId() == RndStdIds::FLY_AT_CHAR
+            || (rAnchor.GetAnchorId() == RndStdIds::FLY_AS_CHAR
+                && RES_DRAWFRMFMT == pFrameFormat->Which()))
+        {
+            assert(rAnchor.GetContentAnchor()->nNode.GetIndex() == rNode.GetIndex());
+            if (!IsShown(rNode.GetIndex(), rAnchor, pIter, pEnd))
+            {
+                pFrameFormat->DelFrames();
+            }
+        }
     }
 }
 
diff --git a/sw/source/core/layout/wsfrm.cxx b/sw/source/core/layout/wsfrm.cxx
index 68d9e1e418f1..239da7288749 100644
--- a/sw/source/core/layout/wsfrm.cxx
+++ b/sw/source/core/layout/wsfrm.cxx
@@ -4181,7 +4181,15 @@ static void UnHideRedlines(SwRootFrame & rLayout,
             {
                 if (pFrame->getRootFrame() == &rLayout)
                 {
-                    frames.push_back(pFrame);
+                    if (pFrame->IsFollow())
+                    {
+                        frames.push_back(pFrame);
+                    }    // when hiding, the loop must remove the anchored flys
+                    else // *before* resetting SetMergedPara anywhere - else
+                    {    // the fly deletion code will access multiple of the
+                         // frames with inconsistent MergedPara and assert
+                        frames.insert(frames.begin(), pFrame);
+                    }
                 }
             }
             // this messes with pRegisteredIn so do it outside SwIterator
@@ -4193,29 +4201,111 @@ static void UnHideRedlines(SwRootFrame & rLayout,
                         rNode.GetRedlineMergeFlag() == SwNode::Merge::NonFirst);
                     if (rNode.IsCreateFrameWhenHidingRedlines())
                     {
-                        pFrame->SetMergedPara(CheckParaRedlineMerge(*pFrame,
-                                rTextNode, sw::FrameMode::Existing));
-                        // ??? TODO flys etc.
+                        {
+                            auto pMerged(CheckParaRedlineMerge(*pFrame,
+                                    rTextNode, sw::FrameMode::Existing));
+                            pFrame->SetMergedPara(std::move(pMerged));
+                        }
+                        auto const pMerged(pFrame->GetMergedPara());
+                        if (pMerged
+                            // do this only *once*, for the *last* frame
+                            // otherwise AppendObj would create multiple frames for fly-frames!
+                            && !pFrame->GetFollow())
+                        {
+                            // add visible flys in non-first node to merged frame
+                            // (hidden flys remain and are deleted via DelFrames())
+                            SwFrameFormats& rTable(*rTextNode.GetDoc()->GetSpzFrameFormats());
+                            SwPageFrame *const pPage(pFrame->FindPageFrame());
+                            std::vector<sw::Extent>::const_iterator iterFirst(pMerged->extents.begin());
+                            std::vector<sw::Extent>::const_iterator iter(iterFirst);
+                            SwTextNode const* pNode(&rTextNode);
+                            for ( ; ; ++iter)
+                            {
+                                if (iter == pMerged->extents.end()
+                                    || iter->pNode != pNode)
+                                {
+                                    if (pNode == &rTextNode)
+                                    {   // remove existing hidden at-char anchored flys
+                                        RemoveHiddenObjsOfNode(
+                                            rTextNode, &iterFirst, &iter);
+                                    }
+                                    else
+                                    {
+                                        // pNode's frame has been deleted by CheckParaRedlineMerge()
+                                        AppendObjsOfNode(&rTable,
+                                            pNode->GetIndex(), pFrame, pPage,
+                                            rTextNode.GetDoc(),
+                                            &iterFirst, &iter);
+                                    }
+                                    if (iter == pMerged->extents.end())
+                                    {
+                                        break;
+                                    }
+                                    pNode = iter->pNode;
+                                    iterFirst = iter;
+                                }
+                            }
+                        }
                     }
                 }
                 else
                 {
                     if (auto const& pMergedPara = pFrame->GetMergedPara())
                     {
-                        // the new text frames don't exist yet, so at this point
-                        // we can only delete the footnote frames so they don't
-                        // point to the merged SwTextFrame any more...
-                        SwTextNode const* pNode(&rTextNode);
-                        for (auto const& rExtent : pMergedPara->extents)
+                        // SwFlyAtContentFrame::Modify() always appends to
+                        // the master frame, so do the same here.
+                        // (RemoveFootnotesForNode must be called at least once)
+                        if (!pFrame->IsFollow())
                         {
-                            if (rExtent.pNode != pNode)
+                            // the new text frames don't exist yet, so at this point
+                            // we can only delete the footnote frames so they don't
+                            // point to the merged SwTextFrame any more...
+                            SwTextNode const* pNode(&rTextNode);
+                            for (auto const& rExtent : pMergedPara->extents)
+                            {
+                                if (rExtent.pNode != pNode)
+                                {
+                                    sw::RemoveFootnotesForNode(*pFrame, *rExtent.pNode, nullptr);
+                                    // similarly, remove the anchored flys
+                                    if (auto const pFlys = rExtent.pNode->GetAnchoredFlys())
+                                    {
+                                        for (SwFrameFormat * pFormat : *pFlys)
+                                        {
+                                            pFormat->DelFrames(/*&rLayout*/);
+                                        }
+                                    }
+                                    pNode = rExtent.pNode;
+                                }
+                            }
+                            // add all flys in first node that are hidden
+                            std::vector<sw::Extent> hidden;
+                            sal_Int32 nLast(0);
+                            for (auto const& rExtent : pMergedPara->extents)
+                            {
+                                if (rExtent.pNode != &rTextNode)
+                                {
+                                    break;
+                                }
+                                if (rExtent.nStart != 0)
+                                {
+                                    assert(rExtent.nStart != nLast);
+
+                                    hidden.emplace_back(&rTextNode, nLast, rExtent.nStart);
+                                }
+                                nLast = rExtent.nEnd;
+                            }
+                            if (nLast != rTextNode.Len())
                             {
-                                sw::RemoveFootnotesForNode(*pFrame, *rExtent.pNode, nullptr);
-                                pNode = rExtent.pNode;
+                                hidden.emplace_back(&rTextNode, nLast, rTextNode.Len());
                             }
+                            SwFrameFormats& rTable(*rTextNode.GetDoc()->GetSpzFrameFormats());
+                            auto iterBegin(hidden.cbegin());
+                            auto const iterEnd(hidden.cend());
+                            AppendObjsOfNode(&rTable, rTextNode.GetIndex(), pFrame,
+                                pFrame->FindPageFrame(), rTextNode.GetDoc(),
+                                &iterBegin, &iterEnd);
                         }
                         pFrame->SetMergedPara(nullptr);
-                        // ??? TODO flys etc.
                         // ??? TODO recreate? or is invalidate enough?
                     }
                 }


More information about the Libreoffice-commits mailing list