tdf#124594 DOCX filter: don't extend margins from effects for rotated shapes

Regression from commit a5a836d8c43dc9cebbbf8af39bf0142de603a7c6 (DOCX
filter: effect extent should be part of the margin, 2014-12-04), the
problem was that extending margins as-is based on the effect extent
values only work correctly in case of non-rotated shapes.

For example, with 90 degree clockwise rotation the top effect extent
should extend the right margin, etc. Fix the bug by limiting this
extension to the non-rotated scenario.

Test the behavior at a layout level, so in case later the effect extent
feature is implemented, it won't be necessary to adjust the test.

Change-Id: I97271bbb7c079951980b436cb8d8e5e54eeead55
Reviewed-on: https://gerrit.libreoffice.org/71878
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

4 files changed, 55 insertions, 13 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/tdf124594.docx b/sw/qa/extras/ooxmlexport/data/tdf124594.docx
new file mode 100644
index 000000000000..27abe1841982
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf124594.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
index aae0813900ed..ff2030df8b5f 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
@@ -229,6 +229,16 @@ DECLARE_OOXMLEXPORT_TEST(testTdf119201, "tdf119201.docx")
     CPPUNIT_ASSERT_MESSAGE("Third shape should be printable.", getProperty<bool>(xShape, "Printable"));
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf124594, "tdf124594.docx")
+{
+    xmlDocPtr pDump = parseLayoutDump();
+    // Without the accompanying fix in place, this test would have failed, as the portion text was
+    // only "Er horte leise Schritte hinter", which means the 1st line of the 2nd paragraph was
+    // split into two by a Special portion, i.e. the top margin of the shape was too large.
+    assertXPath(pDump, "/root/page/body/txt[2]/Text[1]", "Portion",
+                "Er horte leise Schritte hinter sich. Das bedeutete nichts Gutes. Wer wurde ihm ");
+}
+
 DECLARE_OOXMLEXPORT_TEST(testTextInput, "textinput.odt")
 {
     xmlDocPtr pXmlDoc = parseExport("word/document.xml");
diff --git a/sw/source/filter/ww8/docxsdrexport.cxx b/sw/source/filter/ww8/docxsdrexport.cxx
index f04d6e268de7..84943a518cac 100644
--- a/sw/source/filter/ww8/docxsdrexport.cxx
+++ b/sw/source/filter/ww8/docxsdrexport.cxx
@@ -454,6 +454,7 @@ void DocxSdrExport::startDMLAnchorInline(const SwFrameFormat* pFrameFormat, cons
         awt::Point aPos(pFrameFormat->GetHoriOrient().GetPos(),
                         pFrameFormat->GetVertOrient().GetPos());
         const SdrObject* pObj = pFrameFormat->FindRealSdrObject();
+        long nRotation = 0;
         if (pObj != nullptr)
         {
             // SdrObjects know their layer, consider that instead of the frame format.
@@ -464,21 +465,28 @@ void DocxSdrExport::startDMLAnchorInline(const SwFrameFormat* pFrameFormat, cons
                                     ->getIDocumentDrawModelAccess()
                                     .GetInvisibleHellId();
 
-            lclMovePositionWithRotation(aPos, rSize, pObj->GetRotateAngle());
+            nRotation = pObj->GetRotateAngle();
+            lclMovePositionWithRotation(aPos, rSize, nRotation);
         }
         attrList->add(XML_behindDoc, bOpaque ? "0" : "1");
+        // Extend distance with the effect extent if the shape is not rotated, which is the opposite
+        // of the mapping done at import time.
         // The type of dist* attributes is unsigned, so make sure no negative value is written.
-        sal_Int64 nDistT
-            = std::max(static_cast<sal_Int64>(0), TwipsToEMU(aULSpaceItem.GetUpper()) - nTopExt);
+        sal_Int64 nTopExtDist = nRotation ? 0 : nTopExt;
+        sal_Int64 nDistT = std::max(static_cast<sal_Int64>(0),
+                                    TwipsToEMU(aULSpaceItem.GetUpper()) - nTopExtDist);
         attrList->add(XML_distT, OString::number(nDistT).getStr());
-        sal_Int64 nDistB
-            = std::max(static_cast<sal_Int64>(0), TwipsToEMU(aULSpaceItem.GetLower()) - nBottomExt);
+        sal_Int64 nBottomExtDist = nRotation ? 0 : nBottomExt;
+        sal_Int64 nDistB = std::max(static_cast<sal_Int64>(0),
+                                    TwipsToEMU(aULSpaceItem.GetLower()) - nBottomExtDist);
         attrList->add(XML_distB, OString::number(nDistB).getStr());
-        sal_Int64 nDistL
-            = std::max(static_cast<sal_Int64>(0), TwipsToEMU(aLRSpaceItem.GetLeft()) - nLeftExt);
+        sal_Int64 nLeftExtDist = nRotation ? 0 : nLeftExt;
+        sal_Int64 nDistL = std::max(static_cast<sal_Int64>(0),
+                                    TwipsToEMU(aLRSpaceItem.GetLeft()) - nLeftExtDist);
         attrList->add(XML_distL, OString::number(nDistL).getStr());
-        sal_Int64 nDistR
-            = std::max(static_cast<sal_Int64>(0), TwipsToEMU(aLRSpaceItem.GetRight()) - nRightExt);
+        sal_Int64 nRightExtDist = nRotation ? 0 : nRightExt;
+        sal_Int64 nDistR = std::max(static_cast<sal_Int64>(0),
+                                    TwipsToEMU(aLRSpaceItem.GetRight()) - nRightExtDist);
         attrList->add(XML_distR, OString::number(nDistR).getStr());
         attrList->add(XML_simplePos, "0");
         attrList->add(XML_locked, "0");
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 6c04c10dacbb..d9458ba468c2 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -532,19 +532,15 @@ void GraphicImport::lcl_attribute(Id nName, Value& rValue)
         break;
         case NS_ooxml::LN_CT_EffectExtent_l:
             m_pImpl->m_oEffectExtentLeft = nIntValue;
-            m_pImpl->nLeftMargin += oox::drawingml::convertEmuToHmm(nIntValue);
             break;
         case NS_ooxml::LN_CT_EffectExtent_t:
             m_pImpl->m_oEffectExtentTop = nIntValue;
-            m_pImpl->nTopMargin += oox::drawingml::convertEmuToHmm(nIntValue);
             break;
         case NS_ooxml::LN_CT_EffectExtent_r:
             m_pImpl->m_oEffectExtentRight = nIntValue;
-            m_pImpl->nRightMargin += oox::drawingml::convertEmuToHmm(nIntValue);
             break;
         case NS_ooxml::LN_CT_EffectExtent_b:
             m_pImpl->m_oEffectExtentBottom = nIntValue;
-            m_pImpl->nBottomMargin += oox::drawingml::convertEmuToHmm(nIntValue);
             break;
         case NS_ooxml::LN_CT_NonVisualDrawingProps_id:// 90650;
             //id of the object - ignored
@@ -770,7 +766,35 @@ void GraphicImport::lcl_attribute(Id nName, Value& rValue)
                         }
                         m_xShape->setSize(aSize);
                         if (bKeepRotation)
+                        {
                             xShapeProps->setPropertyValue("RotateAngle", uno::makeAny(nRotation));
+                            if (nRotation == 0)
+                            {
+                                // Include effect extent in the margin to bring Writer layout closer
+                                // to Word. But do this for non-rotated shapes only, where effect
+                                // extents map to increased margins as-is.
+                                if (m_pImpl->m_oEffectExtentLeft)
+                                {
+                                    m_pImpl->nLeftMargin += oox::drawingml::convertEmuToHmm(
+                                        *m_pImpl->m_oEffectExtentLeft);
+                                }
+                                if (m_pImpl->m_oEffectExtentTop)
+                                {
+                                    m_pImpl->nTopMargin += oox::drawingml::convertEmuToHmm(
+                                        *m_pImpl->m_oEffectExtentTop);
+                                }
+                                if (m_pImpl->m_oEffectExtentRight)
+                                {
+                                    m_pImpl->nRightMargin += oox::drawingml::convertEmuToHmm(
+                                        *m_pImpl->m_oEffectExtentRight);
+                                }
+                                if (m_pImpl->m_oEffectExtentBottom)
+                                {
+                                    m_pImpl->nBottomMargin += oox::drawingml::convertEmuToHmm(
+                                        *m_pImpl->m_oEffectExtentBottom);
+                                }
+                            }
+                        }
 
                         m_pImpl->bIsGraphic = true;