Skip to content

Commit 81ea5e7

Browse files
dl3sdodl3sdo
authored
Fix OCD text framing import/export (GH-2419)
Fix type of horizontal and vertical offsets for text symbol shadow framing to be signed. Fix the convertSize() functions to round half down for negative values. FileFormatTest: Add text symbol test Call already existing function to setup the text symbol framing attributes for OCD export. Import color when importing a text symbol with line framing and issue warning for unsupported lines styles. Closes GH-2418 (OCAD import and export of text framing is buggy).
1 parent 90376c7 commit 81ea5e7

File tree

6 files changed

+151
-11
lines changed

6 files changed

+151
-11
lines changed

‎src/fileformats/ocd_file_export.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -423,23 +423,23 @@ Ocd::OcdPoint32 convertPoint(const MapCoord& coord)
423423

424424

425425
/**
426-
* Convert a size to the OCD format.
426+
* Convert a size or offset to the OCD format.
427427
*
428-
* This function converts from 1/100 mm to 1/10 mm, rounding half up for positive values.
428+
* This function converts from 1/100 mm to 1/10 mm, rounding halfway cases away from zero.
429429
*/
430430
constexpr qint16 convertSize(qint32 size)
431431
{
432-
return qint16((size+5) / 10);
432+
return qint16(((size < 0) ? (size-5) : (size+5)) / 10);
433433
}
434434

435435
/**
436436
* Convert a size to the OCD format.
437437
*
438-
* This function converts from 1/100 mm to 1/10 mm, rounding half up for positive values.
438+
* This function converts from 1/100 mm to 1/10 mm, rounding halfway cases away from zero.
439439
*/
440440
constexpr qint32 convertSize(qint64 size)
441441
{
442-
return qint32((size+5) / 10);
442+
return qint32(((size < 0) ? (size-5) : (size+5)) / 10);
443443
}
444444

445445

@@ -1877,6 +1877,7 @@ QByteArray OcdFileExport::exportTextSymbol(const TextSymbol* text_symbol, quint3
18771877
setupTextSymbolExtra(text_symbol, ocd_symbol);
18781878
setupTextSymbolBasic(text_symbol, alignment, ocd_symbol.basic);
18791879
setupTextSymbolSpecial(text_symbol, ocd_symbol.special);
1880+
setupTextSymbolFraming(text_symbol, ocd_symbol.framing);
18801881

18811882
auto header_size = int(sizeof(OcdTextSymbol));
18821883
ocd_symbol.base.size = decltype(ocd_symbol.base.size)(header_size);
@@ -1963,6 +1964,7 @@ void OcdFileExport::setupTextSymbolFraming(const TextSymbol* text_symbol, OcdTex
19631964
if (text_symbol->getFramingColor())
19641965
{
19651966
ocd_text_framing.color = convertColor(text_symbol->getFramingColor());
1967+
ocd_text_framing.line_style_V9 = (ocd_version >= 9) ? /* miter join */ 4 : 0;
19661968
switch (text_symbol->getFramingMode())
19671969
{
19681970
case TextSymbol::NoFraming:

‎src/fileformats/ocd_file_import.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2355,7 +2355,13 @@ void OcdFileImport::setFraming(OcdFileImport::OcdImportedTextSymbol* symbol, con
23552355
case Ocd::FramingLine: // since V7
23562356
symbol->framing = true;
23572357
symbol->framing_mode = TextSymbol::LineFraming;
2358+
symbol->framing_color = convertColor(framing.color);
23582359
symbol->framing_line_half_width = convertLength(framing.line_width);
2360+
if (ocd_version >= 9)
2361+
{
2362+
if (framing.line_style_V9 != 0 && framing.line_style_V9 != 4)
2363+
addSymbolWarning(symbol, tr("Ignoring text framing line style."));
2364+
}
23592365
break;
23602366
case Ocd::FramingRectangle:
23612367
default:

‎src/fileformats/ocd_types_v8.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,8 @@ namespace Ocd
353353
quint16 line_width;
354354
quint16 font_weight; /// TextSymbol only
355355
quint16 italic; /// TextSymbol only
356-
quint16 offset_x;
357-
quint16 offset_y;
356+
qint16 offset_x;
357+
qint16 offset_y;
358358
};
359359

360360
struct TextSymbolV8

‎test/data/text-symbol.omap‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<map xmlns="http://openorienteering.org/apps/mapper/xml/v2" version="9">
3+
<notes></notes>
4+
<georeferencing scale="10000"><projected_crs id="Local"/></georeferencing>
5+
<colors count="2">
6+
<color priority="0" name="Black 100%" c="0" m="0" y="0" k="1" opacity="1"><spotcolors knockout="true"><namedcolor>BLACK</namedcolor></spotcolors><cmyk method="custom"/><rgb method="cmyk" r="0" g="0" b="0"/></color>
7+
<color priority="1" name="Purple" c="0.35" m="0.85" y="0" k="0" opacity="1"><spotcolors knockout="true"><namedcolor>PURPLE</namedcolor></spotcolors><cmyk method="custom"/><rgb method="cmyk" r="0.65" g="0.15" b="1"/></color>
8+
</colors>
9+
<barrier version="6" required="0.6.0">
10+
<symbols count="5">
11+
<symbol type="8" id="0" code="1.1" name="Text symbol with negative shadow framing offset"><text_symbol icon_text="" rotatable="true"><font family="Arial" size="6350"/><text color="0" line_spacing="1" paragraph_spacing="0" character_spacing="0" kerning="true"/><framing color="1" mode="2" line_half_width="200" shadow_x_offset="-400" shadow_y_offset="-500"/></text_symbol></symbol>
12+
<symbol type="8" id="1" code="1.2" name="Text symbol with line framing"><text_symbol icon_text="" rotatable="true"><font family="Arial" size="6350"/><text color="1" line_spacing="1" paragraph_spacing="0" character_spacing="0" kerning="true"/><framing color="0" mode="1" line_half_width="200" shadow_x_offset="-200" shadow_y_offset="200"/></text_symbol></symbol>
13+
<symbol type="8" id="2" code="1.3" name="Text symbol without framing"><text_symbol icon_text="" rotatable="true"><font family="Arial" size="6350"/><text color="1" line_spacing="1" paragraph_spacing="0" character_spacing="0" kerning="true"/></text_symbol></symbol>
14+
<symbol type="8" id="3" code="1.4" name="Text symbol with line below"><text_symbol icon_text="" rotatable="true"><font family="Arial" size="6350"/><text color="1" line_spacing="1" paragraph_spacing="0" character_spacing="0" kerning="true"/><line_below color="0" width="250" distance="150"/></text_symbol></symbol>
15+
<symbol type="8" id="4" code="1.5" name="Text symbol with tabs"><text_symbol icon_text="" rotatable="true"><font family="Arial" size="6350"/><text color="1" line_spacing="1" paragraph_spacing="0" character_spacing="0" kerning="true"/><tabs count="4"><tab>0</tab><tab>1010</tab><tab>20200</tab><tab>300000</tab></tabs></text_symbol></symbol>
16+
</symbols>
17+
<parts count="1" current="0">
18+
<part name="default part"><objects count="0">
19+
</objects></part>
20+
</parts>
21+
<templates count="0" first_front_template="0">
22+
<defaults use_meters_per_pixel="true" meters_per_pixel="0" dpi="0" scale="0"/>
23+
</templates>
24+
<view>
25+
<grid color="#80646464" display="0" alignment="0" additional_rotation="0" unit="1" h_spacing="500" v_spacing="500" h_offset="0" v_offset="0" snapping_enabled="true"/>
26+
<map_view zoom="1" position_x="0" position_y="0"><map opacity="1" visible="true"/><templates count="0"/></map_view>
27+
</view>
28+
</barrier>
29+
</map>

‎test/file_format_t.cpp‎

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,17 @@
7171
#include "core/objects/object.h"
7272
#include "core/objects/text_object.h"
7373
#include "core/symbols/area_symbol.h"
74-
#include "core/symbols/symbol.h"
7574
#include "core/symbols/line_symbol.h"
75+
#include "core/symbols/symbol.h"
76+
#include "core/symbols/text_symbol.h"
7677
#include "fileformats/file_format.h"
7778
#include "fileformats/file_format_registry.h"
7879
#include "fileformats/file_import_export.h"
7980
#include "fileformats/iof_course_export.h"
8081
#include "fileformats/kml_course_export.h"
8182
#include "fileformats/ocd_file_export.h"
82-
#include "fileformats/ocd_file_import.h"
8383
#include "fileformats/ocd_file_format.h"
84+
#include "fileformats/ocd_file_import.h"
8485
#include "fileformats/ocd_types.h"
8586
#include "fileformats/ocd_types_v8.h"
8687
#include "fileformats/ocd_types_v12.h"
@@ -143,7 +144,7 @@ namespace QTest
143144
ba += ", overprinting";
144145
return qstrdup(ba.data());
145146
}
146-
}
147+
} // namespace QTest
147148

148149
#endif
149150

@@ -1787,6 +1788,102 @@ void FileFormatTest::ocdAreaSymbolTest()
17871788
}
17881789
}
17891790

1791+
void FileFormatTest::ocdTextSymbolTest_data()
1792+
{
1793+
QTest::addColumn<int>("format_version");
1794+
1795+
#ifndef MAPPER_BIG_ENDIAN
1796+
static struct { int version; const char* id; } const tests[] = {
1797+
{ 8, "OCD8" },
1798+
{ 9, "OCD9" },
1799+
{ 10, "OCD10" },
1800+
{ 11, "OCD11" },
1801+
{ 12, "OCD12" },
1802+
};
1803+
for (auto& t : tests)
1804+
{
1805+
QTest::newRow(t.id) << t.version;
1806+
}
1807+
#endif
1808+
}
1809+
1810+
void FileFormatTest::ocdTextSymbolTest()
1811+
{
1812+
QFETCH(int, format_version);
1813+
1814+
auto* format_id = QTest::currentDataTag();
1815+
const auto* format = FileFormats.findFormat(format_id);
1816+
QVERIFY(format);
1817+
1818+
static const auto filepath = QString::fromLatin1("data:text-symbol.omap");
1819+
QVERIFY(QFileInfo::exists(filepath));
1820+
1821+
// Load the test map
1822+
auto expected = std::make_unique<Map>();
1823+
QVERIFY(expected->loadFrom(filepath));
1824+
1825+
// Save and load the map
1826+
auto actual = saveAndLoadMap(*expected, format);
1827+
QVERIFY2(actual, "Exception while importing / exporting.");
1828+
QCOMPARE(actual->property(OcdFileFormat::versionProperty()), format_version);
1829+
1830+
// Symbols
1831+
QCOMPARE(actual->getNumSymbols(), expected->getNumSymbols());
1832+
for (int i = 0; i < actual->getNumSymbols(); ++i)
1833+
{
1834+
auto* expected_symbol = expected->getSymbol(i);
1835+
if (expected_symbol->getType() != Symbol::Text) // actually redundant for the given test data
1836+
continue;
1837+
1838+
auto* actual_symbol = actual->getSymbol(i);
1839+
QCOMPARE(actual_symbol->getType(), Symbol::Text);
1840+
1841+
COMPARE_SYMBOL_PROPERTY(actual_symbol->getNumberComponent(0), expected_symbol->getNumberComponent(0), *expected_symbol);
1842+
// OCD limitation: always two number components
1843+
if (expected_symbol->getNumberComponent(1) != -1)
1844+
COMPARE_SYMBOL_PROPERTY(actual_symbol->getNumberComponent(1), expected_symbol->getNumberComponent(1), *expected_symbol);
1845+
1846+
QVERIFY(expected_symbol->stateEquals(actual_symbol));
1847+
1848+
auto* expected_text_symbol = expected_symbol->asText();
1849+
auto* actual_text_symbol = actual_symbol->asText();
1850+
1851+
COMPARE_SYMBOL_PROPERTY(bool(actual_text_symbol->getColor()), bool(expected_text_symbol->getColor()), *expected_text_symbol);
1852+
if (expected_text_symbol->getColor())
1853+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getColor()->operator QRgb(), expected_text_symbol->getColor()->operator QRgb(), *expected_text_symbol);
1854+
COMPARE_SYMBOL_PROPERTY(bool(actual_text_symbol->getFramingColor()), bool(expected_text_symbol->getFramingColor()), *expected_text_symbol);
1855+
if (expected_text_symbol->getFramingColor())
1856+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getFramingColor()->operator QRgb(), expected_text_symbol->getFramingColor()->operator QRgb(), *expected_text_symbol);
1857+
COMPARE_SYMBOL_PROPERTY(bool(actual_text_symbol->getLineBelowColor()), bool(expected_text_symbol->getLineBelowColor()), *expected_text_symbol);
1858+
if (expected_text_symbol->getLineBelowColor())
1859+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getLineBelowColor()->operator QRgb(), expected_text_symbol->getLineBelowColor()->operator QRgb(), *expected_text_symbol);
1860+
1861+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getFramingMode(), expected_text_symbol->getFramingMode(), *expected_text_symbol);
1862+
if (expected_text_symbol->getFramingMode() == TextSymbol::FramingMode::ShadowFraming)
1863+
{
1864+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getFramingShadowYOffset(), expected_text_symbol->getFramingShadowYOffset(), *expected_text_symbol);
1865+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getFramingShadowXOffset(), expected_text_symbol->getFramingShadowXOffset(), *expected_text_symbol);
1866+
}
1867+
if (expected_text_symbol->getFramingMode() == TextSymbol::FramingMode::LineFraming)
1868+
{
1869+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getFramingLineHalfWidth(), expected_text_symbol->getFramingLineHalfWidth(), *expected_text_symbol);
1870+
}
1871+
1872+
if (expected_text_symbol->hasLineBelow())
1873+
{
1874+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getLineBelowWidth(), expected_text_symbol->getLineBelowWidth(), *expected_text_symbol);
1875+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getLineBelowDistance(), expected_text_symbol->getLineBelowDistance(), *expected_text_symbol);
1876+
}
1877+
1878+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getNumCustomTabs(), expected_text_symbol->getNumCustomTabs(), *expected_text_symbol);
1879+
if (expected_text_symbol->getNumCustomTabs())
1880+
{
1881+
for (int i = 0; i < expected_text_symbol->getNumCustomTabs(); ++i)
1882+
COMPARE_SYMBOL_PROPERTY(actual_text_symbol->getCustomTab(i), expected_text_symbol->getCustomTab(i), *expected_text_symbol);
1883+
}
1884+
}
1885+
}
1886+
17901887

17911888
/*
17921889
* We don't need a real GUI window.

‎test/file_format_t.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,13 @@ private slots:
133133
*/
134134
void ocdAreaSymbolTest_data();
135135
void ocdAreaSymbolTest();
136-
136+
137+
/**
138+
* Test OCD text symbol export and import.
139+
*/
140+
void ocdTextSymbolTest_data();
141+
void ocdTextSymbolTest();
142+
137143
};
138144

139145
#endif // OPENORIENTEERING_FILE_FORMAT_T_H

0 commit comments

Comments
 (0)