[Libreoffice-commits] core.git: 3 commits - include/oox oox/source sw/qa writerfilter/source

Jacobo Aragunde Pérez jaragunde at igalia.com
Fri May 16 05:12:01 PDT 2014


 include/oox/drawingml/effectproperties.hxx                      |   19 +
 include/oox/drawingml/effectpropertiescontext.hxx               |    3 
 include/oox/export/drawingml.hxx                                |    1 
 oox/source/drawingml/effectproperties.cxx                       |   71 +++---
 oox/source/drawingml/effectpropertiescontext.cxx                |   77 +++----
 oox/source/drawingml/shape.cxx                                  |   55 +++--
 oox/source/export/drawingml.cxx                                 |  102 +++++-----
 oox/source/export/shapes.cxx                                    |    4 
 sw/qa/extras/ooxmlexport/data/picture-effects-preservation.docx |binary
 sw/qa/extras/ooxmlexport/data/shape-effect-preservation.docx    |binary
 sw/qa/extras/ooxmlexport/ooxmlexport.cxx                        |    6 
 sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx                     |   82 ++++++++
 sw/qa/extras/ooxmlimport/ooxmlimport.cxx                        |    8 
 writerfilter/source/dmapper/GraphicImport.cxx                   |   13 +
 14 files changed, 284 insertions(+), 157 deletions(-)

New commits:
commit 73ad72e820ed3de346efa1914432a7f2c6264dde
Author: Jacobo Aragunde Pérez <jaragunde at igalia.com>
Date:   Fri May 16 11:38:18 2014 +0200

    ooxml: Preserve effects on pictures
    
    If a picture contains some 2D (glow, shadow...) or 3D effect
    (rotation, extrusion...), we prevent the importer from transforming
    it into a XTextContent so the XShape grab bag is not removed and
    the effects are preserved using the existing mechanisms. Added a unit
    test for this issue, and modified some existing unit tests to match
    the new behaviour.
    
    Change-Id: I3b87069ea208604383a592d34d0a4ceb6b0f9fc7

diff --git a/oox/source/export/shapes.cxx b/oox/source/export/shapes.cxx
index c9eaa6f..82d2b5e 100644
--- a/oox/source/export/shapes.cxx
+++ b/oox/source/export/shapes.cxx
@@ -530,6 +530,10 @@ void ShapeExport::WriteGraphicObjectShapePart( Reference< XShape > xShape, const
     WritePresetShape( "rect" );
     // graphic object can come with the frame (bnc#654525)
     WriteOutline( xShapeProps );
+
+    WriteShapeEffects( xShapeProps );
+    WriteShape3DEffects( xShapeProps );
+
     pFS->endElementNS( mnXmlNamespace, XML_spPr );
 
     pFS->endElementNS( mnXmlNamespace, XML_pic );
diff --git a/sw/qa/extras/ooxmlexport/data/picture-effects-preservation.docx b/sw/qa/extras/ooxmlexport/data/picture-effects-preservation.docx
new file mode 100644
index 0000000..d26def4
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/picture-effects-preservation.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
index 014c3b9..67ee861 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
@@ -1079,8 +1079,10 @@ DECLARE_OOXMLEXPORT_TEST(testFdo67737, "fdo67737.docx")
 DECLARE_OOXMLEXPORT_TEST(testTransparentShadow, "transparent-shadow.docx")
 {
     uno::Reference<drawing::XShape> xPicture = getShape(1);
-    table::ShadowFormat aShadow = getProperty<table::ShadowFormat>(xPicture, "ShadowFormat");
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0x7f808080), aShadow.Color);
+    sal_Int32 nShadowColor = getProperty<sal_Int32>(xPicture, "ShadowColor");
+    sal_Int16 nShadowTransparence = getProperty<sal_Int16>(xPicture, "ShadowTransparence");
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0x808080), nShadowColor);
+    CPPUNIT_ASSERT_EQUAL(sal_Int16(50), nShadowTransparence);
 }
 
 DECLARE_OOXMLEXPORT_TEST(testBnc834035, "bnc834035.odt")
diff --git a/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx b/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx
index 736546a..21eff8e 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx
@@ -1285,6 +1285,59 @@ DECLARE_OOXMLEXPORT_TEST(testShape3DEffectPreservation, "shape-3d-effect-preserv
             "prstMaterial", "legacyWireframe");
 }
 
+DECLARE_OOXMLEXPORT_TEST(testPictureEffectPreservation, "picture-effects-preservation.docx")
+{
+    xmlDocPtr pXmlDoc = parseExport("word/document.xml");
+    if (!pXmlDoc)
+       return;
+
+    // first picture: glow effect with theme color and transformations, 3d rotation and extrusion
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:glow",
+            "rad", "228600");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:glow/a:schemeClr",
+            "val", "accent1");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:glow/a:schemeClr/*",
+            2);
+
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:scene3d/a:camera",
+            "prst", "isometricRightUp");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:scene3d/a:lightRig",
+            "rig", "threePt");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:sp3d",
+            "extrusionH", "76200");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:sp3d/a:extrusionClr/a:srgbClr",
+            "val", "92d050");
+
+    // second picture: shadow and reflection effects
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:outerShdw",
+            "dir", "8100000");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:outerShdw/a:srgbClr",
+            "val", "000000");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:outerShdw/a:srgbClr/a:alpha",
+            "val", "40000");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:reflection",
+            "dir", "5400000");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:reflection/*",
+            0 ); // should not be present
+
+    // third picture: soft edge effect
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[3]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/pic:pic/pic:spPr/a:effectLst/a:softEdge",
+            "rad", "63500");
+}
+
 DECLARE_OOXMLEXPORT_TEST(fdo77719, "fdo77719.docx")
 {
     xmlDocPtr pXmlDoc = parseExport("word/document.xml");
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index 3ffe132..5c08d7a 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -828,9 +828,11 @@ DECLARE_OOXMLIMPORT_TEST(testShadow, "imgshadow.docx")
      */
     uno::Reference<beans::XPropertySet> xPropertySet(getShape(2), uno::UNO_QUERY);
 
-    table::ShadowFormat aShadow;
-    xPropertySet->getPropertyValue("ShadowFormat") >>= aShadow;
-    CPPUNIT_ASSERT(sal_Int32(aShadow.ShadowWidth) > 0);
+    bool bShadow = getProperty<bool>(xPropertySet, "Shadow");
+    CPPUNIT_ASSERT(bShadow);
+
+    sal_Int32 nShadowXDistance = getProperty<sal_Int32>(xPropertySet, "ShadowXDistance");
+    CPPUNIT_ASSERT(nShadowXDistance != 0);
 }
 
 DECLARE_OOXMLIMPORT_TEST(testN782061, "n782061.docx")
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 10c7029..59f27fc 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -643,6 +643,17 @@ void GraphicImport::lcl_attribute(Id nName, Value& rValue)
                         sal_Int32 nRotation = 0;
                         xShapeProps->getPropertyValue("RotateAngle") >>= nRotation;
 
+                        ::css::beans::PropertyValues aGrabBag;
+                        bool bContainsEffects = false;
+                        xShapeProps->getPropertyValue("InteropGrabBag") >>= aGrabBag;
+                        for( sal_Int32 i = 0; i < aGrabBag.getLength(); ++i )
+                        {
+                            // if the shape contains effects in the grab bag, we should not transform it
+                            // in a XTextContent so those effects can be preserved
+                            if( aGrabBag[i].Name == "EffectProperties" || aGrabBag[i].Name == "3DEffectProperties" )
+                                bContainsEffects = true;
+                        }
+
                         ::com::sun::star::beans::PropertyValues aMediaProperties( 1 );
                         aMediaProperties[0].Name = "URL";
                         aMediaProperties[0].Value <<= sUrl;
@@ -661,7 +672,7 @@ void GraphicImport::lcl_attribute(Id nName, Value& rValue)
                         xShapeProps->getPropertyValue("AdjustContrast") >>= m_pImpl->nContrast;
 
                         // fdo#70457: transform XShape into a SwXTextGraphicObject only if there's no rotation
-                        if ( nRotation == 0 )
+                        if ( nRotation == 0 && !bContainsEffects )
                             m_xGraphicObject = createGraphicObject( aMediaProperties );
 
                         bUseShape = !m_xGraphicObject.is( );
commit be415a0f9a65d44d1042b313141f49c617bedd93
Author: Jacobo Aragunde Pérez <jaragunde at igalia.com>
Date:   Fri May 16 09:56:58 2014 +0200

    ooxml: Preserve shape effects when there's more than one
    
    Transformed the preservation process of shape effects to be able to
    store more than one effect. For that we:
    
    * Created the Effect struct and added a vector member to the
      EffectProperties struct.
    * Changed the shadow effect to use the new Effect struct,
      EffectShadowProperties struct is preserved because the direction
      field still has some use but we should remove it.
    * Changed the structure of the grab bag to store more than one effect.
    * Modified an existing unit test to check shapes with several effects.
    
    Change-Id: I0dd908fa1d9578827c02ef6272fc9e2b914391be

diff --git a/include/oox/drawingml/effectproperties.hxx b/include/oox/drawingml/effectproperties.hxx
index 618c7b9..a25aa12 100644
--- a/include/oox/drawingml/effectproperties.hxx
+++ b/include/oox/drawingml/effectproperties.hxx
@@ -30,13 +30,23 @@ struct EffectShadowProperties
 
 
 
+struct Effect
+{
+    OUString msName;
+    std::map< OUString, css::uno::Any > maAttribs;
+    Color moColor;
+
+    css::beans::PropertyValue getEffect();
+};
+
+
+
 struct OOX_DLLPUBLIC EffectProperties
 {
     EffectShadowProperties maShadow;
 
-    /** Store unsupported effect type name and its attributes */
-    OptValue< OUString > msUnsupportedEffectName;
-    std::vector< css::beans::PropertyValue > maUnsupportedEffectAttribs;
+    /** Stores all effect properties, including those not supported by core yet */
+    std::vector< Effect* > maEffects;
 
     /** Overwrites all members that are explicitly set in rSourceProps. */
     void                assignUsed( const EffectProperties& rSourceProps );
@@ -45,9 +55,6 @@ struct OOX_DLLPUBLIC EffectProperties
     void                pushToPropMap(
                             PropertyMap& rPropMap,
                             const GraphicHelper& rGraphicHelper ) const;
-
-    void appendUnsupportedEffectAttrib( const OUString& aKey, const css::uno::Any& aValue );
-    css::beans::PropertyValue getUnsupportedEffect();
 };
 
 
diff --git a/include/oox/drawingml/effectpropertiescontext.hxx b/include/oox/drawingml/effectpropertiescontext.hxx
index f81396d..6106254 100644
--- a/include/oox/drawingml/effectpropertiescontext.hxx
+++ b/include/oox/drawingml/effectpropertiescontext.hxx
@@ -12,6 +12,7 @@
 
 #include <oox/core/contexthandler2.hxx>
 #include <oox/dllapi.h>
+#include <oox/drawingml/effectproperties.hxx>
 
 namespace oox { namespace drawingml {
 
@@ -33,7 +34,7 @@ protected:
     EffectProperties& mrEffectProperties;
 
 private:
-    void saveUnsupportedAttribs( const AttributeList& rAttribs );
+    void saveUnsupportedAttribs( Effect& rEffect, const AttributeList& rAttribs );
 };
 
 } }
diff --git a/include/oox/export/drawingml.hxx b/include/oox/export/drawingml.hxx
index 9153196..86e3d07 100644
--- a/include/oox/export/drawingml.hxx
+++ b/include/oox/export/drawingml.hxx
@@ -174,6 +174,7 @@ public:
     void WriteFill( ::com::sun::star::uno::Reference< ::com::sun::star::beans::XPropertySet > xPropSet );
     void WriteShapeStyle( ::com::sun::star::uno::Reference< ::com::sun::star::beans::XPropertySet > rXPropSet );
     void WriteShapeEffects( ::com::sun::star::uno::Reference< ::com::sun::star::beans::XPropertySet > rXPropSet );
+    void WriteShapeEffect( const OUString& sName, const css::uno::Sequence< css::beans::PropertyValue >& aEffectProps );
     void WriteShape3DEffects( ::com::sun::star::uno::Reference< ::com::sun::star::beans::XPropertySet > rXPropSet );
 
     static void ResetCounters();
diff --git a/oox/source/drawingml/effectproperties.cxx b/oox/source/drawingml/effectproperties.cxx
index be3b3d0..be4af0c 100644
--- a/oox/source/drawingml/effectproperties.cxx
+++ b/oox/source/drawingml/effectproperties.cxx
@@ -30,54 +30,57 @@ void EffectShadowProperties::assignUsed(const EffectShadowProperties& rSourcePro
 void EffectProperties::assignUsed( const EffectProperties& rSourceProps )
 {
     maShadow.assignUsed(rSourceProps.maShadow);
-    msUnsupportedEffectName.assignIfUsed( rSourceProps.msUnsupportedEffectName );
-    maUnsupportedEffectAttribs = rSourceProps.maUnsupportedEffectAttribs;
+    if( rSourceProps.maEffects.size() > 0 )
+        maEffects = rSourceProps.maEffects;
 }
 
 void EffectProperties::pushToPropMap( PropertyMap& rPropMap,
         const GraphicHelper& rGraphicHelper ) const
 {
-    if (maShadow.moShadowDist.has())
-    {
-        // Negative X or Y dist indicates left or up, respectively
-        double nAngle = (maShadow.moShadowDir.get(0) / PER_DEGREE) * F_PI180;
-        sal_Int32 nDist = convertEmuToHmm(maShadow.moShadowDist.get(0));
-        sal_Int32 nXDist = cos(nAngle) * nDist;
-        sal_Int32 nYDist = sin(nAngle) * nDist;
-
-        rPropMap.setProperty( PROP_Shadow, true );
-        rPropMap.setProperty( PROP_ShadowXDistance, nXDist);
-        rPropMap.setProperty( PROP_ShadowYDistance, nYDist);
-        rPropMap.setProperty( PROP_ShadowColor, maShadow.moShadowColor.getColor(rGraphicHelper, -1 ) );
-        rPropMap.setProperty( PROP_ShadowTransparence, maShadow.moShadowColor.getTransparency());
-    }
-}
-
-void EffectProperties::appendUnsupportedEffectAttrib( const OUString& aKey, const css::uno::Any& aValue )
-{
-    css::beans::PropertyValue aProperty;
-    aProperty.Name = aKey;
-    aProperty.Value = aValue;
-    maUnsupportedEffectAttribs.push_back(aProperty);
+    for( std::vector< Effect* >::const_iterator it = maEffects.begin(); it != maEffects.end(); ++it )
+        if( (*it)->msName == "outerShdw" )
+        {
+            sal_Int32 nAttrDir = 0, nAttrDist = 0;
+            std::map< OUString, css::uno::Any >::iterator attribIt = (*it)->maAttribs.find( "dir" );
+            if( attribIt != (*it)->maAttribs.end() )
+                attribIt->second >>= nAttrDir;
+
+            attribIt = (*it)->maAttribs.find( "dist" );
+            if( attribIt != (*it)->maAttribs.end() )
+                attribIt->second >>= nAttrDist;
+
+            // Negative X or Y dist indicates left or up, respectively
+            double nAngle = ( nAttrDir / PER_DEGREE ) * F_PI180;
+            sal_Int32 nDist = convertEmuToHmm( nAttrDist );
+            sal_Int32 nXDist = cos(nAngle) * nDist;
+            sal_Int32 nYDist = sin(nAngle) * nDist;
+
+            rPropMap.setProperty( PROP_Shadow, true );
+            rPropMap.setProperty( PROP_ShadowXDistance, nXDist);
+            rPropMap.setProperty( PROP_ShadowYDistance, nYDist);
+            rPropMap.setProperty( PROP_ShadowColor, (*it)->moColor.getColor(rGraphicHelper, -1 ) );
+            rPropMap.setProperty( PROP_ShadowTransparence, (*it)->moColor.getTransparency());
+        }
 }
 
-css::beans::PropertyValue EffectProperties::getUnsupportedEffect()
+css::beans::PropertyValue Effect::getEffect()
 {
     css::beans::PropertyValue pRet;
-    if(!msUnsupportedEffectName.has())
+    if( msName.isEmpty() )
         return pRet;
 
-    css::uno::Sequence<css::beans::PropertyValue> aSeq(maUnsupportedEffectAttribs.size());
-    css::beans::PropertyValue* pSeq = aSeq.getArray();
-    for (std::vector<css::beans::PropertyValue>::iterator i = maUnsupportedEffectAttribs.begin(); i != maUnsupportedEffectAttribs.end(); ++i)
-        *pSeq++ = *i;
+    css::uno::Sequence< css::beans::PropertyValue > aSeq( maAttribs.size() );
+    sal_uInt32 i = 0;
+    for( std::map< OUString, css::uno::Any >::iterator it = maAttribs.begin(); it != maAttribs.end(); ++it )
+    {
+        aSeq[i].Name = it->first;
+        aSeq[i].Value = it->second;
+        i++;
+    }
 
-    pRet.Name = msUnsupportedEffectName.use();
+    pRet.Name = msName;
     pRet.Value = css::uno::Any( aSeq );
 
-    msUnsupportedEffectName.reset();
-    maUnsupportedEffectAttribs.clear();
-
     return pRet;
 }
 
diff --git a/oox/source/drawingml/effectpropertiescontext.cxx b/oox/source/drawingml/effectpropertiescontext.cxx
index 2493ec9..fa515d1 100644
--- a/oox/source/drawingml/effectpropertiescontext.cxx
+++ b/oox/source/drawingml/effectpropertiescontext.cxx
@@ -33,80 +33,66 @@ EffectPropertiesContext::~EffectPropertiesContext()
 {
 }
 
-void EffectPropertiesContext::saveUnsupportedAttribs( const AttributeList& rAttribs )
+void EffectPropertiesContext::saveUnsupportedAttribs( Effect& rEffect, const AttributeList& rAttribs )
 {
     if( rAttribs.hasAttribute( XML_algn ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "algn",
-                                                          makeAny( rAttribs.getString( XML_algn, "" ) ) );
+        rEffect.maAttribs["algn"] = makeAny( rAttribs.getString( XML_algn, "" ) );
     if( rAttribs.hasAttribute( XML_blurRad ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "blurRad",
-                                                          makeAny( rAttribs.getInteger( XML_blurRad, 0 ) ) );
+        rEffect.maAttribs["blurRad"] = makeAny( rAttribs.getInteger( XML_blurRad, 0 ) );
     if( rAttribs.hasAttribute( XML_dir ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "dir",
-                                                          makeAny( rAttribs.getInteger( XML_dir, 0 ) ) );
+        rEffect.maAttribs["dir"] = makeAny( rAttribs.getInteger( XML_dir, 0 ) );
     if( rAttribs.hasAttribute( XML_dist ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "dist",
-                                                          makeAny( rAttribs.getInteger( XML_dist, 0 ) ) );
+        rEffect.maAttribs["dist"] = makeAny( rAttribs.getInteger( XML_dist, 0 ) );
     if( rAttribs.hasAttribute( XML_kx ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "kx",
-                                                          makeAny( rAttribs.getInteger( XML_kx, 0 ) ) );
+        rEffect.maAttribs["kx"] =  makeAny( rAttribs.getInteger( XML_kx, 0 ) );
     if( rAttribs.hasAttribute( XML_ky ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "ky",
-                                                          makeAny( rAttribs.getInteger( XML_ky, 0 ) ) );
+        rEffect.maAttribs["ky"] = makeAny( rAttribs.getInteger( XML_ky, 0 ) );
     if( rAttribs.hasAttribute( XML_rotWithShape ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "rotWithShape",
-                                                          makeAny( rAttribs.getInteger( XML_rotWithShape, 0 ) ) );
+        rEffect.maAttribs["rotWithShape"] = makeAny( rAttribs.getInteger( XML_rotWithShape, 0 ) );
     if( rAttribs.hasAttribute( XML_sx ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "sx",
-                                                          makeAny( rAttribs.getInteger( XML_sx, 0 ) ) );
+        rEffect.maAttribs["sx"] = makeAny( rAttribs.getInteger( XML_sx, 0 ) );
     if( rAttribs.hasAttribute( XML_sy ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "sy",
-                                                          makeAny( rAttribs.getInteger( XML_sy, 0 ) ) );
+        rEffect.maAttribs["sy"] = makeAny( rAttribs.getInteger( XML_sy, 0 ) );
     if( rAttribs.hasAttribute( XML_rad ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "rad",
-                                                          makeAny( rAttribs.getInteger( XML_rad, 0 ) ) );
+        rEffect.maAttribs["rad"] = makeAny( rAttribs.getInteger( XML_rad, 0 ) );
     if( rAttribs.hasAttribute( XML_endA ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "endA",
-                                                          makeAny( rAttribs.getInteger( XML_endA, 0 ) ) );
+        rEffect.maAttribs["endA"] = makeAny( rAttribs.getInteger( XML_endA, 0 ) );
     if( rAttribs.hasAttribute( XML_endPos ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "endPos",
-                                                          makeAny( rAttribs.getInteger( XML_endPos, 0 ) ) );
+        rEffect.maAttribs["endPos"] = makeAny( rAttribs.getInteger( XML_endPos, 0 ) );
     if( rAttribs.hasAttribute( XML_fadeDir ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "fadeDir",
-                                                          makeAny( rAttribs.getInteger( XML_fadeDir, 0 ) ) );
+        rEffect.maAttribs["fadeDir"] = makeAny( rAttribs.getInteger( XML_fadeDir, 0 ) );
     if( rAttribs.hasAttribute( XML_stA ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "stA",
-                                                          makeAny( rAttribs.getInteger( XML_stA, 0 ) ) );
+        rEffect.maAttribs["stA"] = makeAny( rAttribs.getInteger( XML_stA, 0 ) );
     if( rAttribs.hasAttribute( XML_stPos ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "stPos",
-                                                          makeAny( rAttribs.getInteger( XML_stPos, 0 ) ) );
+        rEffect.maAttribs["stPos"] = makeAny( rAttribs.getInteger( XML_stPos, 0 ) );
     if( rAttribs.hasAttribute( XML_grow ) )
-        mrEffectProperties.appendUnsupportedEffectAttrib( "grow",
-                                                          makeAny( rAttribs.getInteger( XML_grow, 0 ) ) );
+        rEffect.maAttribs["grow"] = makeAny( rAttribs.getInteger( XML_grow, 0 ) );
 }
 
 ContextHandlerRef EffectPropertiesContext::onCreateContext( sal_Int32 nElement, const AttributeList& rAttribs )
 {
+    sal_Int32 nPos = mrEffectProperties.maEffects.size();
+    mrEffectProperties.maEffects.push_back( new Effect() );
     switch( nElement )
     {
         case A_TOKEN( outerShdw ):
         {
-            mrEffectProperties.msUnsupportedEffectName = "outerShdw";
-            saveUnsupportedAttribs( rAttribs );
+            mrEffectProperties.maEffects[nPos]->msName = "outerShdw";
+            saveUnsupportedAttribs( *mrEffectProperties.maEffects[nPos], rAttribs );
 
             mrEffectProperties.maShadow.moShadowDist = rAttribs.getInteger( XML_dist, 0 );
             mrEffectProperties.maShadow.moShadowDir = rAttribs.getInteger( XML_dir, 0 );
-            return new ColorContext( *this, mrEffectProperties.maShadow.moShadowColor );
+            return new ColorContext( *this, mrEffectProperties.maEffects[nPos]->moColor );
         }
         break;
         case A_TOKEN( innerShdw ):
         {
-            mrEffectProperties.msUnsupportedEffectName = "innerShdw";
-            saveUnsupportedAttribs( rAttribs );
+            mrEffectProperties.maEffects[nPos]->msName = "innerShdw";
+            saveUnsupportedAttribs( *mrEffectProperties.maEffects[nPos], rAttribs );
 
             mrEffectProperties.maShadow.moShadowDist = rAttribs.getInteger( XML_dist, 0 );
             mrEffectProperties.maShadow.moShadowDir = rAttribs.getInteger( XML_dir, 0 );
-            return new ColorContext( *this, mrEffectProperties.maShadow.moShadowColor );
+            return new ColorContext( *this, mrEffectProperties.maEffects[nPos]->moColor );
         }
         break;
         case A_TOKEN( glow ):
@@ -115,19 +101,20 @@ ContextHandlerRef EffectPropertiesContext::onCreateContext( sal_Int32 nElement,
         case A_TOKEN( blur ):
         {
             if( nElement == A_TOKEN( glow ) )
-                mrEffectProperties.msUnsupportedEffectName = "glow";
+                mrEffectProperties.maEffects[nPos]->msName = "glow";
             else if( nElement == A_TOKEN( softEdge ) )
-                mrEffectProperties.msUnsupportedEffectName = "softEdge";
+                mrEffectProperties.maEffects[nPos]->msName = "softEdge";
             else if( nElement == A_TOKEN( reflection ) )
-                mrEffectProperties.msUnsupportedEffectName = "reflection";
+                mrEffectProperties.maEffects[nPos]->msName = "reflection";
             else if( nElement == A_TOKEN( blur ) )
-                mrEffectProperties.msUnsupportedEffectName = "blur";
-            saveUnsupportedAttribs( rAttribs );
-            return new ColorContext( *this, mrEffectProperties.maShadow.moShadowColor );
+                mrEffectProperties.maEffects[nPos]->msName = "blur";
+            saveUnsupportedAttribs( *mrEffectProperties.maEffects[nPos], rAttribs );
+            return new ColorContext( *this, mrEffectProperties.maEffects[nPos]->moColor );
         }
         break;
     }
 
+    mrEffectProperties.maEffects.pop_back();
     return 0;
 }
 
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index bce7357..b319c38 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -911,30 +911,41 @@ Reference< XShape > Shape::createAndInsert(
             }
 
             // store unsupported effect attributes in the grab bag
-            PropertyValue aEffect = aEffectProperties.getUnsupportedEffect();
-            if( aEffect.Name != "" )
+            if( aEffectProperties.maEffects.size() > 0 )
             {
-                Sequence< PropertyValue > aEffectsGrabBag( 3 );
-                PUT_PROP( aEffectsGrabBag, 0, aEffect.Name, aEffect.Value );
-
-                OUString sColorScheme = aEffectProperties.maShadow.moShadowColor.getSchemeName();
-                if( sColorScheme.isEmpty() )
-                {
-                    // RGB color and transparency value
-                    PUT_PROP( aEffectsGrabBag, 1, "ShadowRgbClr",
-                              aEffectProperties.maShadow.moShadowColor.getColor( rGraphicHelper, nFillPhClr ) );
-                    PUT_PROP( aEffectsGrabBag, 2, "ShadowRgbClrTransparency",
-                              aEffectProperties.maShadow.moShadowColor.getTransparency() );
-                }
-                else
+                Sequence< PropertyValue > aEffects( aEffectProperties.maEffects.size() );
+                sal_uInt32 i = 0;
+                for( std::vector< Effect* >::iterator it = aEffectProperties.maEffects.begin();
+                        it != aEffectProperties.maEffects.end(); ++it )
                 {
-                    // scheme color with name and transformations
-                    PUT_PROP( aEffectsGrabBag, 1, "ShadowColorSchemeClr", sColorScheme );
-                    PUT_PROP( aEffectsGrabBag, 2, "ShadowColorTransformations",
-                              aEffectProperties.maShadow.moShadowColor.getTransformations() );
-                }
+                    PropertyValue aEffect = (*it)->getEffect();
+                    if( !aEffect.Name.isEmpty() )
+                    {
+                        Sequence< PropertyValue > aEffectsGrabBag( 3 );
+                        PUT_PROP( aEffectsGrabBag, 0, "Attribs", aEffect.Value );
 
-                putPropertyToGrabBag( "EffectProperties", Any( aEffectsGrabBag ) );
+                        Color& aColor( (*it)->moColor );
+                        OUString sColorScheme = aColor.getSchemeName();
+                        if( sColorScheme.isEmpty() )
+                        {
+                            // RGB color and transparency value
+                            PUT_PROP( aEffectsGrabBag, 1, "RgbClr",
+                                      aColor.getColor( rGraphicHelper, nFillPhClr ) );
+                            PUT_PROP( aEffectsGrabBag, 2, "RgbClrTransparency",
+                                      aColor.getTransparency() );
+                        }
+                        else
+                        {
+                            // scheme color with name and transformations
+                            PUT_PROP( aEffectsGrabBag, 1, "SchemeClr", sColorScheme );
+                            PUT_PROP( aEffectsGrabBag, 2, "SchemeClrTransformations",
+                                      aColor.getTransformations() );
+                        }
+                        PUT_PROP( aEffects, i, aEffect.Name, aEffectsGrabBag );
+                        ++i;
+                    }
+                }
+                putPropertyToGrabBag( "EffectProperties", Any( aEffects ) );
             }
 
             // add 3D effects if any
diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx
index 1d663e4..037466a 100644
--- a/oox/source/export/drawingml.cxx
+++ b/oox/source/export/drawingml.cxx
@@ -2076,57 +2076,45 @@ void DrawingML::WriteShapeStyle( Reference< XPropertySet > xPropSet )
     mpFS->singleElementNS( XML_a, XML_fontRef, XML_idx, "minor", FSEND );
 }
 
-void DrawingML::WriteShapeEffects( Reference< XPropertySet > rXPropSet )
+void DrawingML::WriteShapeEffect( const OUString& sName, const Sequence< PropertyValue >& aEffectProps )
 {
-    if( !GetProperty( rXPropSet, "InteropGrabBag" ) )
+    if( aEffectProps.getLength() == 0 )
         return;
 
-    Sequence< PropertyValue > aGrabBag, aEffectProps;
-    mAny >>= aGrabBag;
-    for( sal_Int32 i=0; i < aGrabBag.getLength(); ++i )
+    // assign the proper tag and enable bContainsColor if necessary
+    sal_Int32 nEffectToken = 0;
+    bool bContainsColor = false;
+    if( sName == "outerShdw" )
     {
-        if( aGrabBag[i].Name == "EffectProperties" )
-            aGrabBag[i].Value >>= aEffectProps;
+        nEffectToken = FSNS( XML_a, XML_outerShdw );
+        bContainsColor = true;
     }
-    if( aEffectProps.getLength() == 0 )
-        return;
+    else if( sName == "innerShdw" )
+    {
+        nEffectToken = FSNS( XML_a, XML_innerShdw );
+        bContainsColor = true;
+    }
+    else if( sName == "glow" )
+    {
+        nEffectToken = FSNS( XML_a, XML_glow );
+        bContainsColor = true;
+    }
+    else if( sName == "softEdge" )
+        nEffectToken = FSNS( XML_a, XML_softEdge );
+    else if( sName == "reflection" )
+        nEffectToken = FSNS( XML_a, XML_reflection );
+    else if( sName == "blur" )
+        nEffectToken = FSNS( XML_a, XML_blur );
 
     OUString sSchemeClr;
-    bool bContainsColor = false;
     sal_uInt32 nRgbClr = 0;
-    sal_Int32 nEffectToken = 0;
     sal_Int32 nAlpha = MAX_PERCENT;
     Sequence< PropertyValue > aTransformations;
     sax_fastparser::FastAttributeList *aOuterShdwAttrList = mpFS->createAttrList();
     for( sal_Int32 i=0; i < aEffectProps.getLength(); ++i )
     {
-        if( aEffectProps[i].Name == "outerShdw" || aEffectProps[i].Name == "innerShdw"
-                || aEffectProps[i].Name == "glow" || aEffectProps[i].Name == "softEdge"
-                || aEffectProps[i].Name == "reflection" || aEffectProps[i].Name == "blur" )
+        if( aEffectProps[i].Name == "Attribs" )
         {
-            // assign the proper tag and enable bContainsColor if necessary
-            if( aEffectProps[i].Name == "outerShdw" )
-            {
-                nEffectToken = FSNS( XML_a, XML_outerShdw );
-                bContainsColor = true;
-            }
-            else if( aEffectProps[i].Name == "innerShdw" )
-            {
-                nEffectToken = FSNS( XML_a, XML_innerShdw );
-                bContainsColor = true;
-            }
-            else if( aEffectProps[i].Name == "glow" )
-            {
-                nEffectToken = FSNS( XML_a, XML_glow );
-                bContainsColor = true;
-            }
-            else if( aEffectProps[i].Name == "softEdge" )
-                nEffectToken = FSNS( XML_a, XML_softEdge );
-            else if( aEffectProps[i].Name == "reflection" )
-                nEffectToken = FSNS( XML_a, XML_reflection );
-            else if( aEffectProps[i].Name == "blur" )
-                nEffectToken = FSNS( XML_a, XML_blur );
-
             // read tag attributes
             uno::Sequence< beans::PropertyValue > aOuterShdwProps;
             aEffectProps[i].Value >>= aOuterShdwProps;
@@ -2230,22 +2218,22 @@ void DrawingML::WriteShapeEffects( Reference< XPropertySet > rXPropSet )
                 }
             }
         }
-        else if(aEffectProps[i].Name == "ShadowRgbClr")
+        else if(aEffectProps[i].Name == "RgbClr")
         {
             aEffectProps[i].Value >>= nRgbClr;
         }
-        else if(aEffectProps[i].Name == "ShadowRgbClrTransparency")
+        else if(aEffectProps[i].Name == "RgbClrTransparency")
         {
             sal_Int32 nTransparency;
             aEffectProps[i].Value >>= nTransparency;
             // Calculate alpha value (see oox/source/drawingml/color.cxx : getTransparency())
             nAlpha = MAX_PERCENT - ( PER_PERCENT * nTransparency );
         }
-        else if(aEffectProps[i].Name == "ShadowColorSchemeClr")
+        else if(aEffectProps[i].Name == "SchemeClr")
         {
             aEffectProps[i].Value >>= sSchemeClr;
         }
-        else if(aEffectProps[i].Name == "ShadowColorTransformations")
+        else if(aEffectProps[i].Name == "SchemeClrTransformations")
         {
             aEffectProps[i].Value >>= aTransformations;
         }
@@ -2253,7 +2241,6 @@ void DrawingML::WriteShapeEffects( Reference< XPropertySet > rXPropSet )
 
     if( nEffectToken > 0 )
     {
-        mpFS->startElementNS(XML_a, XML_effectLst, FSEND);
         sax_fastparser::XFastAttributeListRef xAttrList( aOuterShdwAttrList );
         mpFS->startElement( nEffectToken, xAttrList );
 
@@ -2266,10 +2253,39 @@ void DrawingML::WriteShapeEffects( Reference< XPropertySet > rXPropSet )
         }
 
         mpFS->endElement( nEffectToken );
-        mpFS->endElementNS(XML_a, XML_effectLst);
     }
 }
 
+void DrawingML::WriteShapeEffects( Reference< XPropertySet > rXPropSet )
+{
+    if( !GetProperty( rXPropSet, "InteropGrabBag" ) )
+        return;
+
+    Sequence< PropertyValue > aGrabBag, aEffects;
+    mAny >>= aGrabBag;
+    for( sal_Int32 i=0; i < aGrabBag.getLength(); ++i )
+    {
+        if( aGrabBag[i].Name == "EffectProperties" )
+        {
+            aGrabBag[i].Value >>= aEffects;
+            break;
+        }
+    }
+    if( aEffects.getLength() == 0 )
+        return;
+
+    mpFS->startElementNS(XML_a, XML_effectLst, FSEND);
+
+    for( sal_Int32 i=0; i < aEffects.getLength(); ++i )
+    {
+        Sequence< PropertyValue > aEffectProps;
+        aEffects[i].Value >>= aEffectProps;
+        WriteShapeEffect( aEffects[i].Name, aEffectProps );
+    }
+
+    mpFS->endElementNS(XML_a, XML_effectLst);
+}
+
 void DrawingML::WriteShape3DEffects( Reference< XPropertySet > xPropSet )
 {
     // check existence of the grab bag
diff --git a/sw/qa/extras/ooxmlexport/data/shape-effect-preservation.docx b/sw/qa/extras/ooxmlexport/data/shape-effect-preservation.docx
index bd0ac80..f2b91ea 100644
Binary files a/sw/qa/extras/ooxmlexport/data/shape-effect-preservation.docx and b/sw/qa/extras/ooxmlexport/data/shape-effect-preservation.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx b/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx
index 2ea3562..736546a 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlsdrexport.cxx
@@ -1143,6 +1143,35 @@ DECLARE_OOXMLEXPORT_TEST(testShapeEffectPreservation, "shape-effect-preservation
     assertXPath(pXmlDoc, "/w:document/w:body/w:p[7]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
             "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:reflection/*",
             0 ); // should not be present
+
+    // 7th shape with several effects: glow, inner shadow and reflection
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:glow",
+            "rad", "63500");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:glow/a:schemeClr",
+            "val", "accent2");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:glow/a:schemeClr/*",
+            2);
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:innerShdw",
+            "blurRad", "63500");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:innerShdw",
+            "dir", "2700000");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:innerShdw/a:srgbClr",
+            "val", "000000");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:innerShdw/a:srgbClr/a:alpha",
+            "val", "50000");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:reflection",
+            "blurRad", "6350");
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p[8]/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+            "wp:anchor/a:graphic/a:graphicData/wps:wsp/wps:spPr/a:effectLst/a:reflection",
+            "stA", "52000");
 }
 
 DECLARE_OOXMLEXPORT_TEST(testShape3DEffectPreservation, "shape-3d-effect-preservation.docx")
commit a5835285068c1b03171b7953c2fea185111f4da2
Author: Jacobo Aragunde Pérez <jaragunde at igalia.com>
Date:   Wed May 14 19:51:11 2014 +0200

    oox: always save all kinds of 3d effects to the grab bag.
    
    This was actually a bug. It didn't matter much because a document with
    an a:sp3d tag but without a:scene3d section would be invalid, but the
    code was logically wrong.
    
    Change-Id: Ifa838e425849642c2a1bf6fca6b6a8dc8ed3b465

diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index 6d13868..bce7357 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -941,7 +941,7 @@ Reference< XShape > Shape::createAndInsert(
             Sequence< PropertyValue > aCamera3DEffects = get3DProperties().getCameraAttributes();
             Sequence< PropertyValue > aLightRig3DEffects = get3DProperties().getLightRigAttributes();
             Sequence< PropertyValue > aShape3DEffects = get3DProperties().getShape3DAttributes( rGraphicHelper, nFillPhClr );
-            if( aCamera3DEffects.getLength() > 0 || aLightRig3DEffects.getLength() > 0 )
+            if( aCamera3DEffects.getLength() > 0 || aLightRig3DEffects.getLength() > 0 || aShape3DEffects.getLength() > 0 )
             {
                 Sequence< PropertyValue > a3DEffectsGrabBag( 3 );
                 PUT_PROP( a3DEffectsGrabBag, 0, "Camera", Any( aCamera3DEffects ) );


More information about the Libreoffice-commits mailing list