oox/source/vml/vmlformatting.cxx | 17 ++-- sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle2.docx |binary sw/qa/extras/ooxmlexport/ooxmlexport7.cxx | 27 +++++- sw/qa/extras/rtfexport/data/tdf159824_gradientAngle3.rtf | 17 ++++ sw/qa/extras/rtfexport/data/tdf159824_gradientAngle4.rtf | 17 ++++ sw/qa/extras/rtfexport/rtfexport8.cxx | 62 +++++++++++++-- sw/source/filter/ww8/docxattributeoutput.cxx | 23 ++--- sw/source/filter/ww8/rtfattributeoutput.cxx | 37 ++++---- 8 files changed, 153 insertions(+), 47 deletions(-)
New commits: commit 6dd6891a3deed8718bf2b0fcf564f229f676f8ba Author: Justin Luth <jl...@mail.com> AuthorDate: Thu Feb 22 09:24:12 2024 -0500 Commit: Justin Luth <jl...@mail.com> CommitDate: Fri Feb 23 20:28:31 2024 +0100 related tdf#126533 tdf#159824 VML: don't export: negative angles and stop an automatic 180 reversal on import. Some documents had gradient reversals on every round trip, typically when the angle was 0-179 (mainly seen in ODT examples, since DOCX/RTF imports defaulted to be angle 180). The negative sign has special meaning, indicating that the start and end colors should be swapped. Well, swapping colors was not intentional in the export logic. Previously there was a mistaken idea that any angles > 180 needed to be swapped on import, and likely that is what prompted this overly complicated formula to try to avoid any angle > 180 during export by allowing negative angles. This tdf#126533 patchset has already eliminated import checks for angles > 180, so now a sane formula can be applied on export. In order to do that, we have to avoid emulating color swaps with 180 degree rotations at import time. So ONLY do color swapping with start/end, and leave the angle alone. That GREATLY helps unit tests (which otherwise would flip-flop the angle and the color start/stop). Very unhelpful was an undocumented, indecipherable inversion when converting to DML angle. Boy, I hope I got this right... make CppunitTest_sw_rtfexport8 \ CPPUNIT_TEST_NAME=testTdf159824_gradientAngle3 make CppunitTest_sw_rtfexport8 \ CPPUNIT_TEST_NAME=testTdf159824_gradientAngle4 make CppunitTest_sw_ooxmlexport7 \ CPPUNIT_TEST_NAME=testTdf126533_axialAngle2 Eliminating the inversion for ooxml7 test is fine since inversion does nothing to an axial. Otherwise, eliminating inversions corresponds to a color swap. Change-Id: I2aae0a7595807569ffc740689ff3840692d6159d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163798 Tested-by: Jenkins Reviewed-by: Justin Luth <jl...@mail.com> diff --git a/oox/source/vml/vmlformatting.cxx b/oox/source/vml/vmlformatting.cxx index f4022d0639d6..5fb422e63b9d 100644 --- a/oox/source/vml/vmlformatting.cxx +++ b/oox/source/vml/vmlformatting.cxx @@ -800,22 +800,29 @@ void FillModel::pushToPropMap( ShapePropertyMap& rPropMap, const GraphicHelper& } else // focus of -100%, 0%, and 100% is linear gradient { + // LO linear gradients: top == start, but for MSO bottom == start == moColor + bool bSwapColors = true; + /* According to spec, a focus of -100% or 100% swaps the start and stop colors, effectively reversing the gradient. If the angle was provided as a negative, then the colors are also (again) reversed. */ if( fFocus < -0.5 || fFocus > 0.5 ) - nVmlAngle = (nVmlAngle + 180) % 360; + bSwapColors = !bSwapColors; if (moAngle.value_or(0) < 0) - nVmlAngle = (nVmlAngle + 180) % 360; + bSwapColors = !bSwapColors; + const Color& rStartColor = bSwapColors ? aColor2 : aColor1; + const Color& rEndColor = bSwapColors ? aColor1 : aColor2; // set the start and stop colors - lcl_setGradientStop( aFillProps.maGradientProps.maGradientStops, 0.0, aColor1 ); - lcl_setGradientStop( aFillProps.maGradientProps.maGradientStops, 1.0, aColor2 ); + lcl_setGradientStop(aFillProps.maGradientProps.maGradientStops, 0.0, + rStartColor); + lcl_setGradientStop(aFillProps.maGradientProps.maGradientStops, 1.0, + rEndColor); } // VML counts counterclockwise from bottom, DrawingML clockwise from left - sal_Int32 nDmlAngle = (630 - nVmlAngle) % 360; + sal_Int32 nDmlAngle = NormAngle360(90 - nVmlAngle); aFillProps.maGradientProps.moShadeAngle = nDmlAngle * ::oox::drawingml::PER_DEGREE; } else // XML_gradientRadial is rectangular gradient diff --git a/sw/qa/extras/ooxmlexport/data/tdf126533_negativeAxialAngle.docx b/sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle.docx similarity index 100% rename from sw/qa/extras/ooxmlexport/data/tdf126533_negativeAxialAngle.docx rename to sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle.docx diff --git a/sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle2.docx b/sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle2.docx new file mode 100644 index 000000000000..49261dfef567 Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle2.docx differ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx index a362cb36c221..020b9196cc72 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx @@ -640,9 +640,9 @@ DECLARE_OOXMLEXPORT_TEST(testTdf77219_backgroundShape, "tdf77219_backgroundShape CPPUNIT_ASSERT_EQUAL(sal_Int16(3150), aGradient.Angle); } -DECLARE_OOXMLEXPORT_TEST(testTdf126533_negativeAxialAngle, "tdf126533_negativeAxialAngle.docx") +DECLARE_OOXMLEXPORT_TEST(testTdf126533_axialAngle, "tdf126533_axialAngle.docx") { - // axiel gradient is purple foreground/lime background in the middle (top-left to bottom-right) + // axial gradient is purple foreground/lime background in the middle (top-left to bottom-right) uno::Reference<beans::XPropertySet> xPageStyle(getStyles("PageStyles")->getByName("Standard"), uno::UNO_QUERY); CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_GRADIENT, getProperty<drawing::FillStyle>(xPageStyle, "FillStyle")); @@ -658,7 +658,28 @@ DECLARE_OOXMLEXPORT_TEST(testTdf126533_negativeAxialAngle, "tdf126533_negativeAx CPPUNIT_ASSERT(basegfx::fTools::equal(aColorStops[2].getStopOffset(), 1.0)); CPPUNIT_ASSERT_EQUAL(COL_LIGHTMAGENTA, Color(aColorStops[2].getStopColor())); // without the fix, this was 1350 (visually the colors were reversed) - CPPUNIT_ASSERT_EQUAL(sal_Int16(450), aGradient.Angle); + CPPUNIT_ASSERT_EQUAL(sal_Int16(2250), aGradient.Angle); + CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_LINEAR, aGradient.Style); +} + +DECLARE_OOXMLEXPORT_TEST(testTdf126533_axialAngle2, "tdf126533_axialAngle2.docx") +{ + // axial gradient is purple foreground/lime background in the middle (top-right to bottom-left) + uno::Reference<beans::XPropertySet> xPageStyle(getStyles("PageStyles")->getByName("Standard"), + uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_GRADIENT, getProperty<drawing::FillStyle>(xPageStyle, "FillStyle")); + awt::Gradient2 aGradient = getProperty<awt::Gradient2>(xPageStyle, "FillGradient"); + + basegfx::BColorStops aColorStops = model::gradient::getColorStopsFromUno(aGradient.ColorStops); + + CPPUNIT_ASSERT_EQUAL(size_t(3), aColorStops.size()); + CPPUNIT_ASSERT(basegfx::fTools::equal(aColorStops[0].getStopOffset(), 0.0)); + CPPUNIT_ASSERT_EQUAL(COL_LIGHTMAGENTA, Color(aColorStops[0].getStopColor())); + CPPUNIT_ASSERT(basegfx::fTools::equal(aColorStops[1].getStopOffset(), 0.5)); + CPPUNIT_ASSERT_EQUAL(COL_LIGHTGREEN, Color(aColorStops[1].getStopColor())); + CPPUNIT_ASSERT(basegfx::fTools::equal(aColorStops[2].getStopOffset(), 1.0)); + CPPUNIT_ASSERT_EQUAL(COL_LIGHTMAGENTA, Color(aColorStops[2].getStopColor())); + CPPUNIT_ASSERT_EQUAL(sal_Int16(1350), aGradient.Angle); CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_LINEAR, aGradient.Style); } diff --git a/sw/qa/extras/rtfexport/data/tdf159824_gradientAngle3.rtf b/sw/qa/extras/rtfexport/data/tdf159824_gradientAngle3.rtf new file mode 100644 index 000000000000..053696d2a279 --- /dev/null +++ b/sw/qa/extras/rtfexport/data/tdf159824_gradientAngle3.rtf @@ -0,0 +1,17 @@ +{ tf1nsi\deff3deflang1025 + +\landscape\paperh5940\paperw8391 + +{ +\shp{\*\shpinst\shptop382\shpbottom2737\shpleft759\shpright5064 +{\sp{\sn shapeType}{\sv 202}} +{\sp{\sn fillType}{\sv 7}} +{\sp{\sn fillAngle}{\sv 10860000}} +{\sp{\sn fillFocus}{\sv 0}} +{\sp{\sn fillColor}{\sv 1758337}} +{\sp{\sn fillBackColor}{\sv 16777215}} + +{\shptxt\par \pard}} +} + +\par } diff --git a/sw/qa/extras/rtfexport/data/tdf159824_gradientAngle4.rtf b/sw/qa/extras/rtfexport/data/tdf159824_gradientAngle4.rtf new file mode 100644 index 000000000000..f56d82fc82ce --- /dev/null +++ b/sw/qa/extras/rtfexport/data/tdf159824_gradientAngle4.rtf @@ -0,0 +1,17 @@ +{ tf1nsi\deff3deflang1025 + +\landscape\paperh5940\paperw8391 + +{ +\shp{\*\shpinst\shptop382\shpbottom2737\shpleft759\shpright5064 +{\sp{\sn shapeType}{\sv 202}} +{\sp{\sn fillType}{\sv 7}} +{\sp{\sn fillAngle}{\sv -5400000}} +{\sp{\sn fillFocus}{\sv 0}} +{\sp{\sn fillColor}{\sv 1758337}} +{\sp{\sn fillBackColor}{\sv 16777215}} + +{\shptxt\par \pard}} +} + +\par } diff --git a/sw/qa/extras/rtfexport/rtfexport8.cxx b/sw/qa/extras/rtfexport/rtfexport8.cxx index c0b8cbad9a0a..7b303f3fd3da 100644 --- a/sw/qa/extras/rtfexport/rtfexport8.cxx +++ b/sw/qa/extras/rtfexport/rtfexport8.cxx @@ -228,14 +228,16 @@ DECLARE_RTFEXPORT_TEST(testTdf159824_gradientAngle1, "tdf159824_gradientAngle1.r CPPUNIT_ASSERT_EQUAL(size_t(2), aColorStops.size()); CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_LINEAR, aGradient.Style); - CPPUNIT_ASSERT_EQUAL(sal_Int32(8508442), aGradient.StartColor); - CPPUNIT_ASSERT_EQUAL(sal_Int32(COL_WHITE), aGradient.EndColor); - // RTF 1° angle (in 1/60,000 degree units = 60,000) == LO 181° (in 1/10 degree unites) - CPPUNIT_ASSERT_EQUAL(sal_Int32(181), sal_Int32(aGradient.Angle / 10)); + CPPUNIT_ASSERT_EQUAL(sal_Int32(COL_WHITE), aGradient.StartColor); + CPPUNIT_ASSERT_EQUAL(sal_Int32(8508442), aGradient.EndColor); + // RTF 1° angle (in 1/60,000 degree units = 60,000) == LO 1° (in 1/10 degree units) + CPPUNIT_ASSERT_EQUAL(sal_Int16(10), aGradient.Angle); } DECLARE_RTFEXPORT_TEST(testTdf159824_gradientAngle2, "tdf159824_gradientAngle2.rtf") { + // This file is identical to gradientAngle1 except that the fillAngle is -2° instead of 1°. + // given a frame with a lime (top) to white (bottom) linear gradient at an RTF -2° angle. uno::Reference<text::XTextFramesSupplier> xTextFramesSupplier(mxComponent, uno::UNO_QUERY); uno::Reference<container::XIndexAccess> xIndexAccess(xTextFramesSupplier->getTextFrames(), @@ -248,11 +250,59 @@ DECLARE_RTFEXPORT_TEST(testTdf159824_gradientAngle2, "tdf159824_gradientAngle2.r // MCGR: Use the completely imported transparency gradient to check for correctness basegfx::BColorStops aColorStops = model::gradient::getColorStopsFromUno(aGradient.ColorStops); CPPUNIT_ASSERT_EQUAL(size_t(2), aColorStops.size()); + // because of the negative angle, the colors were swapped compared to gradientAngle1.rtf + CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_LINEAR, aGradient.Style); + CPPUNIT_ASSERT_EQUAL(sal_Int32(8508442), aGradient.StartColor); + CPPUNIT_ASSERT_EQUAL(sal_Int32(COL_WHITE), aGradient.EndColor); + // RTF -2° angle (in 1/60,000 degree units = -120,000) == LO 358° (in 1/10 degree units) + CPPUNIT_ASSERT_EQUAL(sal_Int16(3580), aGradient.Angle); +} + +DECLARE_RTFEXPORT_TEST(testTdf159824_gradientAngle3, "tdf159824_gradientAngle3.rtf") +{ + // This file is identical to gradientAngle1 except that the fillAngle is 181° instead of 1°. + + // given a frame with a lime (top) to white (bottom) linear gradient at an RTF 181° angle. + uno::Reference<text::XTextFramesSupplier> xTextFramesSupplier(mxComponent, uno::UNO_QUERY); + uno::Reference<container::XIndexAccess> xIndexAccess(xTextFramesSupplier->getTextFrames(), + uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xFrame(xIndexAccess->getByIndex(0), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_GRADIENT, + getProperty<drawing::FillStyle>(xFrame, "FillStyle")); + awt::Gradient2 aGradient = getProperty<awt::Gradient2>(xFrame, "FillGradient"); + + // MCGR: Use the completely imported transparency gradient to check for correctness + basegfx::BColorStops aColorStops = model::gradient::getColorStopsFromUno(aGradient.ColorStops); + CPPUNIT_ASSERT_EQUAL(size_t(2), aColorStops.size()); + CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_LINEAR, aGradient.Style); + CPPUNIT_ASSERT_EQUAL(sal_Int32(COL_WHITE), aGradient.StartColor); + CPPUNIT_ASSERT_EQUAL(sal_Int32(8508442), aGradient.EndColor); + // RTF 181° angle (in 1/60,000 degree units) == LO 181° (in 1/10 degree units) + CPPUNIT_ASSERT_EQUAL(sal_Int16(1810), aGradient.Angle); +} + +DECLARE_RTFEXPORT_TEST(testTdf159824_gradientAngle4, "tdf159824_gradientAngle4.rtf") +{ + // This file is identical to gradientAngle1 except that the fillAngle is -90° instead of 1°. + + // given a frame with a white (left) to lime (right) linear gradient at an RTF -90° angle + uno::Reference<text::XTextFramesSupplier> xTextFramesSupplier(mxComponent, uno::UNO_QUERY); + uno::Reference<container::XIndexAccess> xIndexAccess(xTextFramesSupplier->getTextFrames(), + uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xFrame(xIndexAccess->getByIndex(0), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_GRADIENT, + getProperty<drawing::FillStyle>(xFrame, "FillStyle")); + awt::Gradient2 aGradient = getProperty<awt::Gradient2>(xFrame, "FillGradient"); + + // MCGR: Use the completely imported transparency gradient to check for correctness + basegfx::BColorStops aColorStops = model::gradient::getColorStopsFromUno(aGradient.ColorStops); + CPPUNIT_ASSERT_EQUAL(size_t(2), aColorStops.size()); + // because of the negative angle, the colors were swapped compared to gradientAngle1.rtf CPPUNIT_ASSERT_EQUAL(awt::GradientStyle_LINEAR, aGradient.Style); CPPUNIT_ASSERT_EQUAL(sal_Int32(8508442), aGradient.StartColor); CPPUNIT_ASSERT_EQUAL(sal_Int32(COL_WHITE), aGradient.EndColor); - // RTF -2° angle (in 1/60,000 degree units = -120,000) == LO 358° (in 1/10 degree unites) - CPPUNIT_ASSERT_EQUAL(sal_Int32(358), sal_Int32(aGradient.Angle / 10)); + // RTF -90° angle (in 1/60,000 degree units) == LO 270° (in 1/10 degree units) + CPPUNIT_ASSERT_EQUAL(sal_Int16(2700), aGradient.Angle); } DECLARE_RTFEXPORT_TEST(testTdf158830, "tdf158830.rtf") diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx index 4f3152b6bc2e..47ffb332bf21 100644 --- a/sw/source/filter/ww8/docxattributeoutput.cxx +++ b/sw/source/filter/ww8/docxattributeoutput.cxx @@ -9639,19 +9639,16 @@ void DocxAttributeOutput::FormatFillGradient( const XFillGradientItem& rFillGrad OString sStartColor = msfilter::util::ConvertColor(Color(rGradient.GetColorStops().front().getStopColor())); OString sEndColor = msfilter::util::ConvertColor(Color(rGradient.GetColorStops().back().getStopColor())); - // Calculate the angle that was originally in the imported DOCX file - // (reverse calculate the angle that was converted in the file - // /oox/source/vml/vmlformatting.cxx :: FillModel::pushToPropMap - // and also in - // /oox/source/drawingml/fillproperties.cxx :: FillProperties::pushToPropMap - sal_Int32 nReverseAngle = toDegrees(4500_deg10 - rGradient.GetAngle()); - nReverseAngle = (270 - nReverseAngle) % 360; - if (nReverseAngle != 0) + const sal_Int32 nAngle = toDegrees(rGradient.GetAngle()); + if (nAngle != 0) AddToAttrList( m_rExport.SdrExporter().getFlyFillAttrList(), - XML_angle, OString::number( nReverseAngle ) ); + XML_angle, OString::number(nAngle)); - OString sColor1 = sStartColor; - OString sColor2 = sEndColor; + // LO does linear gradients top to bottom, while MSO does bottom to top. + // LO does axial gradients inner to outer, while MSO does outer to inner. + // Conclusion: swap start and end colors. + const OString sColor1 = sEndColor; // LO end color is MSO start color + OString sColor2 = sStartColor; // LO start color is MSO end color switch (rGradient.GetGradientStyle()) { @@ -9671,8 +9668,8 @@ void DocxAttributeOutput::FormatFillGradient( const XFillGradientItem& rFillGrad if (basegfx::fTools::more(rStop.getStopOffset(), 0.5)) break; - // the color in the middle is considered the start color for focus 50 - sColor1 = msfilter::util::ConvertColor(Color(rStop.getStopColor())); + // from MSO export perspective, the inner color is the end color + sColor2 = msfilter::util::ConvertColor(Color(rStop.getStopColor())); bIsSymmetrical = true; } } diff --git a/sw/source/filter/ww8/rtfattributeoutput.cxx b/sw/source/filter/ww8/rtfattributeoutput.cxx index 1d28ccabed8b..03fc0fa354cb 100644 --- a/sw/source/filter/ww8/rtfattributeoutput.cxx +++ b/sw/source/filter/ww8/rtfattributeoutput.cxx @@ -3745,29 +3745,22 @@ void RtfAttributeOutput::FormatFillGradient(const XFillGradientItem& rFillGradie // colors as start/end and - when more than two ColorStops are defined - // guess that GradientStyle_AXIAL is used and thus create a "fillFocus" // entry - // NOTE: I also found that loading file from testTextframeGradient - // "textframe-gradient.rtf" and save-as *inverts* the gradient, so I - // exchanged here fillColor/fillBackColor to get the correct order - const Color aStartColor(rColorStops.front().getStopColor()); - m_aFlyProperties.push_back(std::make_pair<OString, OString>( - "fillColor"_ostr, OString::number(wwUtility::RGBToBGR(aStartColor)))); - // LO's angle is 180° different from VML/RTF. Copied docxattribute angle formula here. - sal_Int32 nReverseAngle = toDegrees(4500_deg10 - rGradient.GetAngle()); - nReverseAngle = (270 - nReverseAngle) % 360; - if (nReverseAngle != 0) + // LO does linear gradients top to bottom, while MSO does bottom to top. + // LO does axial gradients inner to outer, while MSO does outer to inner. + // Conclusion: swap start and end colors (and stop emulating this with 180deg rotations). + const Color aMSOStartColor(rColorStops.back().getStopColor()); + Color aMSOEndColor(rColorStops.front().getStopColor()); + + const sal_Int32 nAngle = toDegrees(rGradient.GetAngle()); + if (nAngle != 0) { - m_aFlyProperties.push_back(std::make_pair<OString, OString>( - "fillAngle"_ostr, OString::number(nReverseAngle * 60000))); + m_aFlyProperties.push_back( + std::make_pair<OString, OString>("fillAngle"_ostr, OString::number(nAngle * 60000))); } if (rColorStops.size() < 3) { - // two-color version, use back as 2nd color - const Color aEndColor(rColorStops.back().getStopColor()); - m_aFlyProperties.push_back(std::make_pair<OString, OString>( - "fillBackColor"_ostr, OString::number(wwUtility::RGBToBGR(aEndColor)))); - if (rGradient.GetGradientStyle() == awt::GradientStyle_AXIAL) { m_aFlyProperties.push_back( @@ -3779,12 +3772,16 @@ void RtfAttributeOutput::FormatFillGradient(const XFillGradientItem& rFillGradie // assume what was formally GradientStyle_AXIAL, see above and also refer to // FillModel::pushToPropMap 'fFocus' value and usage. // The 2nd color is the in-between color, use it - const Color aEndColor(rColorStops[1].getStopColor()); - m_aFlyProperties.push_back(std::make_pair<OString, OString>( - "fillBackColor"_ostr, OString::number(wwUtility::RGBToBGR(aEndColor)))); + aMSOEndColor = Color(rColorStops[1].getStopColor()); + m_aFlyProperties.push_back( std::make_pair<OString, OString>("fillFocus"_ostr, OString::number(50))); } + + m_aFlyProperties.push_back(std::make_pair<OString, OString>( + "fillColor"_ostr, OString::number(wwUtility::RGBToBGR(aMSOStartColor)))); + m_aFlyProperties.push_back(std::make_pair<OString, OString>( + "fillBackColor"_ostr, OString::number(wwUtility::RGBToBGR(aMSOEndColor)))); } void RtfAttributeOutput::FormatBox(const SvxBoxItem& rBox)