[Libreoffice-commits] core.git: sc/inc sc/qa sc/source test/source

Regina Henschel (via logerrit) logerrit at kemper.freedesktop.org
Sat Oct 31 14:36:34 UTC 2020


 sc/inc/drwlayer.hxx                                 |    2 
 sc/inc/table.hxx                                    |    2 
 sc/qa/unit/filters-test.cxx                         |   16 
 sc/qa/unit/scshapetest.cxx                          |    2 
 sc/qa/unit/ucalc.cxx                                |   10 
 sc/source/core/data/drwlayer.cxx                    |  358 +++++++++++++++-----
 sc/source/core/data/table5.cxx                      |   41 ++
 sc/source/filter/xml/xmlexprt.cxx                   |    4 
 sc/source/filter/xml/xmlimprt.cxx                   |   32 +
 sc/source/filter/xml/xmlimprt.hxx                   |    2 
 test/source/sheet/xsheetannotationshapesupplier.cxx |    2 
 11 files changed, 362 insertions(+), 109 deletions(-)

New commits:
commit 1f0b3c7a40edfa81bbc7a58d123a6a2dfd83e4ca
Author:     Regina Henschel <rb.henschel at t-online.de>
AuthorDate: Sat Oct 10 17:55:31 2020 +0200
Commit:     Regina Henschel <rb.henschel at t-online.de>
CommitDate: Sat Oct 31 15:36:00 2020 +0100

    Improve 'resize with cell' handling
    
    The patch contains a larger rework of RecalcPos and connected areas
    and covers several bugs. Essentials in short:
    Move initialization from RecalcPos to own method and use it in
    ScXMLImport::endDocument
    Do not change hidden objects, which includes not setting width or
    height to zero, and be consistent in using object visibility.
    Special handling of vertical flipped customshapes.
    Repair anchor on import of line and measure line.
    ODF conformance: Create logical rectangle from anchor instead using
    size.
    
    Details:
    tdf#137044
    ScDrawLayer::SetPageSize is called several times while loading a
    document. It includes a call to ScDrawLayer::RecalcPos for all cell
    anchored objects. An object gets initialized with the first call.
    Problem was, that the row heights were not finished at that time and
    anchor cells and offsets were partly calculated based on default cell
    height. That results in wrong height and offset of objects.
    The solution separates initialization from RecalcPos and puts it into
    an own method ScDrawLayer::InitializeCellAnchoredObj. This is then
    called from ScXMLImport::endDocument when row height settings are
    finished.
    The call to RecalcPos is not totally removed from SetPageSize but only
    excluded while loading, because it is needed for size changes after
    the document is loaded.
    
    tdf#137576 partly
    For measure lines and ordinary lines, which were anchored 'To cell
    (resize with cell)', LibreOffice has written wrong end-cell info to
    file. So reopening results in wrong lines. The geometry of lines is
    based on two points. Fortunatelly the combination of position of the
    cell, which contains the shape, and start and end points gives correct
    absolute position of these points.
    Solution is, to regenerate the initial ScDrawObjData infos from these
    points and do not use the stored end-cell info. For a total fix
    implementation of NbcSetSnapRect for SdrMeasureObj is needed, which is
    not included here.
    
    tdf#137020
    Cell anchored shapes are contained in a cell in file. To determine
    size and position of the shape a rectangle is used, so defined, that
    after applying transformation you get the desired shape. In case of
    custom shapes, a vertical flip is not contained in the transformation
    but it is an attribute inside the shape and flip is done at the shape
    center and will not change the rectangle.
    This rectangle determines start and end addresses and offsets in
    ScDrawObjData in rNoRotatedAnchor. The info is used directly in XML
    export. It is correctly build while loading the file.
    But in case of vertical flipped custom shapes the logical rectangle
    of the shape has an additional 180deg rotation. Changing that behavior
    is currently out of scope. Therefore special handling of vertical
    flipped custom shapes was added.
    
    tdf#99549
    ODF specifies that in case of existence of end-cell attribute, size
    attributes have to be ignored. But LO has based the logical rectangle
    on size. In addition it has written zero width and height in case of
    hidden row and cols. Result was, that objects are 'lost' on opening
    although they still exists in the file.
    With the patch the object size is recalculated from anchor on opening.
    
    tdf#137355, tdf#137044, tdf#115655
    The old solution has recalculated the snap rectangle based on current
    state of hidden row or column. That has produced shapes of zero width
    or height and loss of offset in case start or end cell of the shape
    was hidden. In running LO it was partly offset by using cached infos
    in ScDrawObjData. That failed in case of save and reload.
    Solution is, to only change visible shapes. It is enough to adapt the
    shape when it becomes visible. That is introduced in RecalcPos and
    SetCellAnchoredFromPosition.
    
    tdf#137216
    Shapes anchored to cell were not hidden, if the column of its anchor
    was hidden, and undo of hiding an image in a cell by hiding its column
    didn't work. Reason was, that the shapes were not set to hidden in the
    shapes geometry.
    Solution is to copy a similar part from SetRowHidden to SetColHidden.
    
    without bugreport, but detected while debugging
    LO has used a cell reference with bHiddenAsZero as true in shape
    export. That has resulted in wrong offsets.
    
    Unittest changes:
    Test::testGraphicsInGroup()
    ScShapeTest::testCustomShapeCellAnchoredRotatedShape()
    Set expected values so, that they correspond to anchor in file.
    ScFiltersTest::testLegacyCellAnchoredRotatedShape()
    FIXME is solved now and the test is adaped to reflect that.
    XSheetAnnotationShapeSupplier::testGetAnnotationShape()
    Expected value is adapted to the fact, that now annotation shape
    gets its position after optimal row height is applied.
    
    Change-Id: Iffee996054ebf79e04044da5520f8d1a8a48b7c1
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104643
    Tested-by: Jenkins
    Reviewed-by: Regina Henschel <rb.henschel at t-online.de>

diff --git a/sc/inc/drwlayer.hxx b/sc/inc/drwlayer.hxx
index c8fd1fc7336a..1aa0998e2648 100644
--- a/sc/inc/drwlayer.hxx
+++ b/sc/inc/drwlayer.hxx
@@ -136,6 +136,7 @@ public:
 
     void            MoveArea( SCTAB nTab, SCCOL nCol1,SCROW nRow1, SCCOL nCol2,SCROW nRow2,
                                 SCCOL nDx,SCROW nDy, bool bInsDel, bool bUpdateNoteCaptionPos );
+    void InitializeCellAnchoredObj(SdrObject* pObj, ScDrawObjData& rData);
     void            RecalcPos( SdrObject* pObj, ScDrawObjData& rData, bool bNegativePage, bool bUpdateNoteCaptionPos );
 
     bool            HasObjectsInRows( SCTAB nTab, SCROW nStartRow, SCROW nEndRow );
@@ -189,6 +190,7 @@ public:
     std::vector<SdrObject*> GetObjectsAnchoredToRows(SCTAB nTab, SCROW nStartRow, SCROW nEndRow);
     std::map<SCROW, std::vector<SdrObject*>> GetObjectsAnchoredToRange(SCTAB nTab, SCCOL nCol, SCROW nStartRow, SCROW nEndRow);
     bool HasObjectsAnchoredInRange(const ScRange& rRange);
+    std::vector<SdrObject*> GetObjectsAnchoredToCols(SCTAB nTab, SCCOL nStartCol, SCCOL nEndCol);
     void MoveObject(SdrObject* pObj, const ScAddress& rNewPosition);
 
     // positions for detective lines
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 5a5bc5540963..9edf642f6f46 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -77,7 +77,7 @@ class RowHeightContext;
 class CompileFormulaContext;
 struct SetFormulaDirtyContext;
 class ColumnIterator;
-
+class ScDrawObjData;
 }
 
 class SfxItemSet;
diff --git a/sc/qa/unit/filters-test.cxx b/sc/qa/unit/filters-test.cxx
index 8cf4795ca91d..3c6e6c7d9bbb 100644
--- a/sc/qa/unit/filters-test.cxx
+++ b/sc/qa/unit/filters-test.cxx
@@ -555,11 +555,8 @@ void ScFiltersTest::testLegacyCellAnchoredRotatedShape()
         ScDocShellRef xDocSh = loadDoc("legacycellanchoredrotatedhiddenshape.", FORMAT_ODS, true);
         ScDocument& rDoc = xDocSh->GetDocument();
         // ensure the imported legacy rotated shape is in the expected position
-        // when a shape is fully hidden reloading seems to result is in some errors, usually
-        // ( same but different error happens pre-patch ) - we should do better here, I regard it
-        // as a pre-existing bug though (#FIXME)
-        //Rectangle aRect( 6000, -2000, 8000, 4000 ); // proper dimensions
-        tools::Rectangle aRect( 6000, -2000, 7430, 4000 );
+        tools::Rectangle aRect( 6000, -2000, 8000, 4000 );
+
         // ensure the imported (and converted) anchor (note we internally now store the anchor in
         // terms of the rotated shape) is more or less contains the correct info
         ScDrawObjData aAnchor;
@@ -569,10 +566,11 @@ void ScFiltersTest::testLegacyCellAnchoredRotatedShape()
         aAnchor.maEnd.SetCol( 7 );
         rDoc.ShowRows(0, 9, 0, true); // show relevant rows
         rDoc.SetDrawPageSize(0); // trigger recalcpos
-
-        // apply hefty (1 mm) tolerance here, as some opensuse tinderbox
-        // failing
-        impl_testLegacyCellAnchoredRotatedShape( rDoc, aRect, aAnchor, 100 );
+        impl_testLegacyCellAnchoredRotatedShape( rDoc, aRect, aAnchor);
+        // test save and reload
+        xDocSh = saveAndReload( &(*xDocSh), FORMAT_ODS);
+        ScDocument& rDoc2 = xDocSh->GetDocument();
+        impl_testLegacyCellAnchoredRotatedShape( rDoc2, aRect, aAnchor );
 
         xDocSh->DoClose();
     }
diff --git a/sc/qa/unit/scshapetest.cxx b/sc/qa/unit/scshapetest.cxx
index 5236f124cbc3..5efad446a70f 100644
--- a/sc/qa/unit/scshapetest.cxx
+++ b/sc/qa/unit/scshapetest.cxx
@@ -146,7 +146,7 @@ void ScShapeTest::testCustomShapeCellAnchoredRotatedShape()
     CPPUNIT_ASSERT(pObj);
 
     // Check Position and Size
-    tools::Rectangle aRect(2406, 754, 5774, 3692); // expected snap rect
+    tools::Rectangle aRect(2400, 751, 5772, 3693); // expected snap rect
     rDoc.SetDrawPageSize(0); // trigger recalcpos
     const tools::Rectangle& rShapeRect(pObj->GetSnapRect());
     const OUString sPosSizeErrors(lcl_compareRectWithTolerance(aRect, rShapeRect, 1));
diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index 78e27555e7b8..f5da5620744c 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -2797,17 +2797,15 @@ void Test::testGraphicsInGroup()
         m_pDoc->ShowRows(0, 100, 0, false);
         m_pDoc->SetDrawPageSize(0);
 
-        const tools::Long TOLERANCE = 30; //30 hmm
+        CPPUNIT_ASSERT_EQUAL_MESSAGE("Hiding should not change the logic rectangle",
+                               const_cast<const tools::Rectangle &>(aOrigRect), rNewRect);
+        CPPUNIT_ASSERT_MESSAGE("Hiding should make invisible", !pObj->IsVisible());
 
-        CPPUNIT_ASSERT_MESSAGE("Left and Right should be unchanged",
-            testEqualsWithTolerance(aOrigRect.Left(), rNewRect.Left(), TOLERANCE) &&
-            testEqualsWithTolerance(aOrigRect.Right(), rNewRect.Right(), TOLERANCE));
-        CPPUNIT_ASSERT_MESSAGE("Height should be minimum allowed height",
-            (rNewRect.Bottom() - rNewRect.Top()) <= 1);
         m_pDoc->ShowRows(0, 100, 0, true);
         m_pDoc->SetDrawPageSize(0);
         CPPUNIT_ASSERT_EQUAL_MESSAGE("Should not change when cell anchored",
                                const_cast<const tools::Rectangle &>(aOrigRect), rNewRect);
+        CPPUNIT_ASSERT_MESSAGE("Show should make visible", pObj->IsVisible());
     }
 
     {
diff --git a/sc/source/core/data/drwlayer.cxx b/sc/source/core/data/drwlayer.cxx
index 87b29f5749dc..302b76036384 100644
--- a/sc/source/core/data/drwlayer.cxx
+++ b/sc/source/core/data/drwlayer.cxx
@@ -32,9 +32,13 @@
 #include <svx/svdoutl.hxx>
 #include <svx/svditer.hxx>
 #include <svx/svdlayer.hxx>
+#include <svx/svdoashp.hxx>
+#include <svx/svdobj.hxx>
 #include <svx/svdocapt.hxx>
 #include <svx/svdograf.hxx>
 #include <svx/svdoole2.hxx>
+#include <svx/svdopath.hxx>
+#include <svx/svdtrans.hxx>
 #include <svx/svdundo.hxx>
 #include <svx/sdsxyitm.hxx>
 #include <svx/svxids.hrc>
@@ -70,6 +74,7 @@
 
 #include <vcl/fieldvalues.hxx>
 #include <memory>
+#include <algorithm>
 
 namespace com::sun::star::embed { class XEmbeddedObject; }
 
@@ -579,6 +584,12 @@ void ScDrawLayer::SetPageSize( sal_uInt16 nPageNo, const Size& rSize, bool bUpda
     //  even if size is still the same
     //  (individual rows/columns can have been changed))
 
+    // Do not call RecalcPos while loading, because row height is not finished, when SetPageSize
+    // is called first time. Instead the objects are initialized from ScXMLImport::endDocument() and
+    // RecalcPos is called from there.
+    if (!pDoc || pDoc->IsImportingXML())
+        return;
+
     bool bNegativePage = pDoc && pDoc->IsNegativePage( static_cast<SCTAB>(nPageNo) );
 
     // Disable mass broadcasts from drawing objects' position changes.
@@ -593,6 +604,7 @@ void ScDrawLayer::SetPageSize( sal_uInt16 nPageNo, const Size& rSize, bool bUpda
             RecalcPos( pObj, *pData, bNegativePage, bUpdateNoteCaptionPos );
     }
     setLock(bWasLocked);
+
 }
 
 namespace
@@ -627,8 +639,81 @@ namespace
         return tools::Rectangle(static_cast<tools::Long>(aRange.getMinX()), static_cast<tools::Long>(aRange.getMinY()),
             static_cast<tools::Long>(aRange.getMaxX()), static_cast<tools::Long>(aRange.getMaxY()));
     }
+
+bool lcl_AreRectanglesApproxEqual(const tools::Rectangle& rRectA, const tools::Rectangle& rRectB)
+{
+    // Twips <-> Hmm conversions introduce +-1 differences although there are no real changes in the object.
+    // Therefore test with == is not appropriate in some cases.
+    if (std::labs(rRectA.Left() - rRectB.Left()) > 1)
+        return false;
+    if (std::labs(rRectA.Top() - rRectB.Top()) > 1)
+        return false;
+    if (std::labs(rRectA.Right() - rRectB.Right()) > 1)
+        return false;
+    if (std::labs(rRectA.Bottom() - rRectB.Bottom()) > 1)
+        return false;
+    return true;
+}
+
+bool lcl_NeedsMirrorYCorrection(SdrObject* pObj)
+{
+    return pObj->GetObjIdentifier() == OBJ_CUSTOMSHAPE
+           && static_cast<SdrObjCustomShape*>(pObj)->IsMirroredY();
+}
+
+void lcl_SetLogicRectFromAnchor(SdrObject* pObj, ScDrawObjData& rAnchor, ScDocument* pDoc)
+{
+    // This is only used during initialization. At that time, shape handling is always LTR. No need
+    // to consider negative page.
+    if (!pObj || !pDoc || !rAnchor.maEnd.IsValid() || !rAnchor.maStart.IsValid())
+        return;
+
+    // In case of a vertical mirrored custom shape, LibreOffice uses internally an additional 180deg
+    // in aGeo.nRotationAngle and in turn has a different logic rectangle position. We remove flip,
+    // set the logic rectangle, and apply flip again. You cannot simple use a 180deg-rotated
+    // rectangle, because custom shape mirroring is internally applied after the other
+    // transformations.
+    if (lcl_NeedsMirrorYCorrection(pObj))
+    {
+        const tools::Rectangle aRect(pObj->GetSnapRect());
+        const Point aLeft(aRect.Left(), (aRect.Top() + aRect.Bottom()) >> 1);
+        const Point aRight(aLeft.X() + 1000, aLeft.Y());
+        pObj->NbcMirror(aLeft, aRight);
+    }
+
+    // Build full sized logic rectangle from start and end given in anchor.
+    const tools::Rectangle aStartCellRect(
+        pDoc->GetMMRect(rAnchor.maStart.Col(), rAnchor.maStart.Row(), rAnchor.maStart.Col(),
+                        rAnchor.maStart.Row(), rAnchor.maStart.Tab(), false /*bHiddenAsZero*/));
+    Point aStartPoint(aStartCellRect.Left(), aStartCellRect.Top());
+    aStartPoint.AdjustX(rAnchor.maStartOffset.getX());
+    aStartPoint.AdjustY(rAnchor.maStartOffset.getY());
+
+    const tools::Rectangle aEndCellRect(
+        pDoc->GetMMRect(rAnchor.maEnd.Col(), rAnchor.maEnd.Row(), rAnchor.maEnd.Col(),
+                        rAnchor.maEnd.Row(), rAnchor.maEnd.Tab(), false /*bHiddenAsZero*/));
+    Point aEndPoint(aEndCellRect.Left(), aEndCellRect.Top());
+    aEndPoint.AdjustX(rAnchor.maEndOffset.getX());
+    aEndPoint.AdjustY(rAnchor.maEndOffset.getY());
+
+    // Set this as new, full sized logical rectangle
+    tools::Rectangle aNewRectangle(aStartPoint, aEndPoint);
+    aNewRectangle.Justify();
+    if (!lcl_AreRectanglesApproxEqual(pObj->GetLogicRect(), aNewRectangle))
+        pObj->NbcSetLogicRect(lcl_makeSafeRectangle(aNewRectangle));
+
+    // The shape has the correct logical rectangle now. Reapply the above removed mirroring.
+    if (lcl_NeedsMirrorYCorrection(pObj))
+    {
+        const tools::Rectangle aRect(pObj->GetSnapRect());
+        const Point aLeft(aRect.Left(), (aRect.Top() + aRect.Bottom()) >> 1);
+        const Point aRight(aLeft.X() + 1000, aLeft.Y());
+        pObj->NbcMirror(aLeft, aRight);
+    }
 }
 
+} // namespace
+
 void ScDrawLayer::ResizeLastRectFromAnchor(const SdrObject* pObj, ScDrawObjData& rData,
                                            bool bUseLogicRect, bool bNegativePage, bool bCanResize,
                                            bool bHiddenAsZero)
@@ -783,6 +868,124 @@ void ScDrawLayer::ResizeLastRectFromAnchor(const SdrObject* pObj, ScDrawObjData&
     rData.setShapeRect(GetDocument(), lcl_makeSafeRectangle(aRect), pObj->IsVisible());
 }
 
+void ScDrawLayer::InitializeCellAnchoredObj(SdrObject* pObj, ScDrawObjData& rData)
+{
+    // This is called from ScXMLImport::endDocument()
+    if (!pDoc || !pObj)
+        return;
+    if (!rData.getShapeRect().IsEmpty())
+        return; // already initialized, should not happen
+    if (rData.meType == ScDrawObjData::CellNote || rData.meType == ScDrawObjData::ValidationCircle
+        || rData.meType == ScDrawObjData::DetectiveArrow)
+        return; // handled in RecalcPos
+
+    // Prevent multiple broadcasts during the series of changes.
+    bool bWasLocked = pObj->getSdrModelFromSdrObject().isLocked();
+    pObj->getSdrModelFromSdrObject().setLock(true);
+
+    // rNoRotatedAnchor refers in its start and end addresses and its start and end offsets to
+    // the logic rectangle of the object. The values are so, as if no hidden columns and rows
+    // exists and if it is a LTR sheet. These values are directly used for XML in ODF file.
+    // ToDO: Check whether its maShapeRect member is actually used.
+    ScDrawObjData& rNoRotatedAnchor = *GetNonRotatedObjData(pObj, true /*bCreate*/);
+
+    // From XML import, rData contains temporarily the anchor information as they are given in
+    // XML. Copy it to rNoRotatedAnchor, where it belongs. rData will later contain the anchor
+    // of the transformed object as visible on screen.
+    rNoRotatedAnchor.maStart = rData.maStart;
+    rNoRotatedAnchor.maEnd = rData.maEnd;
+    rNoRotatedAnchor.maStartOffset = rData.maStartOffset;
+    rNoRotatedAnchor.maEndOffset = rData.maEndOffset;
+
+    SCCOL nCol1 = rNoRotatedAnchor.maStart.Col();
+    SCROW nRow1 = rNoRotatedAnchor.maStart.Row();
+    SCTAB nTab1 = rNoRotatedAnchor.maStart.Tab(); // Used as parameter several times
+
+    // Object has coordinates relative to left/top of containing cell in XML. Change object to
+    // absolute coordinates as internally used.
+    const tools::Rectangle aRect(
+        pDoc->GetMMRect(nCol1, nRow1, nCol1, nRow1, nTab1, false /*bHiddenAsZero*/));
+    const Size aShift(aRect.Left(), aRect.Top());
+    pObj->NbcMove(aShift);
+
+    const ScAnchorType aAnchorType = ScDrawLayer::GetAnchorType(*pObj);
+    if (aAnchorType == SCA_CELL_RESIZE)
+    {
+        if (pObj->GetObjIdentifier() == OBJ_LINE)
+        {
+            // Horizontal lines might have wrong start and end anchor because of erroneously applied
+            // 180deg rotation (tdf#137446). Other lines have wrong end anchor. Coordinates in
+            // object are correct. Use them for recreating the anchor.
+            const basegfx::B2DPolygon aPoly(
+                reinterpret_cast<SdrPathObj*>(pObj)->GetPathPoly().getB2DPolygon(0));
+            const basegfx::B2DPoint aB2DPoint0(aPoly.getB2DPoint(0));
+            const basegfx::B2DPoint aB2DPoint1(aPoly.getB2DPoint(1));
+            const Point aPointLT(FRound(std::min(aB2DPoint0.getX(), aB2DPoint1.getX())),
+                                 FRound(std::min(aB2DPoint0.getY(), aB2DPoint1.getY())));
+            const Point aPointRB(FRound(std::max(aB2DPoint0.getX(), aB2DPoint1.getX())),
+                                 FRound(std::max(aB2DPoint0.getY(), aB2DPoint1.getY())));
+            const tools::Rectangle aObjRect(aPointLT, aPointRB);
+            GetCellAnchorFromPosition(aObjRect, rNoRotatedAnchor, *pDoc, nTab1,
+                                      false /*bHiddenAsZero*/);
+        }
+        else if (pObj->GetObjIdentifier() == OBJ_MEASURE)
+        {
+            // Measure lines might have got wrong start and end anchor from XML import. Recreate
+            // them from start and end point.
+            const Point aPoint0(pObj->GetPoint(0));
+            const Point aPoint1(pObj->GetPoint(1));
+            const Point aPointLT(std::min(aPoint0.X(), aPoint1.X()),
+                                 std::min(aPoint0.Y(), aPoint1.Y()));
+            const Point aPointRB(std::max(aPoint0.X(), aPoint1.X()),
+                                 std::max(aPoint0.Y(), aPoint1.Y()));
+            const tools::Rectangle aObjRect(aPointLT, aPointRB);
+            GetCellAnchorFromPosition(aObjRect, rNoRotatedAnchor, *pDoc, rData.maStart.Tab(),
+                                      false /*bHiddenAsZero*/);
+        }
+        else
+        {
+            // In case there are hidden rows or cols, versions 7.0 and earlier have written width and
+            // height in file so that hidden row or col are count as zero. XML import bases the
+            // logical rectangle of the object on it. Shapes have at least wrong size, when row or col
+            // are shown. We try to regenerate the logic rectangle as far as possible from the anchor.
+            // ODF specifies anyway, that the size has to be ignored, if end cell attributes exist.
+            lcl_SetLogicRectFromAnchor(pObj, rNoRotatedAnchor, pDoc);
+        }
+    }
+    else // aAnchorType == SCA_CELL, other types will not occure here.
+    {
+        // XML has no end cell address in this case. We generate it from position.
+        UpdateCellAnchorFromPositionEnd(*pObj, rNoRotatedAnchor, *pDoc, nTab1,
+                                        true /*bUseLogicRect*/);
+    }
+
+    // Make sure maShapeRect of rNoRotatedAnchor is not empty. ToDo: Really used?
+    // Currently ScExportTest::testMoveCellAnchoredShapesODS checks it.
+    rNoRotatedAnchor.setShapeRect(GetDocument(), pObj->GetLogicRect(), true);
+
+    // Start and end addresses and offsets in rData refer to the actual snap rectangle of the
+    // shape. We initialize them here based on the "full" sized object. Adaption to reduced size
+    // (by hidden row/col) is done later in RecalcPos.
+    GetCellAnchorFromPosition(pObj->GetSnapRect(), rData, *pDoc, nTab1, false /*bHiddenAsZero*/);
+
+    // As of ODF 1.3 strict there is no attribute to store whether an object is hidden. So a "visible"
+    // object might actually be hidden by being in hidden row or column. We detect it here.
+    // Note, that visibility by hidden row or column refers to the snap rectangle.
+    if (pObj->IsVisible()
+        && (pDoc->RowHidden(rData.maStart.Row(), rData.maStart.Tab())
+            || pDoc->ColHidden(rData.maStart.Col(), rData.maStart.Tab())))
+        pObj->SetVisible(false);
+
+    // Set visibiliy. ToDo: Really used?
+    rNoRotatedAnchor.setShapeRect(GetDocument(), pObj->GetLogicRect(), pObj->IsVisible());
+
+    // And set maShapeRect in rData. It stores not only the current rectangles, but currently,
+    // existance of maShapeRect is the flag for initialization is done.
+    rData.setShapeRect(GetDocument(), pObj->GetSnapRect(), pObj->IsVisible());
+
+    pObj->getSdrModelFromSdrObject().setLock(bWasLocked);
+}
+
 void ScDrawLayer::RecalcPos( SdrObject* pObj, ScDrawObjData& rData, bool bNegativePage, bool bUpdateNoteCaptionPos )
 {
     OSL_ENSURE( pDoc, "ScDrawLayer::RecalcPos - missing document" );
@@ -934,96 +1137,34 @@ void ScDrawLayer::RecalcPos( SdrObject* pObj, ScDrawObjData& rData, bool bNegati
                 }
             }
         }
-    }
-    else
+    } // end ScDrawObjData::DetectiveArrow
+    else // start ScDrawObjData::DrawingObject
     {
+        // Do not change hidden objects. That would produce zero height or width and loss of offsets.
+        if (!pObj->IsVisible())
+            return;
+
         // Prevent multiple broadcasts during the series of changes.
         bool bWasLocked = pObj->getSdrModelFromSdrObject().isLocked();
         pObj->getSdrModelFromSdrObject().setLock(true);
-        bool bCanResize = bValid2 && !pObj->IsResizeProtect() && rData.mbResizeWithCell;
 
-        //First time positioning, must be able to at least move it
-        ScDrawObjData& rNoRotatedAnchor = *GetNonRotatedObjData( pObj, true );
-        if (rData.getShapeRect().IsEmpty())
-        {
-            // Every shape it is saved with a negative offset relative to cell
-            ScAnchorType aAnchorType = ScDrawLayer::GetAnchorType(*pObj);
-            if (aAnchorType == SCA_CELL || aAnchorType == SCA_CELL_RESIZE)
-            {
-                // tdf#117145 All that TR*etBaseGeometry does here is to translate
-                // the existing transformation. This can simply be applied to the existing
-                // matrix, no need to decompose as done before. Also doing this from
-                // Calc did not change metrics in any way.
-                const tools::Rectangle aRect(pDoc->GetMMRect(nCol1, nRow1, nCol1 , nRow1, nTab1));
-                const Point aPoint(bNegativePage ? aRect.Right() : aRect.Left(), aRect.Top());
-                basegfx::B2DPolyPolygon aPolyPolygon;
-                basegfx::B2DHomMatrix aOriginalMatrix;
-
-                pObj->TRGetBaseGeometry(aOriginalMatrix, aPolyPolygon);
-                aOriginalMatrix.translate(aPoint.X(), aPoint.Y());
-                pObj->TRSetBaseGeometry(aOriginalMatrix, aPolyPolygon);
-            }
-
-            // It's confusing ( but blame that we persist the anchor in terms of unrotated shape )
-            // that the initial anchor we get here is in terms of an unrotated shape ( if the shape is rotated )
-            // we need to save the old anchor ( for persisting ) and also track any resize or repositions that happen.
-
-            // This is an evil hack, having an anchor that is one minute in terms of untransformed object and then later
-            // in terms of the transformed object is not ideal, similarly having 2 anchors per object is wasteful, can't
-            // see another way out of this at the moment though.
-            rNoRotatedAnchor.maStart = rData.maStart;
-            rNoRotatedAnchor.maEnd = rData.maEnd;
-            rNoRotatedAnchor.maStartOffset = rData.maStartOffset;
-            rNoRotatedAnchor.maEndOffset = rData.maEndOffset;
-
-            // get bounding rectangle of shape ( include any hidden row/columns ), <sigh> we need to do this
-            // because if the shape is rotated the anchor from xml is in terms of the unrotated shape, if
-            // the shape is hidden ( by the rows that contain the shape being hidden ) then our hack of
-            // trying to infer the 'real' e.g. rotated anchor from the SnapRect will fail (because the LogicRect will
-            // not have the correct position or size). The only way we can possible do this is to first get the
-            // 'unrotated' shape dimensions from the persisted Anchor (from xml) and then 'create' an Anchor from the
-            // associated rotated shape (note: we do this by actually setting the LogicRect for the shape temporarily to the
-            // *full* size then grabbing the SnapRect (which gives the transformed rotated dimensions), it would be
-            // wonderful if we could do this mathematically without having to temporarily tweak the object... otoh this way
-            // is guaranteed to get consistent results)
-            ResizeLastRectFromAnchor( pObj, rData, true, bNegativePage, bCanResize, false );
-            // aFullRect contains the unrotated size and position of the shape (regardless of any hidden row/columns)
-            tools::Rectangle aFullRect = rData.getShapeRect();
-
-            // get current size and position from the anchor for use later
-            ResizeLastRectFromAnchor( pObj, rNoRotatedAnchor, true, bNegativePage, bCanResize );
-
-            // resize/position the shape to *full* size e.g. how it would be ( if no hidden rows/cols affected things )
-            pObj->SetLogicRect(aFullRect);
-
-            // Ok, here is more nastiness, from xml the Anchor is in terms of the LogicRect which is the
-            // untransformed unrotated shape, here we swap out that initial anchor and from now on use
-            // an Anchor based on the SnapRect ( which is what you see on the screen )
-            const tools::Rectangle aObjRect(pObj->GetSnapRect());
-            ScDrawLayer::GetCellAnchorFromPosition(
-                aObjRect,
-                rData,
-                *pDoc,
-                nTab1,
-                false);
-
-            // reset shape to true 'maybe affected by hidden rows/cols' size calculated previously
-            pObj->SetLogicRect(rNoRotatedAnchor.getShapeRect());
-        }
+        bool bCanResize = bValid2 && !pObj->IsResizeProtect() && rData.mbResizeWithCell;
 
         // update anchor with snap rect
         ResizeLastRectFromAnchor( pObj, rData, false, bNegativePage, bCanResize );
 
+        ScDrawObjData& rNoRotatedAnchor = *GetNonRotatedObjData( pObj, true /*bCreate*/);
+
         if( bCanResize )
         {
             tools::Rectangle aNew = rData.getShapeRect();
-
-            if ( pObj->GetSnapRect() != aNew )
+            tools::Rectangle aOld(pObj->GetSnapRect());
+            if (!lcl_AreRectanglesApproxEqual(aNew, aOld))
             {
-                tools::Rectangle aOld(pObj->GetSnapRect());
-
                 if (bRecording)
                     AddCalcUndo( std::make_unique<SdrUndoGeoObj>( *pObj ) );
+
+                // ToDo: Implement NbcSetSnapRect of SdrMeasureObj. Then this can be removed.
                 tools::Long nOldWidth = aOld.GetWidth();
                 tools::Long nOldHeight = aOld.GetHeight();
                 if (pObj->IsPolyObj() && nOldWidth && nOldHeight)
@@ -1036,16 +1177,17 @@ void ScDrawLayer::RecalcPos( SdrObject* pObj, ScDrawObjData& rData, bool bNegati
                     double fYFrac = static_cast<double>(aNew.GetHeight()) / static_cast<double>(nOldHeight);
                     pObj->NbcResize(aNew.TopLeft(), Fraction(fXFrac), Fraction(fYFrac));
                 }
-                // order of these lines is important, modify rData.maLastRect carefully it is used as both
-                // a value and a flag for initialisation
+
                 rData.setShapeRect(GetDocument(), lcl_makeSafeRectangle(rData.getShapeRect()), pObj->IsVisible());
                 if (pObj->GetObjIdentifier() == OBJ_CUSTOMSHAPE)
                     pObj->AdjustToMaxRect(rData.getShapeRect());
                 else
                     pObj->SetSnapRect(rData.getShapeRect());
+
                 // update 'unrotated anchor' it's the anchor we persist, it must be kept in sync
                 // with the normal Anchor
-                ResizeLastRectFromAnchor( pObj, rNoRotatedAnchor, true, bNegativePage, bCanResize );
+                // ToDo: Is maShapeRect of rNoRotatedAnchor used at all?
+                ResizeLastRectFromAnchor(pObj, rNoRotatedAnchor, true, bNegativePage, bCanResize);
             }
         }
         else
@@ -1076,7 +1218,7 @@ void ScDrawLayer::RecalcPos( SdrObject* pObj, ScDrawObjData& rData, bool bNegati
         pObj->getSdrModelFromSdrObject().setLock(bWasLocked);
         if (!bWasLocked)
             pObj->BroadcastObjectChange();
-    }
+    } // end ScDrawObjData::DrawingObject
 }
 
 bool ScDrawLayer::GetPrintArea( ScRange& rRange, bool bSetHor, bool bSetVer ) const
@@ -2040,6 +2182,8 @@ void ScDrawLayer::SetCellAnchored( SdrObject &rObj, const ScDrawObjData &rAnchor
 void ScDrawLayer::SetCellAnchoredFromPosition( SdrObject &rObj, const ScDocument &rDoc, SCTAB nTab,
                                                bool bResizeWithCell )
 {
+    if (!rObj.IsVisible())
+        return;
     ScDrawObjData aAnchor;
     // set anchor in terms of the visual ( SnapRect )
     // object ( e.g. for when object is rotated )
@@ -2052,24 +2196,40 @@ void ScDrawLayer::SetCellAnchoredFromPosition( SdrObject &rObj, const ScDocument
 
     aAnchor.mbResizeWithCell = bResizeWithCell;
     SetCellAnchored( rObj, aAnchor );
+
+    // absolutely necessary to set flag, ScDrawLayer::RecalcPos expects it.
+    if ( ScDrawObjData* pAnchor = GetObjData( &rObj ) )
+    {
+        pAnchor->setShapeRect(&rDoc, rObj.GetSnapRect());
+    }
+
     // - keep also an anchor in terms of the Logic ( untransformed ) object
     // because that's what we stored ( and still do ) to xml
+
+    // Vertical flipped custom shapes need special treatment, see comment in
+    // lcl_SetLogicRectFromAnchor
+    tools::Rectangle aObjRect2;
+    if (lcl_NeedsMirrorYCorrection(&rObj))
+    {
+        const tools::Rectangle aRect(rObj.GetSnapRect());
+        const Point aLeft(aRect.Left(), (aRect.Top() + aRect.Bottom()) >> 1);
+        const Point aRight(aLeft.X() + 1000, aLeft.Y());
+        rObj.NbcMirror(aLeft, aRight);
+        aObjRect2 = rObj.GetLogicRect();
+        rObj.NbcMirror(aLeft, aRight);
+    }
+    else
+        aObjRect2 = rObj.GetLogicRect();
+
     ScDrawObjData aVisAnchor;
-    const tools::Rectangle aObjRect2(rObj.GetLogicRect());
     GetCellAnchorFromPosition(
         aObjRect2,
         aVisAnchor,
         rDoc,
-        nTab);
+        nTab, false);
 
     aVisAnchor.mbResizeWithCell = bResizeWithCell;
     SetVisualCellAnchored( rObj, aVisAnchor );
-    // absolutely necessary to set flag that in order to prevent ScDrawLayer::RecalcPos
-    // doing an initialisation hack
-    if ( ScDrawObjData* pAnchor = GetObjData( &rObj ) )
-    {
-        pAnchor->setShapeRect(&rDoc, rObj.GetSnapRect());
-    }
 }
 
 void ScDrawLayer::GetCellAnchorFromPosition(
@@ -2236,6 +2396,30 @@ bool ScDrawLayer::HasObjectsAnchoredInRange(const ScRange& rRange)
     return false;
 }
 
+std::vector<SdrObject*> ScDrawLayer::GetObjectsAnchoredToCols(SCTAB nTab, SCCOL nStartCol,
+                                                              SCCOL nEndCol)
+{
+    SdrPage* pPage = GetPage(static_cast<sal_uInt16>(nTab));
+    if (!pPage || pPage->GetObjCount() < 1)
+        return std::vector<SdrObject*>();
+
+    std::vector<SdrObject*> aObjects;
+    SdrObjListIter aIter(pPage, SdrIterMode::Flat);
+    SdrObject* pObject = aIter.Next();
+    ScRange aRange(nStartCol, 0, nTab, nEndCol, MAXROW, nTab);
+    while (pObject)
+    {
+        if (!dynamic_cast<SdrCaptionObj*>(pObject)) // Caption objects are handled differently
+        {
+            ScDrawObjData* pObjData = GetObjData(pObject);
+            if (pObjData && aRange.In(pObjData->maStart))
+                aObjects.push_back(pObject);
+        }
+        pObject = aIter.Next();
+    }
+    return aObjects;
+}
+
 void ScDrawLayer::MoveObject(SdrObject* pObject, const ScAddress& rNewPosition)
 {
     // Get anchor data
diff --git a/sc/source/core/data/table5.cxx b/sc/source/core/data/table5.cxx
index 06465dbc24a1..ff90463ba4c7 100644
--- a/sc/source/core/data/table5.cxx
+++ b/sc/source/core/data/table5.cxx
@@ -36,6 +36,7 @@
 #include <printopt.hxx>
 #include <bcaslot.hxx>
 #include <compressedarray.hxx>
+#include <userdat.hxx>
 
 #include <com/sun/star/sheet/TablePageBreakData.hpp>
 
@@ -583,13 +584,25 @@ bool ScTable::SetRowHidden(SCROW nStartRow, SCROW nEndRow, bool bHidden)
     else
         bChanged = mpHiddenRows->setFalse(nStartRow, nEndRow);
 
-    std::vector<SdrObject*> aRowDrawObjects;
+    // Cell anchored objects might change visibility
     ScDrawLayer* pDrawLayer = rDocument.GetDrawLayer();
-    if (pDrawLayer) {
+    if (pDrawLayer)
+    {
+        std::vector<SdrObject*> aRowDrawObjects;
         aRowDrawObjects = pDrawLayer->GetObjectsAnchoredToRows(GetTab(), nStartRow, nEndRow);
         for (auto aObj : aRowDrawObjects)
         {
-            aObj->SetVisible(!bHidden);
+            ScDrawObjData* pData = ScDrawLayer::GetObjData(aObj);
+            if (pData)
+            {
+                if (bHidden)
+                    aObj->SetVisible(false);
+                else if (!GetDoc().ColHidden(pData->maStart.Col(), pData->maStart.Tab()))
+                {
+                    // Only change visibility if object is not hidden by a hidden col
+                    aObj->SetVisible(true);
+                }
+            }
         }
     }
 
@@ -621,6 +634,28 @@ void ScTable::SetColHidden(SCCOL nStartCol, SCCOL nEndCol, bool bHidden)
     else
         bChanged = mpHiddenCols->setFalse(nStartCol, nEndCol);
 
+    // Cell anchored objects might change visibility
+    ScDrawLayer* pDrawLayer = rDocument.GetDrawLayer();
+    if (pDrawLayer)
+    {
+        std::vector<SdrObject*> aColDrawObjects;
+        aColDrawObjects = pDrawLayer->GetObjectsAnchoredToCols(GetTab(), nStartCol, nEndCol);
+        for (auto aObj : aColDrawObjects)
+        {
+            ScDrawObjData* pData = ScDrawLayer::GetObjData(aObj);
+            if (pData)
+            {
+                if (bHidden)
+                    aObj->SetVisible(false);
+                else if (!GetDoc().RowHidden(pData->maStart.Row(), pData->maStart.Tab()))
+                {
+                    // Only change visibility if object is not hidden by a hidden row
+                    aObj->SetVisible(true);
+                }
+            }
+        }
+    }
+
     if (bChanged)
         SetStreamValid(false);
 }
diff --git a/sc/source/filter/xml/xmlexprt.cxx b/sc/source/filter/xml/xmlexprt.cxx
index 0d748fcc4071..636fa9dcc75c 100644
--- a/sc/source/filter/xml/xmlexprt.cxx
+++ b/sc/source/filter/xml/xmlexprt.cxx
@@ -3473,8 +3473,10 @@ void ScXMLExport::WriteShapes(const ScMyCell& rMyCell)
         return;
 
     awt::Point aPoint;
+    // Hiding row or col does not change the shape geometry.
     tools::Rectangle aRect = pDoc->GetMMRect(rMyCell.maCellAddress.Col(), rMyCell.maCellAddress.Row(),
-        rMyCell.maCellAddress.Col(), rMyCell.maCellAddress.Row(), rMyCell.maCellAddress.Tab());
+        rMyCell.maCellAddress.Col(), rMyCell.maCellAddress.Row(), rMyCell.maCellAddress.Tab(),
+        false /*bHiddenAsZero*/);
     bool bNegativePage = pDoc->IsNegativePage(rMyCell.maCellAddress.Tab());
     if (bNegativePage)
         aPoint.X = aRect.Right();
diff --git a/sc/source/filter/xml/xmlimprt.cxx b/sc/source/filter/xml/xmlimprt.cxx
index f692d7d04568..e88fe74dd9f2 100644
--- a/sc/source/filter/xml/xmlimprt.cxx
+++ b/sc/source/filter/xml/xmlimprt.cxx
@@ -38,6 +38,7 @@
 #include <xmloff/xmltoken.hxx>
 #include <xmloff/xmlerror.hxx>
 #include <xmloff/ProgressBarHelper.hxx>
+#include <svx/svdpage.hxx>
 
 #include <svl/languageoptions.hxx>
 #include <editeng/editstat.hxx>
@@ -53,6 +54,7 @@
 #include "xmlbodyi.hxx"
 #include "xmlstyli.hxx"
 #include <ViewSettingsSequenceDefines.hxx>
+#include <userdat.hxx>
 
 #include <compiler.hxx>
 
@@ -1657,6 +1659,36 @@ void SAL_CALL ScXMLImport::endDocument()
             }
         }
 
+        // Initialize and set position and size of objects
+        if (pDoc && pDoc->GetDrawLayer())
+        {
+            ScDrawLayer* pDrawLayer = pDoc->GetDrawLayer();
+            SCTAB nTabCount = pDoc->GetTableCount();
+            for (SCTAB nTab = 0; nTab < nTabCount; ++nTab)
+            {
+                const SdrPage* pPage = pDrawLayer->GetPage(nTab);
+                if (!pPage)
+                    continue;
+                bool bNegativePage = pDoc->IsNegativePage(nTab);
+                const size_t nCount = pPage->GetObjCount();
+                for (size_t i = 0; i < nCount; ++i)
+                {
+                    SdrObject* pObj = pPage->GetObj(i);
+                    ScDrawObjData* pData
+                        = ScDrawLayer::GetObjDataTab(pObj, nTab);
+                    // Existance of pData means, that it is a cell anchored object
+                    if (pData)
+                    {
+                        // Finish and correct import based on full size (no hidden row/col) and LTR
+                        pDrawLayer->InitializeCellAnchoredObj(pObj, *pData);
+                        // Adapt object to hidden row/col and RTL
+                        pDrawLayer->RecalcPos(pObj, *pData, bNegativePage,
+                                              true /*bUpdateNoteCaptionPos*/);
+                    }
+                }
+            }
+        }
+
         aTables.FixupOLEs();
     }
     if (GetModel().is())
diff --git a/sc/source/filter/xml/xmlimprt.hxx b/sc/source/filter/xml/xmlimprt.hxx
index 0a5d2e7f815d..be25dc026062 100644
--- a/sc/source/filter/xml/xmlimprt.hxx
+++ b/sc/source/filter/xml/xmlimprt.hxx
@@ -49,10 +49,12 @@ class XMLNumberFormatAttributesExportHelper;
 class ScEditEngineDefaulter;
 class ScDocumentImport;
 class ScMyImpDetectiveOpArray;
+class SdrPage;
 
 namespace sc {
 struct ImportPostProcessData;
 struct PivotTableSources;
+class ScDrawObjData;
 }
 
 
diff --git a/test/source/sheet/xsheetannotationshapesupplier.cxx b/test/source/sheet/xsheetannotationshapesupplier.cxx
index 9b2cd6d773ee..dac446b72b52 100644
--- a/test/source/sheet/xsheetannotationshapesupplier.cxx
+++ b/test/source/sheet/xsheetannotationshapesupplier.cxx
@@ -28,7 +28,7 @@ void XSheetAnnotationShapeSupplier::testGetAnnotationShape()
     CPPUNIT_ASSERT_EQUAL_MESSAGE("getAnnotationShape() wrong X position",
                                  sal_Int32(7373), xShape->getPosition().X);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("getAnnotationShape() wrong Y position",
-                                 sal_Int32(426), xShape->getPosition().Y);
+                                 sal_Int32(451), xShape->getPosition().Y);
 
     CPPUNIT_ASSERT_EQUAL_MESSAGE("getAnnotationShape() wrong width",
                                  sal_Int32(11275), xShape->getSize().Width);


More information about the Libreoffice-commits mailing list