svgio/inc/SvgNumber.hxx | 4 +-- svgio/inc/svgnode.hxx | 3 -- svgio/inc/svgtspannode.hxx | 2 - svgio/qa/cppunit/SvgImportTest.cxx | 34 ++++++++++++++++++++++++++ svgio/qa/cppunit/SvgNumberTest.cxx | 2 - svgio/qa/cppunit/data/dy_in_ems.svg | 7 +++++ svgio/qa/cppunit/data/dy_in_exs.svg | 7 +++++ svgio/source/svgreader/SvgNumber.cxx | 3 +- svgio/source/svgreader/svgnode.cxx | 17 +++---------- svgio/source/svgreader/svgstyleattributes.cxx | 8 +++--- svgio/source/svgreader/svgtspannode.cxx | 5 --- 11 files changed, 62 insertions(+), 30 deletions(-)
New commits: commit 97586e0b4712e44e360e6572af8483d81ac2346a Author: Mike Kaganski <mike.kagan...@collabora.com> AuthorDate: Tue Apr 9 13:56:13 2024 +0500 Commit: Xisco Fauli <xiscofa...@libreoffice.org> CommitDate: Fri Apr 19 09:09:17 2024 +0200 tdf#160594: Use the recommended fallback of 0.5em for ex in font-size This fixes the error of identical treatment of em and ex in font-size, which violated https://drafts.csswg.org/css-values-4/#font-relative-length. The fix uses the fallback of 0.5em for ex, similar to the code used in SvgNumber::solveNonPercentage. A follow-up should implement the correct use of "x-height of the first available font". Change-Id: Id9d581994e158d629d9752299ad93ac7e9fe4cad Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166234 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166261 diff --git a/svgio/qa/cppunit/SvgImportTest.cxx b/svgio/qa/cppunit/SvgImportTest.cxx index 099f1d199406..cc25dec16233 100644 --- a/svgio/qa/cppunit/SvgImportTest.cxx +++ b/svgio/qa/cppunit/SvgImportTest.cxx @@ -2085,6 +2085,22 @@ CPPUNIT_TEST_FIXTURE(Test, testDyInEms) assertXPath(pDocument, "//textsimpleportion[2]"_ostr, "y"_ostr, u"32"_ustr); } +CPPUNIT_TEST_FIXTURE(Test, testExs) +{ + // tdf#160594 given an SVG file with <tspan dy="3ex" style="font-size:1ex">: + xmlDocUniquePtr pDocument = dumpAndParseSvg(u"/svgio/qa/cppunit/data/dy_in_exs.svg"); + + assertXPath(pDocument, "//textsimpleportion"_ostr, 2); + assertXPath(pDocument, "//textsimpleportion[1]"_ostr, "height"_ostr, u"16"_ustr); + + sal_Int32 nSize = getXPath(pDocument, "//textsimpleportion[2]"_ostr, "height"_ostr).toInt32(); + // Without the accompanying fix in place, this test would have failed with: + // - Expected less than: 16 + // - Actual : 16 + // i.e. the parent font-size was used, instead of its x-size. + CPPUNIT_ASSERT_LESS(sal_Int32(16), nSize); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/svgio/qa/cppunit/data/dy_in_exs.svg b/svgio/qa/cppunit/data/dy_in_exs.svg new file mode 100644 index 000000000000..816a64a4586c --- /dev/null +++ b/svgio/qa/cppunit/data/dy_in_exs.svg @@ -0,0 +1,7 @@ +<?xml version="1.0" encoding="UTF-8" standalone="no"?> + +<svg xmlns="http://www.w3.org/2000/svg" width="1in" height="1in" viewBox="0 0 100% 100%"> + + <text x="5" y="20" style="font-size:16px;font-family:Liberation Sans">foo + <tspan x="5" dy="3ex" style="font-size:1ex">bar</tspan></text> +</svg> \ No newline at end of file diff --git a/svgio/source/svgreader/svgstyleattributes.cxx b/svgio/source/svgreader/svgstyleattributes.cxx index 58bdb9add84b..50f72199e40b 100644 --- a/svgio/source/svgreader/svgstyleattributes.cxx +++ b/svgio/source/svgreader/svgstyleattributes.cxx @@ -2642,11 +2642,11 @@ namespace svgio::svgreader if(pSvgStyleAttributes) { const SvgNumber aParentNumber = pSvgStyleAttributes->getFontSizeNumber(); + double n = aParentNumber.getNumber() * maFontSizeNumber.getNumber(); + if (SvgUnit::ex == maFontSizeNumber.getUnit()) + n *= 0.5; // FIXME: use "x-height of the first available font" - return SvgNumber( - aParentNumber.getNumber() * maFontSizeNumber.getNumber(), - aParentNumber.getUnit(), - true); + return SvgNumber(n, aParentNumber.getUnit()); } } commit 334446935b194ebdadb10004c01bff550f09838e Author: Mike Kaganski <mike.kagan...@collabora.com> AuthorDate: Tue Apr 9 13:03:07 2024 +0500 Commit: Xisco Fauli <xiscofa...@libreoffice.org> CommitDate: Fri Apr 19 09:09:09 2024 +0200 tdf#160593: make sure to use current element's font size for em unit According to https://drafts.csswg.org/css-values-4/#font-relative-length em is "equal to the computed value of the font-size property of the element on which it is used". This means, that for an element that defines its own font-size, attributes like 'dy' using em refer to the new font-size, not to inherited font-size. Change-Id: Ie5a013df99a68edddf466e4c0ee5311f6219fcb2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166233 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166260 diff --git a/svgio/inc/SvgNumber.hxx b/svgio/inc/SvgNumber.hxx index 4d03335cf424..fe4ab8e2683e 100644 --- a/svgio/inc/SvgNumber.hxx +++ b/svgio/inc/SvgNumber.hxx @@ -37,8 +37,8 @@ class InfoProvider public: virtual ~InfoProvider() {} virtual basegfx::B2DRange getCurrentViewPort() const = 0; - /// return font size of node inherited from parents - virtual double getCurrentFontSizeInherited() const = 0; + /// return font size of node, either set here or inherited from parents + virtual double getCurrentFontSize() const = 0; /// return xheight of node inherited from parents virtual double getCurrentXHeightInherited() const = 0; }; diff --git a/svgio/inc/svgnode.hxx b/svgio/inc/svgnode.hxx index 16c1f50bc3db..98a444cba5fc 100644 --- a/svgio/inc/svgnode.hxx +++ b/svgio/inc/svgnode.hxx @@ -163,10 +163,9 @@ namespace svgio::svgreader /// InfoProvider support for %, em and ex values virtual basegfx::B2DRange getCurrentViewPort() const override; - virtual double getCurrentFontSizeInherited() const override; + virtual double getCurrentFontSize() const override; virtual double getCurrentXHeightInherited() const override; - double getCurrentFontSize() const; double getCurrentXHeight() const; /// Id access diff --git a/svgio/inc/svgtspannode.hxx b/svgio/inc/svgtspannode.hxx index 84033685d8f9..0740c3cece24 100644 --- a/svgio/inc/svgtspannode.hxx +++ b/svgio/inc/svgtspannode.hxx @@ -53,8 +53,6 @@ namespace svgio::svgreader virtual const SvgStyleAttributes* getSvgStyleAttributes() const override; virtual void parseAttribute(SVGToken aSVGToken, const OUString& aContent) override; - double getCurrentFontSize() const; - /// X content const SvgNumberVector& getX() const { return maX; } void setX(SvgNumberVector&& aX) { maX = std::move(aX); } diff --git a/svgio/qa/cppunit/SvgImportTest.cxx b/svgio/qa/cppunit/SvgImportTest.cxx index 6969dc406a0e..099f1d199406 100644 --- a/svgio/qa/cppunit/SvgImportTest.cxx +++ b/svgio/qa/cppunit/SvgImportTest.cxx @@ -2067,6 +2067,24 @@ CPPUNIT_TEST_FIXTURE(Test, testTspanFillOpacity) CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(70), nTransparence); } +CPPUNIT_TEST_FIXTURE(Test, testDyInEms) +{ + // tdf#160593 given an SVG file with <tspan dy="1.5em" style="font-size:0.5em">: + xmlDocUniquePtr pDocument = dumpAndParseSvg(u"/svgio/qa/cppunit/data/dy_in_ems.svg"); + + assertXPath(pDocument, "//textsimpleportion"_ostr, 2); + assertXPath(pDocument, "//textsimpleportion[1]"_ostr, "y"_ostr, u"20"_ustr); + // Then make sure that the vertical offset is based on font-size of tspan, not of its parent. + // Given the parent's font-size is 16 px, the expected vertical offset is 1.5 * (16 * 0.5) = 12, + // which means that the resulting y is expected to be 20 + 12 = 32. + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 32 + // - Actual : 44 + // i.e. the offset was calculated as 1.5 multiplied by the parent's font-size of 16 px, + // not by the current tspan's half font-size. + assertXPath(pDocument, "//textsimpleportion[2]"_ostr, "y"_ostr, u"32"_ustr); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/svgio/qa/cppunit/SvgNumberTest.cxx b/svgio/qa/cppunit/SvgNumberTest.cxx index f420a44b42fe..9b12e52bf956 100644 --- a/svgio/qa/cppunit/SvgNumberTest.cxx +++ b/svgio/qa/cppunit/SvgNumberTest.cxx @@ -38,7 +38,7 @@ public: return basegfx::B2DRange(0.0, 0.0, 0.0, 0.0); } - double getCurrentFontSizeInherited() const override { return 12.0; } + double getCurrentFontSize() const override { return 12.0; } double getCurrentXHeightInherited() const override { return 5.0; } }; diff --git a/svgio/qa/cppunit/data/dy_in_ems.svg b/svgio/qa/cppunit/data/dy_in_ems.svg new file mode 100644 index 000000000000..316239edf778 --- /dev/null +++ b/svgio/qa/cppunit/data/dy_in_ems.svg @@ -0,0 +1,7 @@ +<?xml version="1.0" encoding="UTF-8" standalone="no"?> + +<svg xmlns="http://www.w3.org/2000/svg" width="1in" height="1in" viewBox="0 0 100% 100%"> + + <text x="5" y="20" style="font-size:16px;font-family:Liberation Sans">foo + <tspan x="5" dy="1.5em" style="font-size:0.5em">bar</tspan></text> +</svg> \ No newline at end of file diff --git a/svgio/source/svgreader/SvgNumber.cxx b/svgio/source/svgreader/SvgNumber.cxx index c1558f3e6451..4a48ffbfb4e9 100644 --- a/svgio/source/svgreader/SvgNumber.cxx +++ b/svgio/source/svgreader/SvgNumber.cxx @@ -35,8 +35,9 @@ double SvgNumber::solveNonPercentage(const InfoProvider& rInfoProvider) const switch (meUnit) { + // See https://drafts.csswg.org/css-values-4/#font-relative-length case SvgUnit::em: - return mfNumber * rInfoProvider.getCurrentFontSizeInherited(); + return mfNumber * rInfoProvider.getCurrentFontSize(); case SvgUnit::ex: return mfNumber * rInfoProvider.getCurrentXHeightInherited() * 0.5; case SvgUnit::px: diff --git a/svgio/source/svgreader/svgnode.cxx b/svgio/source/svgreader/svgnode.cxx index 7a45c681f19c..b881830a77a3 100644 --- a/svgio/source/svgreader/svgnode.cxx +++ b/svgio/source/svgreader/svgnode.cxx @@ -691,24 +691,15 @@ namespace { } } - double SvgNode::getCurrentFontSizeInherited() const - { - if(getParent()) - { - return getParent()->getCurrentFontSize(); - } - else - { - return 0.0; - } - } - double SvgNode::getCurrentFontSize() const { if(getSvgStyleAttributes()) return getSvgStyleAttributes()->getFontSizeNumber().solve(*this, NumberType::xcoordinate); - return getCurrentFontSizeInherited(); + if(getParent()) + return getParent()->getCurrentFontSize(); + + return 0.0; } double SvgNode::getCurrentXHeightInherited() const diff --git a/svgio/source/svgreader/svgtspannode.cxx b/svgio/source/svgreader/svgtspannode.cxx index 27d714e66a85..4e97c2bfc4cc 100644 --- a/svgio/source/svgreader/svgtspannode.cxx +++ b/svgio/source/svgreader/svgtspannode.cxx @@ -141,11 +141,6 @@ namespace svgio::svgreader } } - double SvgTspanNode::getCurrentFontSize() const - { - return getCurrentFontSizeInherited(); - } - } // end of namespace svgio::svgreader /* vim:set shiftwidth=4 softtabstop=4 expandtab: */