tdf#154821 improve shape export with hidden row/col

XML needs shape geometry so as if no rows/cols are hidden. This had
been calculated by using the shape start and end cell. But that did
not work in some cases. Now the snap rectangle of the shape is used.

During testing I noticed, that the fix in tdf#154005 does not work
correctly. The fix had provided only a workaround for controls. It did
not address the root cause that a shape was size protected but has an
anchor 'To Cell (resize with cell)' at the same time. Such combination
is no longer needed as we have 'To Cell' anchor now, but shapes using
it still exist. The current solution catches such cases now in general.

As large parts of the method are changed anyway, I have put existance
tests at the beginning. That allows shorter conditions and flattens
if-constructions.

Change-Id: I6bd1e15dbdafc43e309a6e12c1c5e3218bb12675
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151717
Tested-by: Jenkins
Reviewed-by: Regina Henschel <rb.henschel@t-online.de>

4 files changed, 173 insertions, 127 deletions

diff --git a/sc/qa/unit/data/ods/tdf154821_shape_in_group.ods b/sc/qa/unit/data/ods/tdf154821_shape_in_group.ods
new file mode 100644
index 000000000000..905a64418cf6
--- /dev/null
+++ b/sc/qa/unit/data/ods/tdf154821_shape_in_group.ods
diff --git a/sc/qa/unit/scshapetest.cxx b/sc/qa/unit/scshapetest.cxx
index 0e4f1b555f99..21406c52cc7d 100644
--- a/sc/qa/unit/scshapetest.cxx
+++ b/sc/qa/unit/scshapetest.cxx
@@ -1169,6 +1169,52 @@ CPPUNIT_TEST_FIXTURE(ScShapeTest, testTdf125938_anchor_after_copy_paste)
     CPPUNIT_ASSERT_EQUAL(aExpectedAddress, (*pObjData).maStart);
 }
 
+CPPUNIT_TEST_FIXTURE(ScShapeTest, testTdf154821_shape_in_group)
+{
+    // The document contains a shape in A7, a group spanning rows 2 to 4 and a second group spanning
+    // rows 6 to 10. Error was, that when the document was saved with collapsed groups, the shape
+    // lost its position.
+    createScDoc("ods/tdf154821_shape_in_group.ods");
+
+    // Get snap rectangle before collapse and save
+    ScDocument* pDoc = getScDoc();
+    SdrObject* pObj = lcl_getSdrObjectWithAssert(*pDoc, 0);
+    tools::Rectangle aRectOrig = pObj->GetSnapRect();
+
+    // Collapse the lower group
+    ScTabViewShell* pViewShell = getViewShell();
+    pViewShell->GetViewData().SetCurX(0);
+    pViewShell->GetViewData().SetCurY(5);
+    pViewShell->GetViewData().GetDispatcher().Execute(SID_OUTLINE_HIDE);
+    // Collapse the upper group
+    pViewShell->GetViewData().SetCurX(0);
+    pViewShell->GetViewData().SetCurY(1);
+    pViewShell->GetViewData().GetDispatcher().Execute(SID_OUTLINE_HIDE);
+
+    // Save and reload
+    // FIXME: validation fails with
+    // Error: unexpected attribute "drawooo:display"
+    skipValidation();
+    saveAndReload("calc8");
+
+    // Expand the lower group
+    pViewShell = getViewShell();
+    pViewShell->GetViewData().SetCurX(0);
+    pViewShell->GetViewData().SetCurY(5);
+    pViewShell->GetViewData().GetDispatcher().Execute(SID_OUTLINE_SHOW);
+    // Expande the upper group
+    pViewShell = getViewShell();
+    pViewShell->GetViewData().SetCurX(0);
+    pViewShell->GetViewData().SetCurY(1);
+    pViewShell->GetViewData().GetDispatcher().Execute(SID_OUTLINE_SHOW);
+
+    // Verify shape position is not changed besides rounding errors from twips<->mm
+    pDoc = getScDoc();
+    pObj = lcl_getSdrObjectWithAssert(*pDoc, 0);
+    tools::Rectangle aRectReload = pObj->GetSnapRect();
+    CPPUNIT_ASSERT_RECTANGLE_EQUAL_WITH_TOLERANCE(aRectOrig, aRectReload, 1);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/drwlayer.cxx b/sc/source/core/data/drwlayer.cxx
index 134c7258ed4a..8e989012b4e8 100644
--- a/sc/source/core/data/drwlayer.cxx
+++ b/sc/source/core/data/drwlayer.cxx
@@ -776,15 +776,6 @@ void lcl_SetLogicRectFromAnchor(SdrObject* pObj, const ScDrawObjData& rAnchor, c
     if (!pObj || !pDoc || !rAnchor.maEnd.IsValid() || !rAnchor.maStart.IsValid())
         return;
 
-    SCROW nHiddenRows = 0;
-    SCCOL nHiddenCols = 0;
-    // tdf#154005: Handle hidden row/col: remove hidden row/cols size from the ScDrawObjData shape size in case of forms
-    if (pObj->GetObjIdentifier() == SdrObjKind::UNO && pObj->GetObjInventor() == SdrInventor::FmForm)
-    {
-        nHiddenRows = pDoc->CountHiddenRows(rAnchor.maStart.Row(), rAnchor.maEnd.Row(), rAnchor.maStart.Tab());
-        nHiddenCols = pDoc->CountHiddenCols(rAnchor.maStart.Col(), rAnchor.maEnd.Col(), rAnchor.maStart.Tab());
-    }
-
     // 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
@@ -808,8 +799,9 @@ void lcl_SetLogicRectFromAnchor(SdrObject* pObj, const ScDrawObjData& rAnchor, c
     aStartPoint.AdjustY(rAnchor.maStartOffset.getY());
 
     const tools::Rectangle aEndCellRect(
-        pDoc->GetMMRect(rAnchor.maEnd.Col() - nHiddenCols, rAnchor.maEnd.Row() - nHiddenRows, rAnchor.maEnd.Col() - nHiddenCols,
-                        rAnchor.maEnd.Row() - nHiddenRows, rAnchor.maEnd.Tab(), false /*bHiddenAsZero*/));
+        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());
@@ -1057,6 +1049,14 @@ void ScDrawLayer::InitializeCellAnchoredObj(SdrObject* pObj, ScDrawObjData& rDat
             GetCellAnchorFromPosition(aObjRect, rNoRotatedAnchor, *pDoc, rData.maStart.Tab(),
                                       false /*bHiddenAsZero*/);
         }
+        else if (pObj->IsResizeProtect())
+        {
+            // tdf#154005: This is a workaround for documents created with LO 6 and older.
+            rNoRotatedAnchor.mbResizeWithCell = false;
+            rData.mbResizeWithCell = false;
+            UpdateCellAnchorFromPositionEnd(*pObj, rNoRotatedAnchor, *pDoc, nTab1,
+                                            true /*bUseLogicRect*/);
+        }
         else
         {
             // In case there are hidden rows or cols, versions 7.0 and earlier have written width and
@@ -1067,7 +1067,7 @@ void ScDrawLayer::InitializeCellAnchoredObj(SdrObject* pObj, ScDrawObjData& rDat
             lcl_SetLogicRectFromAnchor(pObj, rNoRotatedAnchor, pDoc);
         }
     }
-    else // aAnchorType == SCA_CELL, other types will not occur here.
+    else // aAnchorType == SCA_CELL
     {
         // XML has no end cell address in this case. We generate it from position.
         UpdateCellAnchorFromPositionEnd(*pObj, rNoRotatedAnchor, *pDoc, nTab1,
diff --git a/sc/source/filter/xml/xmlexprt.cxx b/sc/source/filter/xml/xmlexprt.cxx
index a552de5f0ddf..7c84ac43c4fc 100644
--- a/sc/source/filter/xml/xmlexprt.cxx
+++ b/sc/source/filter/xml/xmlexprt.cxx
@@ -120,6 +120,7 @@
 #include <svx/svdoashp.hxx>
 #include <svx/svdobj.hxx>
 #include <svx/svdocapt.hxx>
+#include <svx/svdomeas.hxx>
 #include <vcl/svapp.hxx>
 
 #include <comphelper/processfactory.hxx>
@@ -3484,135 +3485,134 @@ void ScXMLExport::WriteShapes(const ScMyCell& rMyCell)
 
     for (const auto& rShape : rMyCell.aShapeList)
     {
-        if (rShape.xShape.is())
+        // Skip the shape if requirements are not met. The tests should not fail, but allow
+        // shorter conditions in main part below.
+        if (!rShape.xShape.is())
+            continue;
+        SdrObject* pObj = SdrObject::getSdrObjectFromXShape(rShape.xShape);
+        if (!pObj)
+            continue;
+        ScDrawObjData* pObjData = ScDrawLayer::GetObjData(pObj);
+        if (!pObjData)
+            continue;
+        ScAddress aSnapStartAddress = pObjData->maStart;
+        if (!aSnapStartAddress.IsValid())
+            continue;
+
+        // The current object geometry is based on bHiddenAsZero=true, but ODF file format
+        // needs it as if there were no hidden rows or columns. We termine a fictive snap
+        // rectangle from the anchor as if all column/rows are shown. Then we move and resize
+        // (in case of "resize with cell") the object to meet this snap rectangle. We need to
+        // manipulate the object itself, because the used methods in xmloff do not evaluate the
+        // ObjData. This manipulation is only done temporarily for export. Thus we stash the geometry
+        // and restore it when export is done and we use NbcFoo methods.
+        bool bNeedsRestore = false;
+        std::unique_ptr<SdrObjGeoData> pGeoData = pObj->GetGeoData();
+
+        // Determine top point of fictive snap rectangle ('Full' rectangle).
+        SCTAB aTab(aSnapStartAddress.Tab());
+        SCCOL aCol(aSnapStartAddress.Col());
+        SCROW aRow(aSnapStartAddress.Row());
+        tools::Rectangle aFullStartCellRect
+            = pDoc->GetMMRect(aCol, aRow, aCol, aRow, aTab, false /*bHiddenAsZero*/);
+        // The reference corner for the offset is top-left in case of LTR and top-right for RTL.
+        Point aFullTopPoint;
+        if (bNegativePage)
+            aFullTopPoint.setX(aFullStartCellRect.Right() - pObjData->maStartOffset.X());
+        else
+            aFullTopPoint.setX(aFullStartCellRect.Left() + pObjData->maStartOffset.X());
+        aFullTopPoint.setY(aFullStartCellRect.Top() + pObjData->maStartOffset.Y());
+
+        // Compare actual top point and full top point and move object accordingly.
+        tools::Rectangle aOrigSnapRect(pObj->GetSnapRect());
+        Point aActualTopPoint = bNegativePage ? aOrigSnapRect.TopRight() : aOrigSnapRect.TopLeft();
+        if (aFullTopPoint != aActualTopPoint)
         {
-            // The current object geometry is based on bHiddenAsZero=true, but ODF file format
-            // needs it as if there were no hidden rows or columns. We manipulate the geometry
-            // accordingly for writing xml markup and restore geometry later.
-            bool bNeedsRestore = false;
-            SdrObject* pObj = SdrObject::getSdrObjectFromXShape(rShape.xShape);
-            // Remember original geometry
-            std::unique_ptr<SdrObjGeoData> pGeoData;
-            if (pObj)
-                pGeoData = pObj->GetGeoData();
-
-            // Hiding row or column affects the shape based on its snap rect. So we need start and
-            // end cell address of snap rect. In case of a transformed shape, it is not in rMyCell.
-            ScAddress aSnapStartAddress = rMyCell.maCellAddress;
-            ScDrawObjData* pObjData = nullptr;
-            if (pObj)
-            {
-                pObjData = ScDrawLayer::GetObjData(pObj);
-                if (pObjData)
-                    aSnapStartAddress = pObjData->maStart;
-            }
+            bNeedsRestore = true;
+            Point aMoveBy = aFullTopPoint - aActualTopPoint;
+            pObj->NbcMove(Size(aMoveBy.X(), aMoveBy.Y()));
+        }
 
-            // In case rows or columns are hidden above or before the snap rect, move the shape to the
-            // position it would have, if these rows and columns are visible.
-            tools::Rectangle aRectFull = pDoc->GetMMRect(
-                aSnapStartAddress.Col(), aSnapStartAddress.Row(), aSnapStartAddress.Col(),
-                aSnapStartAddress.Row(), aSnapStartAddress.Tab(), false /*bHiddenAsZero*/);
-            tools::Rectangle aRectReduced = pDoc->GetMMRect(
-                aSnapStartAddress.Col(), aSnapStartAddress.Row(), aSnapStartAddress.Col(),
-                aSnapStartAddress.Row(), aSnapStartAddress.Tab(), true /*bHiddenAsZero*/);
-            const tools::Long nLeftDiff(aRectFull.Left() - aRectReduced.Left());
-            const tools::Long nTopDiff(aRectFull.Top() - aRectReduced.Top());
-            if (pObj && (abs(nLeftDiff) > 1 || abs(nTopDiff) > 1))
-            {
-                bNeedsRestore = true;
-                pObj->NbcMove(Size(nLeftDiff, nTopDiff));
-            }
+        ScAddress aSnapEndAddress = pObjData->maEnd;
+        // tdf#154005: We treat the combination of "To cell (resize with cell)" with 'size protected'
+        // as being "To cell".
+        if (pObjData->mbResizeWithCell && aSnapEndAddress.IsValid() && !pObj->IsResizeProtect())
+        {
+            // Object is anchored "To cell (resize with cell)". Compare size of actual snap rectangle
+            // and fictive full one. Resize object accordingly.
+            tools::Rectangle aActualSnapRect(pObj->GetSnapRect());
+            Point aSnapEndOffset(pObjData->maEndOffset);
+            aCol = aSnapEndAddress.Col();
+            aRow = aSnapEndAddress.Row();
+            tools::Rectangle aFullEndCellRect
+                = pDoc->GetMMRect(aCol, aRow, aCol, aRow, aTab, false /*bHiddenAsZero*/);
+            Point aFullBottomPoint;
+            if (bNegativePage)
+                aFullBottomPoint.setX(aFullEndCellRect.Right() - aSnapEndOffset.X());
+            else
+                aFullBottomPoint.setX(aFullEndCellRect.Left() + aSnapEndOffset.X());
+            aFullBottomPoint.setY(aFullEndCellRect.Top() + aSnapEndOffset.Y());
+            tools::Rectangle aFullSnapRect(aFullTopPoint, aFullBottomPoint);
+            aFullSnapRect.Normalize();
 
-            // tdf#137033 In case the shape is anchored "To Cell (resize with cell)" hiding rows or
-            // columns inside the snap rect has not only changed size of the shape but rotate and shear
-            // angle too. We resize the shape to full size. That will recover the original angles too.
-            if (rShape.bResizeWithCell && pObjData) // implies pObj & aSnapStartAddress = pObjData->maStart
+            if (aFullSnapRect != aActualSnapRect)
             {
-                // Get original size from anchor
-                const Point aSnapStartOffset = pObjData->maStartOffset;
-                // In case of 'resize with cell' maEnd and maEndOffset should be valid.
-                const ScAddress aSnapEndAddress(pObjData->maEnd);
-                const Point aSnapEndOffset = pObjData->maEndOffset;
-                const tools::Rectangle aStartCellRect = pDoc->GetMMRect(
-                    aSnapStartAddress.Col(), aSnapStartAddress.Row(), aSnapStartAddress.Col(),
-                    aSnapStartAddress.Row(), aSnapStartAddress.Tab(), false /*bHiddenAsZero*/);
-                const tools::Rectangle aEndCellRect = pDoc->GetMMRect(
-                    aSnapEndAddress.Col(), aSnapEndAddress.Row(), aSnapEndAddress.Col(),
-                    aSnapEndAddress.Row(), aSnapEndAddress.Tab(), false /*bHiddenAsZero*/);
-                if (bNegativePage)
-                {
-                    aRectFull.SetLeft(aEndCellRect.Right() - aSnapEndOffset.X());
-                    aRectFull.SetRight(aStartCellRect.Right() - aSnapStartOffset.X());
-                }
-                else
-                {
-                    aRectFull.SetLeft(aStartCellRect.Left() + aSnapStartOffset.X());
-                    aRectFull.SetRight(aEndCellRect.Left() + aSnapEndOffset.X());
-                }
-                aRectFull.SetTop(aStartCellRect.Top() + aSnapStartOffset.Y());
-                aRectFull.SetBottom(aEndCellRect.Top() + aSnapEndOffset.Y());
-                aRectReduced = pObjData->getShapeRect();
-                if(abs(aRectFull.getOpenWidth() - aRectReduced.getOpenWidth()) > 1
-                   || abs(aRectFull.getOpenHeight() - aRectReduced.getOpenHeight()) > 1)
-                {
-                    bNeedsRestore = true;
-                    Fraction aScaleWidth(aRectFull.getOpenWidth(), aRectReduced.getOpenWidth());
-                    if (!aScaleWidth.IsValid())
-                        aScaleWidth = Fraction(1.0);
-                    Fraction aScaleHeight(aRectFull.getOpenHeight(), aRectReduced.getOpenHeight());
-                    if (!aScaleHeight.IsValid())
-                        aScaleHeight = Fraction(1.0);
-                    pObj->NbcResize(pObj->GetRelativePos(), aScaleWidth, aScaleHeight);
-                }
+                bNeedsRestore = true;
+                Fraction aScaleWidth(aFullSnapRect.getOpenWidth(), aActualSnapRect.getOpenWidth());
+                if (!aScaleWidth.IsValid())
+                    aScaleWidth = Fraction(1, 1);
+                Fraction aScaleHeight(aFullSnapRect.getOpenHeight(),
+                                      aActualSnapRect.getOpenHeight());
+                if (!aScaleHeight.IsValid())
+                    aScaleHeight = Fraction(1, 1);
+                pObj->NbcResize(aFullTopPoint, aScaleWidth, aScaleHeight);
             }
+        }
 
-            // We only write the end address if we want the shape to resize with the cell
-            if ( rShape.bResizeWithCell &&
-                rShape.xShape->getShapeType() != "com.sun.star.drawing.CaptionShape" )
-            {
-                OUString sEndAddress;
-                ScRangeStringConverter::GetStringFromAddress(sEndAddress, rShape.aEndAddress, pDoc, FormulaGrammar::CONV_OOO);
-                AddAttribute(XML_NAMESPACE_TABLE, XML_END_CELL_ADDRESS, sEndAddress);
-                OUStringBuffer sBuffer;
-                GetMM100UnitConverter().convertMeasureToXML(
-                        sBuffer, rShape.nEndX);
-                AddAttribute(XML_NAMESPACE_TABLE, XML_END_X, sBuffer.makeStringAndClear());
-                GetMM100UnitConverter().convertMeasureToXML(
-                        sBuffer, rShape.nEndY);
-                AddAttribute(XML_NAMESPACE_TABLE, XML_END_Y, sBuffer.makeStringAndClear());
-            }
+        // The existance of an end address is equivalent to anchor mode "To Cell (resize with cell)".
+        // XML needs end address in regard of untransformed shape. Those are contained in rShape but
+        // could be received from NonRotatedObjData as well.
+        // tdf#154005: We treat the combination of "To Cell (resize with cell)" anchor with 'size
+        // protected' property as being "To cell" anchor.
+        if (pObjData->mbResizeWithCell && !pObj->IsResizeProtect())
+        {
+            OUString sEndAddress;
+            ScRangeStringConverter::GetStringFromAddress(sEndAddress, rShape.aEndAddress, pDoc,
+                                                         FormulaGrammar::CONV_OOO);
+            AddAttribute(XML_NAMESPACE_TABLE, XML_END_CELL_ADDRESS, sEndAddress);
+            OUStringBuffer sBuffer;
+            GetMM100UnitConverter().convertMeasureToXML(sBuffer, rShape.nEndX);
+            AddAttribute(XML_NAMESPACE_TABLE, XML_END_X, sBuffer.makeStringAndClear());
+            GetMM100UnitConverter().convertMeasureToXML(sBuffer, rShape.nEndY);
+            AddAttribute(XML_NAMESPACE_TABLE, XML_END_Y, sBuffer.makeStringAndClear());
+        }
 
-            // Correct above calculated reference point for some cases:
-            // a) For a RTL-sheet translate from matrix is not suitable, because the shape
-            // from xml (which is always LTR) is not mirrored to negative page but shifted.
-            // b) In case of horizontal mirrored, 'resize with cell' anchored custom shape, translate
-            // has wrong values. FixMe: Why is translate wrong?
-            // c) Measure lines do not use transformation matrix but use start and end point directly.
-            ScDrawObjData* pNRObjData = nullptr;
-            if (pObj && bNegativePage
-                && rShape.xShape->getShapeType() == "com.sun.star.drawing.MeasureShape")
-            {
-                // inverse of shift when import
-                tools::Rectangle aSnapRect = pObj->GetSnapRect();
-                aPoint.X = aSnapRect.Left() + aSnapRect.Right() - aPoint.X;
-            }
-            else if (pObj && (pNRObjData = ScDrawLayer::GetNonRotatedObjData(pObj))
-                     && ((rShape.bResizeWithCell && pObj->GetObjIdentifier() == SdrObjKind::CustomShape
-                          && static_cast<SdrObjCustomShape*>(pObj)->IsMirroredX())
-                         || bNegativePage))
+        // Correct above calculated reference point for these cases:
+        // a) For a RTL-sheet translate from matrix is not suitable, because the shape
+        // from xml (which is always LTR) is not mirrored to negative page but shifted.
+        // b) In case of horizontal mirrored, 'resize with cell' anchored custom shape, translate from
+        // matrix has wrong values. FixMe: Why is translate wrong?
+        if (bNegativePage
+            || (pObj->GetObjIdentifier() == SdrObjKind::CustomShape
+                && static_cast<SdrObjCustomShape*>(pObj)->IsMirroredX()
+                && pObjData->mbResizeWithCell))
+        {
+            // In these cases we set reference point so that the offset calculation in XML export
+            // (=  matrix translate - reference point) results in maStartOffset.
+            ScDrawObjData* pNRObjData = ScDrawLayer::GetNonRotatedObjData(pObj);
+            if (pNRObjData)
             {
-                //In these cases we set reference Point = matrix translate - startOffset.
                 awt::Point aMatrixTranslate = rShape.xShape->getPosition();
                 aPoint.X = aMatrixTranslate.X - pNRObjData->maStartOffset.X();
                 aPoint.Y = aMatrixTranslate.Y - pNRObjData->maStartOffset.Y();
             }
+        }
 
-            ExportShape(rShape.xShape, &aPoint);
+        ExportShape(rShape.xShape, &aPoint);
 
-            // Restore object geometry
-            if (bNeedsRestore && pObj && pGeoData)
-                pObj->SetGeoData(*pGeoData);
-        }
+        // Restore object geometry
+        if (bNeedsRestore && pGeoData)
+            pObj->SetGeoData(*pGeoData);
     }
 }