[Libreoffice-commits] core.git: emfio/qa emfio/source
Hossein (via logerrit)
logerrit at kemper.freedesktop.org
Fri Sep 3 07:38:10 UTC 2021
emfio/qa/cppunit/emf/EmfImportTest.cxx | 10 +--
emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf |binary
emfio/qa/cppunit/wmf/wmfimporttest.cxx | 63 +++++++++++++++----
emfio/source/reader/wmfreader.cxx | 7 --
4 files changed, 61 insertions(+), 19 deletions(-)
New commits:
commit d25906087918c085239aac30fd72cb65aa7b9eb4
Author: Hossein <hossein at libreoffice.org>
AuthorDate: Thu Sep 2 18:29:08 2021 +0200
Commit: Miklos Vajna <vmiklos at collabora.com>
CommitDate: Fri Sep 3 09:37:37 2021 +0200
tdf#88163 Fix WMF font size
The problems in tdf#88163 can be categorized into two parts:
1. A regression which is caused by the commit
a9020e461803964a206d5551884b70717eed165c. The font size for text is
wrongly calculated for wmf files WITHOUT the placeable header.
2. A long-lasting bug that has existed from LO 3.5 (tested with
various LibreOffice versions from 3.5 to master). The font size for
text is wrongly calculated for wmf files WITH the placeable header.
This patch solves the first part. In this patch the way wmf is scaled
is changed using mnUnitsPerInch as a variable that controls the scale.
The previous method was dividing the left/right/top/bottom fields of
aPlaceableBound which caused the bad scaling of text.
The second problem still remains, and possibly can be solved by usign
the old scaling method that was previously used here (dividing pos
values of aPlaceableBound), but the scaling factor should be
calculated, which varies in different wmf files. More study should be
done to solve this part.
Values used for the example "visio_import_source.wmf" used in the test
WmfTest::testNonPlaceableWmf() are slightly changed, but the rendering
should not noticeably change, except the fix for the text font size.
A new test WmfTest::testTdf88163NonPlaceableWmf() is added which
checks for both font height and text boxes' positions. Font height
can depend on the exact font that is used for Roman in the platform,
so it vary between Linux, Windows and macOS. Thus, a range is used
for ensuring that the font height is correct.
By visual inspection, it is confirmed that this fix does not affect
fdo#74336 and the fix for it still works for attachment 93188.
Change-Id: I55bf38ba4345a0aa48c3e522976a2a5c32f7ec8c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121272
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
diff --git a/emfio/qa/cppunit/emf/EmfImportTest.cxx b/emfio/qa/cppunit/emf/EmfImportTest.cxx
index 1d5d68cf28f8..7fc03e2262a2 100644
--- a/emfio/qa/cppunit/emf/EmfImportTest.cxx
+++ b/emfio/qa/cppunit/emf/EmfImportTest.cxx
@@ -921,17 +921,19 @@ void Test::TestExtTextOutOpaqueAndClipWMF()
// On some operating systems (Linux on LO Jenkins CI), the `/mask/` string is not added to XPath
// As a result tests are failing. On my Ubuntu 20.04 the `/mask/` string was added
// I would leave this test case for macOS to make sure there is no regression at least on one platform.
+
+ // These values come from the fix for tdf#88163
assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor", 5);
assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[1]/polypolygon", "path",
- "m7219 1825h319v3608h-319z");
+ "m7257 1836h320v3628h-320z");
assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[1]", "color", "#ff0000");
assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[2]/polypolygon", "path",
- "m7219 5942h319v318h-319z");
+ "m7257 5976h320v321h-320z");
assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[2]", "color", "#00ff00");
assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[3]/polypolygon", "path",
- "m10149 5942h319v318h-319z");
+ "m10203 5976h320v321h-320z");
assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[3]", "color", "#8080ff");
assertXPath(pDocument, aXPathPrefix + "mask/group", 5);
@@ -948,7 +950,7 @@ void Test::TestExtTextOutOpaqueAndClipWMF()
assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/polypolygoncolor", "color",
"#ff8000");
assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/polypolygoncolor/polypolygon",
- "path", "m1062 1061h1270v473h-1270z");
+ "path", "m1067 1067h1270v473h-1270z");
assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/textsimpleportion", "text",
"OOOO");
assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/textsimpleportion", "fontcolor",
diff --git a/emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf b/emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf
new file mode 100644
index 000000000000..b39514bd1b5c
Binary files /dev/null and b/emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf differ
diff --git a/emfio/qa/cppunit/wmf/wmfimporttest.cxx b/emfio/qa/cppunit/wmf/wmfimporttest.cxx
index 62a669fe3676..00da6f14ad84 100644
--- a/emfio/qa/cppunit/wmf/wmfimporttest.cxx
+++ b/emfio/qa/cppunit/wmf/wmfimporttest.cxx
@@ -46,6 +46,7 @@ public:
}
void testNonPlaceableWmf();
+ void testTdf88163NonPlaceableWmf();
void testSine();
void testEmfProblem();
void testEmfLineStyles();
@@ -57,6 +58,7 @@ public:
CPPUNIT_TEST_SUITE(WmfTest);
CPPUNIT_TEST(testNonPlaceableWmf);
+ CPPUNIT_TEST(testTdf88163NonPlaceableWmf);
CPPUNIT_TEST(testSine);
CPPUNIT_TEST(testEmfProblem);
CPPUNIT_TEST(testEmfLineStyles);
@@ -77,24 +79,65 @@ void WmfTest::testNonPlaceableWmf()
MetafileXmlDump dumper;
dumper.filterAllActionTypes();
dumper.filterActionType(MetaActionType::POLYLINE, false);
+
xmlDocUniquePtr pDoc = dumpAndParse(dumper, aGDIMetaFile);
CPPUNIT_ASSERT(pDoc);
- assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "x", "16798");
- assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "y", "1003");
+ // These values come from changes done in tdf#88163
+ assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "x", "16813");
+ assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "y", "1004");
+
+ assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "x", "16813");
+ assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "y", "7514");
+
+ assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "x", "26112");
+ assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "y", "7514");
+
+ assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "x", "26112");
+ assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "y", "1004");
+
+ assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "x", "16813");
+ assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "y", "1004");
+}
- assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "x", "16798");
- assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "y", "7507");
+void WmfTest::testTdf88163NonPlaceableWmf()
+{
+ OUString fileName(u"tdf88163-non-placeable.wmf");
+ SvFileStream aFileStream(getFullUrl(fileName), StreamMode::READ);
+ GDIMetaFile aGDIMetaFile;
+ ReadWindowMetafile(aFileStream, aGDIMetaFile);
- assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "x", "26090");
- assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "y", "7507");
+ MetafileXmlDump dumper;
+ xmlDocUniquePtr pDoc = dumpAndParse(dumper, aGDIMetaFile);
- assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "x", "26090");
- assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "y", "1003");
+ CPPUNIT_ASSERT(pDoc);
- assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "x", "16798");
- assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "y", "1003");
+ // These values come from the fix for tdf#88163
+
+ // Font 'Roman' and its height can vary according to the platform
+ // Fails without the fix
+ // Linux: With fix: 3136, without fix: ~ 8000
+ // Mac: With fix: 3230, without fix: ~ 8000
+ // Windows: With fix: 3303, without fix: ~ 8000
+ auto x = getXPath(pDoc, "/metafile/push[2]/font[1]", "height");
+ CPPUNIT_ASSERT_MESSAGE(fileName.toUtf8().getStr(), x.toInt32() > 3000);
+ CPPUNIT_ASSERT_MESSAGE(fileName.toUtf8().getStr(), x.toInt32() < 3500);
+
+ // Fails without the fix: Expected: 7359, Actual: 7336
+ assertXPath(pDoc, "/metafile/push[2]/textarray[1]", "x", "7359");
+ // Fails without the fix: Expected: 4118, Actual: 4104
+ assertXPath(pDoc, "/metafile/push[2]/textarray[1]", "y", "4118");
+
+ // Fails without the fix: Expected: 5989, Actual: 5971
+ assertXPath(pDoc, "/metafile/push[2]/textarray[2]", "x", "5989");
+ // Fails without the fix: Expected: 16264, Actual: 16208
+ assertXPath(pDoc, "/metafile/push[2]/textarray[2]", "y", "16264");
+
+ // Fails without the fix: Expected: 20769, Actual: 20705
+ assertXPath(pDoc, "/metafile/push[2]/textarray[3]", "x", "20769");
+ // Fails without the fix: Expected: 4077, Actual: 4062
+ assertXPath(pDoc, "/metafile/push[2]/textarray[3]", "y", "4077");
}
void WmfTest::testSine()
diff --git a/emfio/source/reader/wmfreader.cxx b/emfio/source/reader/wmfreader.cxx
index 82c6210acbbf..62f03fc8df25 100644
--- a/emfio/source/reader/wmfreader.cxx
+++ b/emfio/source/reader/wmfreader.cxx
@@ -1470,11 +1470,8 @@ namespace emfio
const double fMaxWidth = static_cast<double>(aMaxWidth);
double fRatio = aPlaceableBound.GetWidth() / fMaxWidth;
- aPlaceableBound = tools::Rectangle(
- aPlaceableBound.Left() / fRatio,
- aPlaceableBound.Top() / fRatio,
- aPlaceableBound.Right() / fRatio,
- aPlaceableBound.Bottom() / fRatio);
+ // changing mnUnitsPerInch as a tool to scale wmf
+ mnUnitsPerInch *= fRatio;
SAL_INFO("emfio", "Placeable bounds "
" left: " << aPlaceableBound.Left() << " top: " << aPlaceableBound.Top()
More information about the Libreoffice-commits
mailing list