svgio/inc/SvgNumber.hxx | 8 ++--
svgio/inc/svgnode.hxx | 7 +--
svgio/inc/svgtspannode.hxx | 2 -
svgio/qa/cppunit/SvgImportTest.cxx | 52 ++++++++++++++++++++++++++
svgio/qa/cppunit/SvgNumberTest.cxx | 4 +-
svgio/qa/cppunit/data/dy_in_ems.svg | 7 +++
svgio/qa/cppunit/data/dy_in_exs.svg | 7 +++
svgio/source/svgreader/SvgNumber.cxx | 5 +-
svgio/source/svgreader/svgnode.cxx | 36 +++---------------
svgio/source/svgreader/svgstyleattributes.cxx | 8 ++--
svgio/source/svgreader/svgtspannode.cxx | 5 --
11 files changed, 88 insertions(+), 53 deletions(-)
New commits:
commit c10fba9c937e4a52bfd1ed13aa813d4faee833fe
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Apr 9 16:04:40 2024 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Apr 22 08:15:12 2024 +0500
tdf#160717: fix ex handling
Same as in commit e27572686130df43d1d65c574b0c34f39fc0d1a9
(tdf#160593: make sure to use current element's font size for em unit,
2024-04-18) for em.
Change-Id: Id9003c0426a6b373456da1aa1550f7ff07f766a3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166235
Tested-by: Mike Kaganski <mike.kagan...@collabora.com>
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/svgio/inc/SvgNumber.hxx b/svgio/inc/SvgNumber.hxx
index fe4ab8e2683e..c62576c2b082 100644
--- a/svgio/inc/SvgNumber.hxx
+++ b/svgio/inc/SvgNumber.hxx
@@ -39,8 +39,8 @@ public:
virtual basegfx::B2DRange getCurrentViewPort() 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;
+ /// return xheight of node, either set here or inherited from parents
+ virtual double getCurrentXHeight() const = 0;
};
enum class SvgUnit
diff --git a/svgio/inc/svgnode.hxx b/svgio/inc/svgnode.hxx
index 5d3a2453d727..d24cdecf3729 100644
--- a/svgio/inc/svgnode.hxx
+++ b/svgio/inc/svgnode.hxx
@@ -157,9 +157,7 @@ namespace svgio::svgreader
/// InfoProvider support for %, em and ex values
virtual basegfx::B2DRange getCurrentViewPort() const override;
virtual double getCurrentFontSize() const override;
- virtual double getCurrentXHeightInherited() const override;
-
- double getCurrentXHeight() const;
+ virtual double getCurrentXHeight() const override;
/// Id access
std::optional<OUString> const & getId() const { return mpId; }
diff --git a/svgio/qa/cppunit/SvgImportTest.cxx
b/svgio/qa/cppunit/SvgImportTest.cxx
index 4d6f7568e2cd..d61c5c761f79 100644
--- a/svgio/qa/cppunit/SvgImportTest.cxx
+++ b/svgio/qa/cppunit/SvgImportTest.cxx
@@ -1148,7 +1148,7 @@ CPPUNIT_TEST_FIXTURE(Test, testDyInEms)
CPPUNIT_TEST_FIXTURE(Test, testExs)
{
- // tdf#160594 given an SVG file with <tspan dy="3ex"
style="font-size:1ex">:
+ // tdf#160594, tdf#160717 given an SVG file with <tspan dy="3ex"
style="font-size:1ex">:
std::u16string_view aPath = u"/svgio/qa/cppunit/data/dy_in_exs.svg";
Primitive2DSequence aSequence = parseSvg(aPath);
drawinglayer::Primitive2dXmlDump aDumper;
@@ -1156,6 +1156,7 @@ CPPUNIT_TEST_FIXTURE(Test, testExs)
assertXPath(pDocument, "//textsimpleportion", 2);
assertXPath(pDocument, "//textsimpleportion[1]", "height", "16");
+ assertXPath(pDocument, "//textsimpleportion[1]", "y", "20");
sal_Int32 nSize = getXPath(pDocument, "//textsimpleportion[2]",
"height").toInt32();
// Without the accompanying fix in place, this test would have failed with:
@@ -1163,6 +1164,17 @@ CPPUNIT_TEST_FIXTURE(Test, testExs)
// - Actual : 16
// i.e. the parent font-size was used, instead of its x-size.
CPPUNIT_ASSERT_LESS(sal_Int32(16), nSize);
+
+ sal_Int32 nYPos = getXPath(pDocument, "//textsimpleportion[2]",
"y").toInt32();
+ // Then make sure that the vertical offset is based on x-size of tspan,
not of its parent.
+ // Given the tspan's font-size is nSize, its x-size is less than nSize,
and the expected
+ // vertical offset is less than 3 * nSize, which means that the resulting
y is expected
+ // to be strictly less than 20 + 3 * nSize.
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected less than: 44
+ // - Actual : 44
+ // i.e. the parent x-size (or current font-size) was used, instead of
current x-size.
+ CPPUNIT_ASSERT_LESS(sal_Int32(20 + 3 * nSize), nYPos);
}
}
diff --git a/svgio/qa/cppunit/SvgNumberTest.cxx
b/svgio/qa/cppunit/SvgNumberTest.cxx
index 9b12e52bf956..49e1394bcef2 100644
--- a/svgio/qa/cppunit/SvgNumberTest.cxx
+++ b/svgio/qa/cppunit/SvgNumberTest.cxx
@@ -40,7 +40,7 @@ public:
double getCurrentFontSize() const override { return 12.0; }
- double getCurrentXHeightInherited() const override { return 5.0; }
+ double getCurrentXHeight() const override { return 5.0; }
};
void TestNumber::testSetting()
diff --git a/svgio/source/svgreader/SvgNumber.cxx
b/svgio/source/svgreader/SvgNumber.cxx
index 4a48ffbfb4e9..72a44dbdd032 100644
--- a/svgio/source/svgreader/SvgNumber.cxx
+++ b/svgio/source/svgreader/SvgNumber.cxx
@@ -39,7 +39,7 @@ double SvgNumber::solveNonPercentage(const InfoProvider&
rInfoProvider) const
case SvgUnit::em:
return mfNumber * rInfoProvider.getCurrentFontSize();
case SvgUnit::ex:
- return mfNumber * rInfoProvider.getCurrentXHeightInherited() * 0.5;
+ return mfNumber * rInfoProvider.getCurrentXHeight();
case SvgUnit::px:
return mfNumber;
case SvgUnit::pt:
diff --git a/svgio/source/svgreader/svgnode.cxx
b/svgio/source/svgreader/svgnode.cxx
index ec2e8e0589a9..5e3c37b07bb7 100644
--- a/svgio/source/svgreader/svgnode.cxx
+++ b/svgio/source/svgreader/svgnode.cxx
@@ -609,25 +609,12 @@ namespace svgio::svgreader
return 0.0;
}
- double SvgNode::getCurrentXHeightInherited() const
- {
- if(getParent())
- {
- return getParent()->getCurrentXHeight();
- }
- else
- {
- return 0.0;
- }
- }
-
double SvgNode::getCurrentXHeight() const
{
- if(getSvgStyleAttributes())
- // for XHeight, use FontSize currently
- return
getSvgStyleAttributes()->getFontSizeNumber().solve(*this,
NumberType::ycoordinate);
-
- return getCurrentXHeightInherited();
+ // https://drafts.csswg.org/css-values-4/#ex
+ // for XHeight, use 0.5em fallback currently
+ // FIXME: use "x-height of the first available font"
+ return getCurrentFontSize() * 0.5;
}
void SvgNode::setId(OUString const & rId)
commit 28ff1b05047ef3a19cf692527a23cb38768c9764
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Apr 9 13:56:13 2024 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Sat Apr 20 17:35:25 2024 +0500
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>
diff --git a/svgio/qa/cppunit/SvgImportTest.cxx
b/svgio/qa/cppunit/SvgImportTest.cxx
index 995f705fb653..4d6f7568e2cd 100644
--- a/svgio/qa/cppunit/SvgImportTest.cxx
+++ b/svgio/qa/cppunit/SvgImportTest.cxx
@@ -1146,6 +1146,25 @@ CPPUNIT_TEST_FIXTURE(Test, testDyInEms)
assertXPath(pDocument, "//textsimpleportion[2]", "y", "32");
}
+CPPUNIT_TEST_FIXTURE(Test, testExs)
+{
+ // tdf#160594 given an SVG file with <tspan dy="3ex"
style="font-size:1ex">:
+ std::u16string_view aPath = u"/svgio/qa/cppunit/data/dy_in_exs.svg";
+ Primitive2DSequence aSequence = parseSvg(aPath);
+ drawinglayer::Primitive2dXmlDump aDumper;
+ xmlDocUniquePtr pDocument =
aDumper.dumpAndParse(Primitive2DContainer(aSequence));
+
+ assertXPath(pDocument, "//textsimpleportion", 2);
+ assertXPath(pDocument, "//textsimpleportion[1]", "height", "16");
+
+ sal_Int32 nSize = getXPath(pDocument, "//textsimpleportion[2]",
"height").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();
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 6c42fbe744f9..0a656e2a1ad9 100644
--- a/svgio/source/svgreader/svgstyleattributes.cxx
+++ b/svgio/source/svgreader/svgstyleattributes.cxx
@@ -2562,11 +2562,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 fdb48ffa8341d015d127e1c99c4be3072199dc67
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Apr 9 13:03:07 2024 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Sat Apr 20 16:15:29 2024 +0500
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>
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 4bdbd3046d75..5d3a2453d727 100644
--- a/svgio/inc/svgnode.hxx
+++ b/svgio/inc/svgnode.hxx
@@ -156,10 +156,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 10a7b7ee16a9..ff53238a3488 100644
--- a/svgio/inc/svgtspannode.hxx
+++ b/svgio/inc/svgtspannode.hxx
@@ -42,8 +42,6 @@ namespace svgio::svgreader
virtual const SvgStyleAttributes* getSvgStyleAttributes() const
override;
virtual void parseAttribute(const OUString& rTokenName, SVGToken
aSVGToken, const OUString& aContent) override;
- double getCurrentFontSize() const;
-
/// access to SvgTextPositions
const SvgTextPositions& getSvgTextPositions() const { return
maSvgTextPositions; }
};
diff --git a/svgio/qa/cppunit/SvgImportTest.cxx
b/svgio/qa/cppunit/SvgImportTest.cxx
index 67793f0db3f1..995f705fb653 100644
--- a/svgio/qa/cppunit/SvgImportTest.cxx
+++ b/svgio/qa/cppunit/SvgImportTest.cxx
@@ -1125,6 +1125,27 @@ 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">:
+ std::u16string_view aPath = u"/svgio/qa/cppunit/data/dy_in_ems.svg";
+ Primitive2DSequence aSequence = parseSvg(aPath);
+ drawinglayer::Primitive2dXmlDump aDumper;
+ xmlDocUniquePtr pDocument =
aDumper.dumpAndParse(Primitive2DContainer(aSequence));
+
+ assertXPath(pDocument, "//textsimpleportion", 2);
+ assertXPath(pDocument, "//textsimpleportion[1]", "y", "20");
+ // 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]", "y", "32");
+}
+
}
CPPUNIT_PLUGIN_IMPLEMENT();
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 d45624d3edc6..ec2e8e0589a9 100644
--- a/svgio/source/svgreader/svgnode.cxx
+++ b/svgio/source/svgreader/svgnode.cxx
@@ -598,24 +598,15 @@ namespace svgio::svgreader
}
}
- 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 db024e2f2ff7..5e19e88dc137 100644
--- a/svgio/source/svgreader/svgtspannode.cxx
+++ b/svgio/source/svgreader/svgtspannode.cxx
@@ -65,11 +65,6 @@ namespace svgio::svgreader
}
}
- double SvgTspanNode::getCurrentFontSize() const
- {
- return getCurrentFontSizeInherited();
- }
-
} // end of namespace svgio::svgreader
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
