Skip to content

Fix OCD text framing import/export #2419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+151 −11
Open

Fix OCD text framing import/export #2419

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dce5853
Fix OCD text framing import/export
dl3sdo Sep 19, 2025
bddbd53
Nitpick
dg0yt Nov 2, 2025
e339bc7
OcdFileExport: Fix conversion for negative values
dl3sdo Nov 2, 2025
1d312bc
OcdFileExport: Rework conversion for negative values
dl3sdo Nov 3, 2025
07789fa
FileFormatTest: Add text symbol test
dl3sdo Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/fileformats/ocd_file_export.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,21 +425,27 @@ Ocd::OcdPoint32 convertPoint(const MapCoord& coord)
/**
* Convert a size to the OCD format.
*
* This function converts from 1/100 mm to 1/10 mm, rounding half up for positive values.
* This function converts from 1/100 mm to 1/10 mm, rounding half up for positive values and rounding half down for negative values.
*/
constexpr qint16 convertSize(qint32 size)
{
return qint16((size+5) / 10);
if (size >= 0)
return qint16((size+5) / 10);
else
return qint16((size-5) / 10);
Copy link
Member

@dg0yt dg0yt Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of constexpr doubt, this is a single expression:

Suggested change
if (size >= 0)
return qint16((size+5) / 10);
else
return qint16((size-5) / 10);
return qint16(((size < 0) ? (size-5) : (size+5)) / 10);

I wonder about the desired result for -5, -15 etc. Half down or half up? Obviously we are not converting a "size" here, but an "offset".

Copy link
Member Author

@dl3sdo dl3sdo Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I first added the convertOffset() function but then decided that this change should be applied to the convertSize() functions (even if it's rather an offset):
If the values are always positive, nothing changes. If the values are negative (besides the shadow framing offsets) then this would be fixed (an assertion would help identifying those occurences).
And obviously, 200..204 should become 20 whereas -200..-204 should become -20 instead of -19.

Copy link
Member

@dg0yt dg0yt Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And obviously, 200..204 should become 20 whereas -200..-204 should become -20 instead of -19.

I'm really asking about the 5. What do we want for -205 when +205 means +21?

Copy link
Member Author

@dl3sdo dl3sdo Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go for the standard rounding behaviour ("rounding halfway cases away from zero") as described in cppreference.com which means that -205 will become -21. Basically, it makes not really a difference if -205 becomes -21 or -20 as rounding means that always something is changed.
Regarding our specific mapper toppic: I thought it cannot even happen since the shadow framing offset integer values can only be a multiple of +-100 due to the GUI setting. But scaling the symbol can lead to any value!

}

/**
* Convert a size to the OCD format.
*
* This function converts from 1/100 mm to 1/10 mm, rounding half up for positive values.
* This function converts from 1/100 mm to 1/10 mm, rounding half up for positive values and rounding half down for negative values.
*/
constexpr qint32 convertSize(qint64 size)
{
return qint32((size+5) / 10);
if (size >= 0)
return qint32((size+5) / 10);
else
return qint32((size-5) / 10);
}


Expand Down Expand Up @@ -1877,6 +1883,7 @@ QByteArray OcdFileExport::exportTextSymbol(const TextSymbol* text_symbol, quint3
setupTextSymbolExtra(text_symbol, ocd_symbol);
setupTextSymbolBasic(text_symbol, alignment, ocd_symbol.basic);
setupTextSymbolSpecial(text_symbol, ocd_symbol.special);
setupTextSymbolFraming(text_symbol, ocd_symbol.framing);

auto header_size = int(sizeof(OcdTextSymbol));
ocd_symbol.base.size = decltype(ocd_symbol.base.size)(header_size);
Expand Down Expand Up @@ -1963,6 +1970,7 @@ void OcdFileExport::setupTextSymbolFraming(const TextSymbol* text_symbol, OcdTex
if (text_symbol->getFramingColor())
{
ocd_text_framing.color = convertColor(text_symbol->getFramingColor());
ocd_text_framing.line_style_V9 = (ocd_version >= 9) ? /* miter join */ 4 : 0;
switch (text_symbol->getFramingMode())
{
case TextSymbol::NoFraming:
Expand Down
6 changes: 6 additions & 0 deletions src/fileformats/ocd_file_import.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2355,7 +2355,13 @@ void OcdFileImport::setFraming(OcdFileImport::OcdImportedTextSymbol* symbol, con
case Ocd::FramingLine: // since V7
symbol->framing = true;
symbol->framing_mode = TextSymbol::LineFraming;
symbol->framing_color = convertColor(framing.color);
symbol->framing_line_half_width = convertLength(framing.line_width);
if (ocd_version >= 9)
{
if (framing.line_style_V9 != 0 && framing.line_style_V9 != 4)
addSymbolWarning(symbol, tr("Ignoring text framing line style."));
}
break;
case Ocd::FramingRectangle:
default:
Expand Down
4 changes: 2 additions & 2 deletions src/fileformats/ocd_types_v8.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,8 @@ namespace Ocd
quint16 line_width;
quint16 font_weight; /// TextSymbol only
quint16 italic; /// TextSymbol only
quint16 offset_x;
quint16 offset_y;
qint16 offset_x;
qint16 offset_y;
};

struct TextSymbolV8
Expand Down