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: */

Reply via email to