From 7e6dac4edce063a766497ecb498e293bf4e16e66 Mon Sep 17 00:00:00 2001 From: Tomaž Vajngerl Date: Sun, 13 May 2018 20:21:17 +0900 Subject: svgio: fix rendering when the width/height isn't present in SVG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The general size of the image should be the same when width/height attributes are present. It is very wrong to assume the size of the image is the area covered by all the primitives in the image. Change-Id: I56f241e84dee37796f9804ce2569c4eb416e83a0 Reviewed-on: https://gerrit.libreoffice.org/54191 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl --- svgio/qa/cppunit/SvgImportTest.cxx | 66 ++++++++++++++++++++++- svgio/qa/cppunit/data/Drawing_NoWidthHeight.svg | 12 +++++ svgio/qa/cppunit/data/Drawing_WithWidthHeight.svg | 14 +++++ svgio/source/svgreader/svgsvgnode.cxx | 51 +++++++++++------- 4 files changed, 124 insertions(+), 19 deletions(-) create mode 100644 svgio/qa/cppunit/data/Drawing_NoWidthHeight.svg create mode 100644 svgio/qa/cppunit/data/Drawing_WithWidthHeight.svg diff --git a/svgio/qa/cppunit/SvgImportTest.cxx b/svgio/qa/cppunit/SvgImportTest.cxx index a83ec4aae189..6b71a0dfeb7a 100644 --- a/svgio/qa/cppunit/SvgImportTest.cxx +++ b/svgio/qa/cppunit/SvgImportTest.cxx @@ -27,11 +27,13 @@ namespace { +using namespace css; using namespace css::uno; using namespace css::io; using namespace css::graphic; using drawinglayer::primitive2d::Primitive2DSequence; using drawinglayer::primitive2d::Primitive2DContainer; +using drawinglayer::primitive2d::Primitive2DReference; class Test : public test::BootstrapFixture, public XmlTestTools { @@ -61,6 +63,7 @@ class Test : public test::BootstrapFixture, public XmlTestTools void testMaskText(); void testTdf99994(); void testTdf101237(); + void testBehaviourWhenWidthAndHeightIsOrIsNotSet(); Primitive2DSequence parseSvg(const OUString& aSource); @@ -90,6 +93,7 @@ public: CPPUNIT_TEST(testMaskText); CPPUNIT_TEST(testTdf99994); CPPUNIT_TEST(testTdf101237); + CPPUNIT_TEST(testBehaviourWhenWidthAndHeightIsOrIsNotSet); CPPUNIT_TEST_SUITE_END(); }; @@ -132,7 +136,6 @@ void Test::checkRectPrimitive(Primitive2DSequence const & rPrimitive) } - bool arePrimitive2DSequencesEqual(const Primitive2DSequence& rA, const Primitive2DSequence& rB) { const sal_Int32 nCount(rA.getLength()); @@ -629,7 +632,68 @@ void Test::testTdf101237() assertXPath(pDocument, "/primitive2D/transform/polypolygoncolor", "color", "#ff0000"); assertXPath(pDocument, "/primitive2D/transform/polypolygonstroke/line", "color", "#000000"); assertXPath(pDocument, "/primitive2D/transform/polypolygonstroke/line", "width", "5"); +} + +void Test::testBehaviourWhenWidthAndHeightIsOrIsNotSet() +{ + // This test checks the behaviour when width and height attributes + // are and are not set. In both cases the result must be the same, + // however if the width / height are set, then the size of the image + // is enforced, but this isn't really possible in LibreOffice (or + // maybe we could lock the size in this case). + // The behaviour in browsers is that when a SVG image has width / height + // attributes set, then the image is shown with that size, but if it + // isn't set then it is shown as scalable image which is the size of + // the container. + + { + Primitive2DSequence aSequence = parseSvg("svgio/qa/cppunit/data/Drawing_WithWidthHeight.svg"); + CPPUNIT_ASSERT(aSequence.hasElements()); + + geometry::RealRectangle2D aRealRect; + basegfx::B2DRange aRange; + uno::Sequence aViewParameters; + + for (Primitive2DReference const & xReference : aSequence) + { + if (xReference.is()) + { + aRealRect = xReference->getRange(aViewParameters); + aRange.expand(basegfx::B2DRange(aRealRect.X1, aRealRect.Y1, aRealRect.X2, aRealRect.Y2)); + } + } + + double fWidth = (aRange.getWidth() / 2540.0) * 96.0; + double fHeight = (aRange.getHeight() / 2540.0) * 96.0; + + CPPUNIT_ASSERT_DOUBLES_EQUAL(11.0, fWidth, 1E-12); + CPPUNIT_ASSERT_DOUBLES_EQUAL(11.0, fHeight, 1E-12); + } + + { + Primitive2DSequence aSequence = parseSvg("svgio/qa/cppunit/data/Drawing_NoWidthHeight.svg"); + CPPUNIT_ASSERT(aSequence.hasElements()); + + geometry::RealRectangle2D aRealRect; + basegfx::B2DRange aRange; + uno::Sequence aViewParameters; + + for (Primitive2DReference const & xReference : aSequence) + { + if (xReference.is()) + { + aRealRect = xReference->getRange(aViewParameters); + aRange.expand(basegfx::B2DRange(aRealRect.X1, aRealRect.Y1, aRealRect.X2, aRealRect.Y2)); + } + } + + double fWidth = (aRange.getWidth() / 2540.0) * 96.0; + double fHeight = (aRange.getHeight() / 2540.0) * 96.0; + + CPPUNIT_ASSERT_DOUBLES_EQUAL(11.0, fWidth, 1E-12); + CPPUNIT_ASSERT_DOUBLES_EQUAL(11.0, fHeight, 1E-12); + } } CPPUNIT_TEST_SUITE_REGISTRATION(Test); diff --git a/svgio/qa/cppunit/data/Drawing_NoWidthHeight.svg b/svgio/qa/cppunit/data/Drawing_NoWidthHeight.svg new file mode 100644 index 000000000000..59520d6ab9bc --- /dev/null +++ b/svgio/qa/cppunit/data/Drawing_NoWidthHeight.svg @@ -0,0 +1,12 @@ + + + + diff --git a/svgio/qa/cppunit/data/Drawing_WithWidthHeight.svg b/svgio/qa/cppunit/data/Drawing_WithWidthHeight.svg new file mode 100644 index 000000000000..bc5afb553e43 --- /dev/null +++ b/svgio/qa/cppunit/data/Drawing_WithWidthHeight.svg @@ -0,0 +1,14 @@ + + + + diff --git a/svgio/source/svgreader/svgsvgnode.cxx b/svgio/source/svgreader/svgsvgnode.cxx index 5803c877331b..40dfaca1a402 100644 --- a/svgio/source/svgreader/svgsvgnode.cxx +++ b/svgio/source/svgreader/svgsvgnode.cxx @@ -350,13 +350,20 @@ namespace svgio seekReferenceWidth(fWReference, bHasFoundWidth); if (!bHasFoundWidth) { - // Even outermost svg has not all information to resolve relative values, - // I use content itself as fallback to set missing values for viewport - // Any better idea for such ill structured svg documents? - const basegfx::B2DRange aChildRange( - aSequence.getB2DRange( - drawinglayer::geometry::ViewInformation2D())); - fWReference = aChildRange.getWidth(); + if (getViewBox()) + { + fWReference = getViewBox()->getWidth(); + } + else + { + // Even outermost svg has not all information to resolve relative values, + // I use content itself as fallback to set missing values for viewport + // Any better idea for such ill structured svg documents? + const basegfx::B2DRange aChildRange( + aSequence.getB2DRange( + drawinglayer::geometry::ViewInformation2D())); + fWReference = aChildRange.getWidth(); + } } // referenced values are already in 'user unit' if (!bXIsAbsolute) @@ -377,13 +384,20 @@ namespace svgio seekReferenceHeight(fHReference, bHasFoundHeight); if (!bHasFoundHeight) { + if (getViewBox()) + { + fHReference = getViewBox()->getHeight(); + } + else + { // Even outermost svg has not all information to resolve relative values, - // I use content itself as fallback to set missing values for viewport - // Any better idea for such ill structured svg documents? - const basegfx::B2DRange aChildRange( - aSequence.getB2DRange( - drawinglayer::geometry::ViewInformation2D())); - fHReference = aChildRange.getHeight(); + // I use content itself as fallback to set missing values for viewport + // Any better idea for such ill structured svg documents? + const basegfx::B2DRange aChildRange( + aSequence.getB2DRange( + drawinglayer::geometry::ViewInformation2D())); + fHReference = aChildRange.getHeight(); + } } // referenced values are already in 'user unit' @@ -527,13 +541,15 @@ namespace svgio // by the viewBox. No mapping. // We get viewport >= content, therefore no clipping. bNeedsMapping = false; + const basegfx::B2DRange aChildRange( aSequence.getB2DRange( drawinglayer::geometry::ViewInformation2D())); - const double fChildWidth(aChildRange.getWidth()); - const double fChildHeight(aChildRange.getHeight()); - const double fLeft(aChildRange.getMinX()); - const double fTop(aChildRange.getMinY()); + + const double fChildWidth(getViewBox() ? getViewBox()->getWidth() : aChildRange.getWidth()); + const double fChildHeight(getViewBox() ? getViewBox()->getHeight() : aChildRange.getHeight()); + const double fLeft(getViewBox() ? getViewBox()->getMinX() : aChildRange.getMinX()); + const double fTop (getViewBox() ? getViewBox()->getMinY() : aChildRange.getMinY()); if ( fChildWidth / fViewBoxWidth > fChildHeight / fViewBoxHeight ) { // expand y fW = fChildWidth; @@ -547,7 +563,6 @@ namespace svgio aSvgCanvasRange = basegfx::B2DRange(fLeft, fTop, fLeft + fW, fTop + fH); } - if (bNeedsMapping) { // create mapping -- cgit