diff options
author | Justin Luth <jluth@mail.com> | 2024-02-22 09:24:12 -0500 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.com> | 2024-02-26 09:08:29 +0100 |
commit | 8d3f85cf1d1eaa489b468c66dee465f67e0a5fe1 (patch) | |
tree | a335fab3ef9d1b3a2d961261d0eceeed9976ac2e /sw | |
parent | 64398a41f7d4110b58f0496f1237a6778772b05c (diff) |
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 <jluth@mail.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163871
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Diffstat (limited to 'sw')
-rw-r--r-- | sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle.docx (renamed from sw/qa/extras/ooxmlexport/data/tdf126533_negativeAxialAngle.docx) | bin | 12994 -> 12994 bytes | |||
-rw-r--r-- | sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle2.docx | bin | 0 -> 12995 bytes | |||
-rw-r--r-- | sw/qa/extras/ooxmlexport/ooxmlexport7.cxx | 27 | ||||
-rw-r--r-- | sw/qa/extras/rtfexport/data/tdf159824_gradientAngle3.rtf | 17 | ||||
-rw-r--r-- | sw/qa/extras/rtfexport/data/tdf159824_gradientAngle4.rtf | 17 | ||||
-rw-r--r-- | sw/qa/extras/rtfexport/rtfexport8.cxx | 62 | ||||
-rw-r--r-- | sw/source/filter/ww8/docxattributeoutput.cxx | 23 | ||||
-rw-r--r-- | sw/source/filter/ww8/rtfattributeoutput.cxx | 37 |
8 files changed, 141 insertions, 42 deletions
diff --git a/sw/qa/extras/ooxmlexport/data/tdf126533_negativeAxialAngle.docx b/sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle.docx Binary files differindex 98077b51a372..98077b51a372 100644 --- a/sw/qa/extras/ooxmlexport/data/tdf126533_negativeAxialAngle.docx +++ b/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 Binary files differnew file mode 100644 index 000000000000..49261dfef567 --- /dev/null +++ b/sw/qa/extras/ooxmlexport/data/tdf126533_axialAngle2.docx 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 @@ +{\rtf1\ansi\deff3\adeflang1025 + +\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 @@ +{\rtf1\ansi\deff3\adeflang1025 + +\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 334e521aba04..3b5d57fc27b2 100644 --- a/sw/source/filter/ww8/docxattributeoutput.cxx +++ b/sw/source/filter/ww8/docxattributeoutput.cxx @@ -9636,19 +9636,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()) { @@ -9668,8 +9665,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 3b9127fb2c53..de7140f091bc 100644 --- a/sw/source/filter/ww8/rtfattributeoutput.cxx +++ b/sw/source/filter/ww8/rtfattributeoutput.cxx @@ -3734,29 +3734,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( @@ -3768,12 +3761,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) |