From 747f3837956e540437d18d424a3b235dbe889c92 Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:42:17 +0200 Subject: [PATCH 1/8] Map: Rename color manipulation methods The number accepted by getMapColor() and similar methods is not exactly an index. The value is treated as the color rendering priority, it is synced into the color Priority member and should be treated as such. Namely, reordering the color table changes the priorities invalidating any previous parameter values. This commit makes it obvious what value we are using for the color lookup. --- src/core/map.cpp | 42 ++++++++-------- src/core/map.h | 37 +++++++------- src/core/map_printer.cpp | 4 +- src/core/objects/symbol_rule_set.cpp | 4 +- src/core/renderables/renderable.cpp | 10 ++-- src/core/symbols/area_symbol.cpp | 8 +-- src/core/symbols/line_symbol.cpp | 8 +-- src/core/symbols/point_symbol.cpp | 8 +-- src/core/symbols/symbol.cpp | 6 +-- src/core/symbols/text_symbol.cpp | 12 ++--- src/fileformats/ocd_file_export.cpp | 26 +++++----- src/fileformats/ocd_file_import.cpp | 8 +-- src/fileformats/xml_file_format.cpp | 2 +- src/gdal/ogr_file_format.cpp | 4 +- src/gui/map/map_editor.cpp | 4 +- src/gui/map/map_widget.cpp | 4 +- src/gui/symbols/area_symbol_settings.cpp | 4 +- src/gui/widgets/color_dropdown.cpp | 4 +- src/gui/widgets/color_list_widget.cpp | 28 +++++------ src/tools/fill_tool.cpp | 4 +- test/file_format_t.cpp | 12 ++--- test/map_t.cpp | 62 ++++++++++++------------ test/symbol_set_t.cpp | 12 ++--- 23 files changed, 158 insertions(+), 155 deletions(-) diff --git a/src/core/map.cpp b/src/core/map.cpp index 35ea58027c..0df3495089 100644 --- a/src/core/map.cpp +++ b/src/core/map.cpp @@ -673,7 +673,7 @@ QHash Map::importMap( qWarning("Map::importMap() called with GeorefImport flag set"); // Determine which symbols and colors to import - std::vector color_filter(std::size_t(imported_map.getNumColors()), true); + std::vector color_filter(std::size_t(imported_map.getNumColorPrios()), true); std::vector symbol_filter(std::size_t(imported_map.getNumSymbols()), true); if (mode.testFlag(MinimalImport)) { @@ -1182,12 +1182,12 @@ QString Map::raw_translation(const QString& symbol_text) const -void Map::setColor(MapColor* color, int pos) +void Map::setColor(MapColor* color, int prio) { // MapColor* old_color = color_set->colors[pos]; - color_set->colors[pos] = color; - color->setPriority(pos); + color_set->colors[prio] = color; + color->setPriority(prio); if (color->getSpotColorMethod() == MapColor::SpotColor) { @@ -1227,20 +1227,20 @@ void Map::setColor(MapColor* color, int pos) } updateSymbolIcons(color); - emit colorChanged(pos, color); + emit colorChanged(prio, color); } -void Map::addColor(MapColor* color, int pos) +void Map::addColor(MapColor* color, int prio) { - color_set->insert(pos, color); + color_set->insert(prio, color); setColorsDirty(); - emit colorAdded(pos, color); - color->setPriority(pos); + emit colorAdded(prio, color); + color->setPriority(prio); } -void Map::deleteColor(int pos) +void Map::deleteColor(int prio) { - MapColor* color = color_set->colors[pos]; + MapColor* color = color_set->colors[prio]; if (color->getSpotColorMethod() == MapColor::SpotColor) { // Update dependent colors @@ -1254,7 +1254,7 @@ void Map::deleteColor(int pos) } } - color_set->erase(pos); + color_set->erase(prio); // Treat combined symbols first before their parts for (Symbol* symbol : symbols) @@ -1267,12 +1267,12 @@ void Map::deleteColor(int pos) if (symbol->getType() != Symbol::Combined) symbol->colorDeletedEvent(color); } - emit colorDeleted(pos, color); + emit colorDeleted(prio, color); delete color; } -int Map::findColorIndex(const MapColor* color) const +int Map::findColorPrio(const MapColor* color) const { std::size_t size = color_set->colors.size(); for (std::size_t i = 0; i < size; ++i) @@ -1317,12 +1317,12 @@ void Map::determineColorsInUse(const std::vector< bool >& by_which_symbols, std: } Q_ASSERT(int(by_which_symbols.size()) == getNumSymbols()); - out.assign(std::size_t(getNumColors()), false); - for (std::size_t c = 0, last = std::size_t(getNumColors()); c != last; ++c) + out.assign(std::size_t(getNumColorPrios()), false); + for (std::size_t c = 0, last = std::size_t(getNumColorPrios()); c != last; ++c) { for (std::size_t s = 0, last_s = std::size_t(getNumSymbols()); s != last_s; ++s) { - if (by_which_symbols[s] && getSymbol(int(s))->containsColor(getColor(int(c)))) + if (by_which_symbols[s] && getSymbol(int(s))->containsColor(getColorByPrio(int(c)))) { out[c] = true; break; @@ -1331,21 +1331,21 @@ void Map::determineColorsInUse(const std::vector< bool >& by_which_symbols, std: } // Include required spot colors, too - for (std::size_t c = 0, last_c = std::size_t(getNumColors()); c != last_c; ++c) + for (std::size_t c = 0, last_c = std::size_t(getNumColorPrios()); c != last_c; ++c) { if (out[c]) continue; - const auto* color = getColor(int(c)); + const auto* color = getColorByPrio(int(c)); if (color->getSpotColorMethod() != MapColor::SpotColor) continue; - for (std::size_t o = 0, last_o = std::size_t(getNumColors()); o != last_o; ++o) + for (std::size_t o = 0, last_o = std::size_t(getNumColorPrios()); o != last_o; ++o) { if (!out[o]) continue; - const auto* other = getColor(int(o)); + const auto* other = getColorByPrio(int(o)); if (other->getSpotColorMethod() != MapColor::CustomColor) continue; diff --git a/src/core/map.h b/src/core/map.h index e251a6abe0..189ac7886f 100644 --- a/src/core/map.h +++ b/src/core/map.h @@ -400,22 +400,25 @@ friend class XMLFileExporter; // Colors - /** Returns the number of map colors defined in this map.*/ - int getNumColors() const; + /** Returns the number of map color priorities defined in this map. The + * count does not include special colors returned by getColorByPrio(). + * The priorities are guaranteed to be a continuous series starting at + * zero. */ + int getNumColorPrios() const; /** Returns a pointer to the MapColor identified by the non-negative priority i. * * Returns nullptr if the color is not defined, or if it is a special color (i.e i<0), * i.e. only actual map colors are returned. */ - const MapColor* getMapColor(int i) const; + const MapColor* getMapColorByPrio(int i) const; /** Returns a pointer to the MapColor identified by the non-negative priority i. * * Returns nullptr if the color is not defined, or if it is a special color (i.e i<0), * i.e. only actual map colors are returned. */ - MapColor* getMapColor(int i); + MapColor* getMapColorByPrio(int i); /** Returns a pointer to the const MapColor identified by the priority i. * @@ -424,33 +427,33 @@ friend class XMLFileExporter; * * Returns nullptr if the color is not defined. */ - const MapColor* getColor(int i) const; + const MapColor* getColorByPrio(int i) const; /** - * Replaces the color at index pos with the given color, updates dependent - * colors and symbol icons. + * Replaces the color with priority prio with the given color, and updates + * dependent colors and symbol icons. * * Emits colorChanged(). Does not delete the replaced color. */ - void setColor(MapColor* color, int pos); + void setColor(MapColor* color, int prio); /** * Adds the given color as a new color at the given index. * Emits colorAdded(). */ - void addColor(MapColor* color, int pos); + void addColor(MapColor* color, int prio); /** - * Deletes the color at the given index. + * Deletes the color with the given priority. * Emits colorDeleted(). */ - void deleteColor(int pos); + void deleteColor(int prio); /** * Loops through the color list, looking for the given color pointer. * Returns the index of the color, or -1 if it is not found. */ - int findColorIndex(const MapColor* color) const; + int findColorPrio(const MapColor* color) const; /** * Marks the colors as "dirty", i.e. as having unsaved changes. @@ -1665,19 +1668,19 @@ protected slots: // ### Map inline code ### inline -int Map::getNumColors() const +int Map::getNumColorPrios() const { return (int)color_set->colors.size(); } inline -MapColor* Map::getMapColor(int i) +MapColor* Map::getMapColorByPrio(int i) { - return const_cast(static_cast(this)->getMapColor(i)); + return const_cast(static_cast(this)->getMapColorByPrio(i)); } inline -const MapColor* Map::getMapColor(int i) const +const MapColor* Map::getMapColorByPrio(int i) const { if (0 <= i && i < (int)color_set->colors.size()) { @@ -1687,7 +1690,7 @@ const MapColor* Map::getMapColor(int i) const } inline -const MapColor* Map::getColor(int i) const +const MapColor* Map::getColorByPrio(int i) const { if (0 <= i && i < (int)color_set->colors.size()) { diff --git a/src/core/map_printer.cpp b/src/core/map_printer.cpp index a5bea1a9a9..b919e47bc0 100644 --- a/src/core/map_printer.cpp +++ b/src/core/map_printer.cpp @@ -1261,9 +1261,9 @@ void MapPrinter::drawSeparationPages(QPrinter* printer, QPainter* device_painter device_painter->setClipRect(page_extent.intersected(print_area).adjusted(-10, 10, 10, 10), Qt::ReplaceClip); bool need_new_page = false; - for (int i = map.getNumColors() - 1; i >= 0; --i) + for (int i = map.getNumColorPrios() - 1; i >= 0; --i) { - const MapColor* color = map.getColor(i); + const MapColor* color = map.getColorByPrio(i); if (color->getSpotColorMethod() == MapColor::SpotColor) { if (need_new_page) diff --git a/src/core/objects/symbol_rule_set.cpp b/src/core/objects/symbol_rule_set.cpp index 6f7dc62b4f..9db7246da1 100644 --- a/src/core/objects/symbol_rule_set.cpp +++ b/src/core/objects/symbol_rule_set.cpp @@ -508,9 +508,9 @@ void SymbolRuleSet::apply(Map& object_map, const Map& symbol_set, Options option std::vector colors_in_use; object_map.determineColorsInUse(all_symbols, colors_in_use); if (colors_in_use.empty()) - colors_in_use.assign(std::size_t(object_map.getNumColors()), false); + colors_in_use.assign(std::size_t(object_map.getNumColorPrios()), false); - for (int i = object_map.getNumColors() - 1; i >= 0; --i) + for (int i = object_map.getNumColorPrios() - 1; i >= 0; --i) { if (!colors_in_use[std::size_t(i)]) object_map.deleteColor(i); diff --git a/src/core/renderables/renderable.cpp b/src/core/renderables/renderable.cpp index 489cdcc977..9699b6ed28 100644 --- a/src/core/renderables/renderable.cpp +++ b/src/core/renderables/renderable.cpp @@ -249,14 +249,14 @@ void MapRenderables::draw(QPainter *painter, const RenderConfig &config) const painter->save(); auto end_of_colors = rend(); auto color = rbegin(); - while (color != end_of_colors && color->first >= map->getNumColors()) + while (color != end_of_colors && color->first >= map->getNumColorPrios()) { ++color; } for (; color != end_of_colors; ++color) { if ( config.testFlag(RenderConfig::RequireSpotColor) && - (color->first < 0 || map->getColor(color->first)->getSpotColorMethod() == MapColor::UndefinedMethod) ) + (color->first < 0 || map->getColorByPrio(color->first)->getSpotColorMethod() == MapColor::UndefinedMethod) ) { continue; } @@ -277,7 +277,7 @@ void MapRenderables::draw(QPainter *painter, const RenderConfig &config) const { // Render the renderables const PainterConfig& state = renderables.first; - const MapColor* map_color = map->getColor(state.color_priority); + const MapColor* map_color = map->getColorByPrio(state.color_priority); if (!map_color) { Q_ASSERT(state.color_priority == MapColor::Reserved); @@ -429,13 +429,13 @@ void MapRenderables::drawColorSeparation(QPainter* painter, const RenderConfig& // For each pair of color priority and its renderables collection... auto end_of_colors = rend(); auto color = rbegin(); - while (color != end_of_colors && color->first >= map->getNumColors()) + while (color != end_of_colors && color->first >= map->getNumColorPrios()) { ++color; } for (; color != end_of_colors; ++color) { - SpotColorComponent drawing_color(map->getColor(color->first), 1.0f); + SpotColorComponent drawing_color(map->getColorByPrio(color->first), 1.0f); // Check whether the current color [priority] applies to the current separation. if (color->first > MapColor::Reserved) diff --git a/src/core/symbols/area_symbol.cpp b/src/core/symbols/area_symbol.cpp index f972bfcfd9..e614fc427a 100644 --- a/src/core/symbols/area_symbol.cpp +++ b/src/core/symbols/area_symbol.cpp @@ -85,7 +85,7 @@ void AreaSymbol::FillPattern::save(QXmlStreamWriter& xml, const Map& map) const switch (type) { case LinePattern: - element.writeAttribute(QLatin1String("color"), map.findColorIndex(line_color)); + element.writeAttribute(QLatin1String("color"), map.findColorPrio(line_color)); element.writeAttribute(QLatin1String("line_width"), line_width); break; @@ -115,7 +115,7 @@ void AreaSymbol::FillPattern::load(QXmlStreamReader& xml, const Map& map, Symbol switch (type) { case LinePattern: - line_color = map.getColor(element.attribute(QLatin1String("color"))); + line_color = map.getColorByPrio(element.attribute(QLatin1String("color"))); line_width = element.attribute(QLatin1String("line_width")); break; @@ -781,7 +781,7 @@ bool AreaSymbol::hasRotatableFillPattern() const void AreaSymbol::saveImpl(QXmlStreamWriter& xml, const Map& map) const { XmlElementWriter element { xml, QLatin1String("area_symbol") }; - element.writeAttribute(QLatin1String{"inner_color"}, map.findColorIndex(color)); + element.writeAttribute(QLatin1String{"inner_color"}, map.findColorPrio(color)); element.writeAttribute(QLatin1String{"min_area"}, minimum_area); element.writeAttribute(QLatin1String{"patterns"}, patterns.size()); for (const auto& pattern : patterns) @@ -794,7 +794,7 @@ bool AreaSymbol::loadImpl(QXmlStreamReader& xml, const Map& map, SymbolDictionar return false; XmlElementReader element { xml }; - color = map.getColor(element.attribute(QLatin1String("inner_color"))); + color = map.getColorByPrio(element.attribute(QLatin1String("inner_color"))); minimum_area = element.attribute(QLatin1String("min_area")); auto num_patterns = element.attribute(QLatin1String("patterns")); diff --git a/src/core/symbols/line_symbol.cpp b/src/core/symbols/line_symbol.cpp index 36b62be6d2..45320cc571 100644 --- a/src/core/symbols/line_symbol.cpp +++ b/src/core/symbols/line_symbol.cpp @@ -62,7 +62,7 @@ using length_type = PathCoord::length_type; void LineSymbolBorder::save(QXmlStreamWriter& xml, const Map& map) const { XmlElementWriter element(xml, QLatin1String("border")); - element.writeAttribute(QLatin1String("color"), map.findColorIndex(color)); + element.writeAttribute(QLatin1String("color"), map.findColorPrio(color)); element.writeAttribute(QLatin1String("width"), width); element.writeAttribute(QLatin1String("shift"), shift); if (dashed) @@ -77,7 +77,7 @@ bool LineSymbolBorder::load(QXmlStreamReader& xml, const Map& map) { Q_ASSERT(xml.name() == QLatin1String("border")); XmlElementReader element(xml); - color = map.getColor(element.attribute(QLatin1String("color"))); + color = map.getColorByPrio(element.attribute(QLatin1String("color"))); width = element.attribute(QLatin1String("width")); shift = element.attribute(QLatin1String("shift")); dashed = element.attribute(QLatin1String("dashed")); @@ -1804,7 +1804,7 @@ void LineSymbol::replaceSymbol(PointSymbol*& old_symbol, PointSymbol* replace_wi void LineSymbol::saveImpl(QXmlStreamWriter& xml, const Map& map) const { xml.writeStartElement(QString::fromLatin1("line_symbol")); - xml.writeAttribute(QString::fromLatin1("color"), QString::number(map.findColorIndex(color))); + xml.writeAttribute(QString::fromLatin1("color"), QString::number(map.findColorPrio(color))); xml.writeAttribute(QString::fromLatin1("line_width"), QString::number(line_width)); xml.writeAttribute(QString::fromLatin1("minimum_length"), QString::number(minimum_length)); xml.writeAttribute(QString::fromLatin1("join_style"), QString::number(join_style)); @@ -1891,7 +1891,7 @@ bool LineSymbol::loadImpl(QXmlStreamReader& xml, const Map& map, SymbolDictionar QXmlStreamAttributes attributes = xml.attributes(); int temp = attributes.value(QLatin1String("color")).toInt(); - color = map.getColor(temp); + color = map.getColorByPrio(temp); line_width = attributes.value(QLatin1String("line_width")).toInt(); minimum_length = attributes.value(QLatin1String("minimum_length")).toInt(); join_style = static_cast(attributes.value(QLatin1String("join_style")).toInt()); diff --git a/src/core/symbols/point_symbol.cpp b/src/core/symbols/point_symbol.cpp index 235a3b08c6..793d21f59d 100644 --- a/src/core/symbols/point_symbol.cpp +++ b/src/core/symbols/point_symbol.cpp @@ -581,9 +581,9 @@ void PointSymbol::saveImpl(QXmlStreamWriter& xml, const Map& map) const if (isRotatable()) xml.writeAttribute(QString::fromLatin1("rotatable"), QString::fromLatin1("true")); xml.writeAttribute(QString::fromLatin1("inner_radius"), QString::number(inner_radius)); - xml.writeAttribute(QString::fromLatin1("inner_color"), QString::number(map.findColorIndex(inner_color))); + xml.writeAttribute(QString::fromLatin1("inner_color"), QString::number(map.findColorPrio(inner_color))); xml.writeAttribute(QString::fromLatin1("outer_width"), QString::number(outer_width)); - xml.writeAttribute(QString::fromLatin1("outer_color"), QString::number(map.findColorIndex(outer_color))); + xml.writeAttribute(QString::fromLatin1("outer_color"), QString::number(map.findColorPrio(outer_color))); int num_elements = getNumElements(); xml.writeAttribute(QString::fromLatin1("elements"), QString::number(num_elements)); for (auto& element : elements) @@ -605,10 +605,10 @@ bool PointSymbol::loadImpl(QXmlStreamReader& xml, const Map& map, SymbolDictiona setRotatable(attributes.value(QLatin1String("rotatable")) == QLatin1String("true")); inner_radius = attributes.value(QLatin1String("inner_radius")).toInt(); int temp = attributes.value(QLatin1String("inner_color")).toInt(); - inner_color = map.getColor(temp); + inner_color = map.getColorByPrio(temp); outer_width = attributes.value(QLatin1String("outer_width")).toInt(); temp = attributes.value(QLatin1String("outer_color")).toInt(); - outer_color = map.getColor(temp); + outer_color = map.getColorByPrio(temp); int num_elements = attributes.value(QLatin1String("elements")).toInt(); elements.reserve(qMin(num_elements, 10)); // 10 is not a limit diff --git a/src/core/symbols/symbol.cpp b/src/core/symbols/symbol.cpp index 8f8c2fec9b..b2ff5a2f65 100644 --- a/src/core/symbols/symbol.cpp +++ b/src/core/symbols/symbol.cpp @@ -953,9 +953,9 @@ bool Symbol::lessByColorPriority(const Symbol* s1, const Symbol* s2) Symbol::lessByColor::lessByColor(const Map* map) { - colors.reserve(std::size_t(map->getNumColors())); - for (int i = 0; i < map->getNumColors(); ++i) - colors.push_back(QRgb(*map->getColor(i))); + colors.reserve(std::size_t(map->getNumColorPrios())); + for (int i = 0; i < map->getNumColorPrios(); ++i) + colors.push_back(QRgb(*map->getColorByPrio(i))); } diff --git a/src/core/symbols/text_symbol.cpp b/src/core/symbols/text_symbol.cpp index 6580c9a9bf..30be35f2e7 100644 --- a/src/core/symbols/text_symbol.cpp +++ b/src/core/symbols/text_symbol.cpp @@ -353,7 +353,7 @@ void TextSymbol::saveImpl(QXmlStreamWriter& xml, const Map& map) const xml.writeEndElement(/*font*/); xml.writeStartElement(QStringLiteral("text")); - xml.writeAttribute(QStringLiteral("color"), QString::number(map.findColorIndex(color))); + xml.writeAttribute(QStringLiteral("color"), QString::number(map.findColorPrio(color))); xml.writeAttribute(QStringLiteral("line_spacing"), QString::number(line_spacing)); xml.writeAttribute(QStringLiteral("paragraph_spacing"), QString::number(paragraph_spacing)); xml.writeAttribute(QStringLiteral("character_spacing"), QString::number(character_spacing)); @@ -364,7 +364,7 @@ void TextSymbol::saveImpl(QXmlStreamWriter& xml, const Map& map) const if (framing) { xml.writeStartElement(QStringLiteral("framing")); - xml.writeAttribute(QStringLiteral("color"), QString::number(map.findColorIndex(framing_color))); + xml.writeAttribute(QStringLiteral("color"), QString::number(map.findColorPrio(framing_color))); xml.writeAttribute(QStringLiteral("mode"), QString::number(framing_mode)); xml.writeAttribute(QStringLiteral("line_half_width"), QString::number(framing_line_half_width)); xml.writeAttribute(QStringLiteral("shadow_x_offset"), QString::number(framing_shadow_x_offset)); @@ -375,7 +375,7 @@ void TextSymbol::saveImpl(QXmlStreamWriter& xml, const Map& map) const if (line_below) { xml.writeStartElement(QStringLiteral("line_below")); - xml.writeAttribute(QStringLiteral("color"), QString::number(map.findColorIndex(line_below_color))); + xml.writeAttribute(QStringLiteral("color"), QString::number(map.findColorPrio(line_below_color))); xml.writeAttribute(QStringLiteral("width"), QString::number(line_below_width)); xml.writeAttribute(QStringLiteral("distance"), QString::number(line_below_distance)); xml.writeEndElement(/*line_below*/); @@ -423,7 +423,7 @@ bool TextSymbol::loadImpl(QXmlStreamReader& xml, const Map& map, SymbolDictionar else if (xml.name() == QLatin1String("text")) { int temp = attributes.value(QLatin1String("color")).toInt(); - color = map.getColor(temp); + color = map.getColorByPrio(temp); line_spacing = attributes.value(QLatin1String("line_spacing")).toFloat(); paragraph_spacing = attributes.value(QLatin1String("paragraph_spacing")).toInt(); character_spacing = attributes.value(QLatin1String("character_spacing")).toFloat(); @@ -434,7 +434,7 @@ bool TextSymbol::loadImpl(QXmlStreamReader& xml, const Map& map, SymbolDictionar { framing = true; int temp = attributes.value(QLatin1String("color")).toInt(); - framing_color = map.getColor(temp); + framing_color = map.getColorByPrio(temp); framing_mode = attributes.value(QLatin1String("mode")).toInt(); framing_line_half_width = attributes.value(QLatin1String("line_half_width")).toInt(); framing_shadow_x_offset = attributes.value(QLatin1String("shadow_x_offset")).toInt(); @@ -445,7 +445,7 @@ bool TextSymbol::loadImpl(QXmlStreamReader& xml, const Map& map, SymbolDictionar { line_below = true; int temp = attributes.value(QLatin1String("color")).toInt(); - line_below_color = map.getColor(temp); + line_below_color = map.getColorByPrio(temp); line_below_width = attributes.value(QLatin1String("width")).toInt(); line_below_distance = attributes.value(QLatin1String("distance")).toInt(); xml.skipCurrentElement(); diff --git a/src/fileformats/ocd_file_export.cpp b/src/fileformats/ocd_file_export.cpp index 111207d709..d363cdf8b8 100644 --- a/src/fileformats/ocd_file_export.cpp +++ b/src/fileformats/ocd_file_export.cpp @@ -111,12 +111,12 @@ static QTextCodec* codecFromSettings() */ int beginOfSpotColors(const Map* map) { - const auto num_colors = map->getNumColors(); + const auto num_colors = map->getNumColorPrios(); auto first_pure_spot_color = num_colors; for (auto i = num_colors; i > 0; ) { --i; - const auto color = map->getColor(i); + const auto color = map->getColorByPrio(i); if (color->getSpotColorMethod() != MapColor::SpotColor || map->isColorUsedByASymbol(color)) break; @@ -823,13 +823,13 @@ void OcdFileExport::exportSetup(OcdFile& file) { auto& symbol_header = file.header()->symbol_header; - auto num_colors = map->getNumColors(); + auto num_colors = map->getNumColorPrios(); auto spotColorNumber = [this, num_colors](const MapColor* color)->quint16 { quint16 number = 0; for (auto i = 0; i < num_colors; ++i) { - const auto* current = map->getColor(i); + const auto* current = map->getColorByPrio(i); if (current == color) break; if (current->getSpotColorMethod() == MapColor::SpotColor) @@ -866,7 +866,7 @@ void OcdFileExport::exportSetup(OcdFile& file) for (int i = 0; i < num_colors; ++i) { - const auto* color = map->getColor(i); + const auto* color = map->getColorByPrio(i); const auto& cmyk = color->getCmyk(); // OC*D stores CMYK values as integers from 0-200. auto ocd_cmyk = Ocd::CmykV8 { @@ -999,7 +999,7 @@ void OcdFileExport::exportSetup() } // Map colors - auto num_colors = map->getNumColors(); + auto num_colors = map->getNumColorPrios(); auto begin_of_spot_colors = beginOfSpotColors(map); auto ocd_number = 0; auto spot_number = 0; @@ -1008,7 +1008,7 @@ void OcdFileExport::exportSetup() SpotColorComponents all_spot_colors; for (int i = 0; i < num_colors; ++i) { - const auto* color = map->getColor(i); + const auto* color = map->getColorByPrio(i); if (color->getSpotColorMethod() == MapColor::SpotColor) { all_spot_colors.push_back({color, 1}); @@ -1023,7 +1023,7 @@ void OcdFileExport::exportSetup() for (int i = 0; i < num_colors; ++i) { - const auto* color = map->getColor(i); + const auto* color = map->getColorByPrio(i); if (color->getSpotColorMethod() == MapColor::SpotColor) { addParameterString(10, stringForSpotColor(spot_number++, *color)); @@ -1136,9 +1136,9 @@ void OcdFileExport::setupSymbolColors(const Symbol* symbol, O *bitpos |= bitmask; bitmask <<= 1; } - for (int c = 0; c < map->getNumColors(); ++c) + for (int c = 0; c < map->getNumColorPrios(); ++c) { - if (symbol->containsColor(map->getColor(c))) + if (symbol->containsColor(map->getColorByPrio(c))) *bitpos |= bitmask; if (bitmask == 0x80u) @@ -1182,9 +1182,9 @@ void OcdFileExport::setupSymbolColors(const Symbol* symbol, Counter& num_colors, } } - for (int c = 0; c < map->getNumColors(); ++c) + for (int c = 0; c < map->getNumColorPrios(); ++c) { - if (!symbol->containsColor(map->getColor(c))) + if (!symbol->containsColor(map->getColorByPrio(c))) continue; ++num_colors; @@ -2898,7 +2898,7 @@ void OcdFileExport::exportExtras() quint16 OcdFileExport::convertColor(const MapColor* color) const { - auto index = map->findColorIndex(color); + auto index = map->findColorPrio(color); if (index >= 0) { return quint16(uses_registration_color ? (index + 1) : index); diff --git a/src/fileformats/ocd_file_import.cpp b/src/fileformats/ocd_file_import.cpp index 0fac5b1c25..c045a5279f 100644 --- a/src/fileformats/ocd_file_import.cpp +++ b/src/fileformats/ocd_file_import.cpp @@ -469,7 +469,7 @@ void OcdFileImport::importColors(const OcdFile& file) { const Ocd::ColorInfoV8& color_info = symbol_header.color_info[i]; const QString name = convertOcdString(color_info.name); - int color_pos = map->getNumColors(); + int color_pos = map->getNumColorPrios(); auto color = new MapColor(name, color_pos); // OC*D stores CMYK values as integers from 0-200. @@ -528,7 +528,7 @@ void OcdFileImport::importColors(const OcdFile& file) // Insert the spot colors into the map for (auto i = 0u; i < num_separations; ++i) { - map->addColor(spot_colors[i], map->getNumColors()); + map->addColor(spot_colors[i], map->getNumColorPrios()); } } @@ -545,7 +545,7 @@ void OcdFileImport::importColors(const OcdFile< F >& file) // Insert the spot colors into the map after (below) the regular colors. for (auto spot_color : spot_colors) { - map->addColor(spot_color, map->getNumColors()); + map->addColor(spot_color, map->getNumColorPrios()); } } @@ -716,7 +716,7 @@ void OcdFileImport::importColor(const QString& param_string) return; } - int color_pos = map->getNumColors(); + int color_pos = map->getNumColorPrios(); auto color = new MapColor(name, color_pos); color->setCmyk(cmyk); color->setOpacity(opacity); diff --git a/src/fileformats/xml_file_format.cpp b/src/fileformats/xml_file_format.cpp index 60d1a324b8..2a2f29e52b 100644 --- a/src/fileformats/xml_file_format.cpp +++ b/src/fileformats/xml_file_format.cpp @@ -884,7 +884,7 @@ void XMLFileImporter::importColors() SpotColorComponents out_components; for (auto&& in_component : item.components) { - const MapColor* out_color = map->getColor(in_component.spot_color->getPriority()); + const MapColor* out_color = map->getColorByPrio(in_component.spot_color->getPriority()); if (!out_color || out_color->getSpotColorMethod() != MapColor::SpotColor) { addWarning(tr("Spot color %1 not found while processing %2 (%3)."). diff --git a/src/gdal/ogr_file_format.cpp b/src/gdal/ogr_file_format.cpp index 37ac197cd5..604ef31c90 100644 --- a/src/gdal/ogr_file_format.cpp +++ b/src/gdal/ogr_file_format.cpp @@ -1415,10 +1415,10 @@ MapColor* OgrFileImport::makeColor(OGRStyleToolH tool, const char* color_string) } else if (a > 0) { - color = new MapColor(QString::fromUtf8(color_string), map->getNumColors()); + color = new MapColor(QString::fromUtf8(color_string), map->getNumColorPrios()); color->setRgb(QColor{ r, g, b }); color->setCmykFromRgb(); - map->addColor(color, map->getNumColors()); + map->addColor(color, map->getNumColorPrios()); } key.detach(); diff --git a/src/gui/map/map_editor.cpp b/src/gui/map/map_editor.cpp index f1900303c6..85257d5139 100644 --- a/src/gui/map/map_editor.cpp +++ b/src/gui/map/map_editor.cpp @@ -832,7 +832,7 @@ void MapEditorController::attach(MainWindow* window) createTemplateWindow(); createTagEditor(); - if (map->getNumColors() == 0) + if (map->getNumColorPrios() == 0) QTimer::singleShot(0, color_dock_widget, &QWidget::show); // Activate the edit tool @@ -4270,7 +4270,7 @@ QHash MapEditorController::importMap( bool merge_duplicate_symbols) { // Check if there is something to import - if (other.getNumColors() == 0 + if (other.getNumColorPrios() == 0 && other.getNumSymbols() == 0 && other.getNumObjects() == 0) { diff --git a/src/gui/map/map_widget.cpp b/src/gui/map/map_widget.cpp index 69bd9acd49..8671052604 100644 --- a/src/gui/map/map_widget.cpp +++ b/src/gui/map/map_widget.cpp @@ -902,7 +902,7 @@ void MapWidget::paintEvent(QPaintEvent* event) { painter.save(); painter.setTransform(transform); - if (view->getMap()->getNumColors() == 0) + if (view->getMap()->getNumColorPrios() == 0) showHelpMessage(&painter, tr("Empty map!\n\nStart by defining some colors:\nSelect Symbols -> Color window to\nopen the color dialog and\ndefine the colors there.")); else if (view->getMap()->getNumSymbols() == 0) showHelpMessage(&painter, tr("No symbols!\n\nNow define some symbols:\nRight-click in the symbol bar\nand select \"New symbol\"\nto create one.")); @@ -1267,7 +1267,7 @@ void MapWidget::updatePlaceholder() if (map->getNumObjects() > 1) return; - if (map->getNumColors() < 2 + if (map->getNumColorPrios() < 2 || map->getNumSymbols() < 2 || map->getNumTemplates() < 2) { diff --git a/src/gui/symbols/area_symbol_settings.cpp b/src/gui/symbols/area_symbol_settings.cpp index 40f79e9d01..ec27f3f1c8 100644 --- a/src/gui/symbols/area_symbol_settings.cpp +++ b/src/gui/symbols/area_symbol_settings.cpp @@ -463,10 +463,10 @@ void AreaSymbolSettings::addPattern(AreaSymbol::FillPattern::Type type) insertPropertiesGroup(group_index, temp_name, editor); } } - else if (map->getNumColors() > 0) + else if (map->getNumColorPrios() > 0) { // Default color for lines - active_pattern->line_color = map->getColor(0); + active_pattern->line_color = map->getColorByPrio(0); } updatePatternNames(); emit propertiesModified(); diff --git a/src/gui/widgets/color_dropdown.cpp b/src/gui/widgets/color_dropdown.cpp index 135057f0f7..8e7b2ce7ed 100644 --- a/src/gui/widgets/color_dropdown.cpp +++ b/src/gui/widgets/color_dropdown.cpp @@ -45,10 +45,10 @@ ColorDropDown::ColorDropDown(const Map* map, const MapColor* initial_color, bool QPixmap pixmap(icon_size, icon_size); int initial_index = 0; - int num_colors = map->getNumColors(); + int num_colors = map->getNumColorPrios(); for (int i = 0; i < num_colors; ++i) { - const MapColor* color = map->getColor(i); + const MapColor* color = map->getColorByPrio(i); if (spot_colors_only && color->getSpotColorMethod() != MapColor::SpotColor) continue; diff --git a/src/gui/widgets/color_list_widget.cpp b/src/gui/widgets/color_list_widget.cpp index c7daf3bd72..b1dd185da2 100644 --- a/src/gui/widgets/color_list_widget.cpp +++ b/src/gui/widgets/color_list_widget.cpp @@ -84,7 +84,7 @@ ColorListWidget::ColorListWidget(Map* map, MainWindow* window, QWidget* parent) react_to_changes = true; // Color table - color_table = new QTableWidget(map->getNumColors(), 7); + color_table = new QTableWidget(map->getNumColorPrios(), 7); color_table->setEditTriggers(QAbstractItemView::SelectedClicked | QAbstractItemView::AnyKeyPressed); color_table->setSelectionMode(QAbstractItemView::SingleSelection); color_table->setSelectionBehavior(QAbstractItemView::SelectRows); @@ -153,7 +153,7 @@ ColorListWidget::ColorListWidget(Map* map, MainWindow* window, QWidget* parent) layout->addLayout(bottom_layout); setLayout(layout); - for (int i = 0; i < map->getNumColors(); ++i) + for (int i = 0; i < map->getNumColorPrios(); ++i) addRow(i); auto header_view = color_table->horizontalHeader(); @@ -164,7 +164,7 @@ ColorListWidget::ColorListWidget(Map* map, MainWindow* window, QWidget* parent) header_view->setSectionResizeMode(5, QHeaderView::Fixed); // Knockout header_view->resizeSection(5, 32); header_view->setSectionsClickable(false); - if (map->getNumColors() == 0) + if (map->getNumColorPrios() == 0) { header_view->resizeSection(1, 160); // Name header_view->resizeSection(2, 128); // Spot colors @@ -203,7 +203,7 @@ void ColorListWidget::showEvent(QShowEvent* event) // Update name, because translation may be changed with new symbol set for (int i = 0, count = color_table->rowCount(); i < count; ++i) { - auto color = map->getColor(i); + auto color = map->getColorByPrio(i); auto item = color_table->item(i, 1); item->setText(map->translate(color->getName())); } @@ -254,7 +254,7 @@ bool ColorListWidget::confirmColorDeletion(const MapColor* color_to_be_removed) // Second: usage as spot color QString spotcolor_usage; std::vector affected_colors; - affected_colors.reserve(std::size_t(map->getNumColors())); + affected_colors.reserve(std::size_t(map->getNumColorPrios())); map->applyOnMatchingColors( [&spotcolor_usage, &affected_colors](const auto* color) { spotcolor_usage += color->getName() + QChar::LineFeed; @@ -309,7 +309,7 @@ void ColorListWidget::deleteColor() if (row < 0) return; // In release mode // Show a warning if the color is used - if (!confirmColorDeletion(map->getColor(row))) + if (!confirmColorDeletion(map->getColorByPrio(row))) return; map->deleteColor(row); @@ -324,7 +324,7 @@ void ColorListWidget::duplicateColor() Q_ASSERT(row >= 0); if (row < 0) return; // In release mode - auto new_color = new MapColor(*map->getColor(row)); + auto new_color = new MapColor(*map->getColorByPrio(row)); //: Future replacement for COLOR_NAME + " (Duplicate)", for better localization. void(tr("%1 (duplicate)")); /// \todo Switch translation new_color->setName(map->translate(new_color->getName()) + tr(" (Duplicate)")); @@ -341,8 +341,8 @@ void ColorListWidget::moveColorUp() Q_ASSERT(row >= 1); if (row < 1) return; // In release mode - auto above_color = map->getMapColor(row - 1); - auto cur_color = map->getMapColor(row); + auto above_color = map->getMapColorByPrio(row - 1); + auto cur_color = map->getMapColorByPrio(row); map->setColor(cur_color, row - 1); map->setColor(above_color, row); updateRow(row - 1); @@ -360,8 +360,8 @@ void ColorListWidget::moveColorDown() Q_ASSERT(row < color_table->rowCount() - 1); if (row >= color_table->rowCount() - 1) return; // In release mode - auto below_color = map->getMapColor(row + 1); - auto cur_color = map->getMapColor(row); + auto below_color = map->getMapColorByPrio(row + 1); + auto cur_color = map->getMapColorByPrio(row); map->setColor(cur_color, row + 1); map->setColor(below_color, row); updateRow(row + 1); @@ -379,7 +379,7 @@ void ColorListWidget::editCurrentColor() int row = color_table->currentRow(); if (row >= 0) { - auto color = map->getMapColor(row); + auto color = map->getMapColorByPrio(row); ColorDialog dialog(*map, *color, this); dialog.setWindowModality(Qt::WindowModal); int result = dialog.exec(); @@ -405,7 +405,7 @@ void ColorListWidget::cellChange(int row, int column) react_to_changes = false; - auto color = map->getMapColor(row); + auto color = map->getMapColorByPrio(row); auto text = color_table->item(row, column)->text().trimmed(); if (column == 1) @@ -509,7 +509,7 @@ void ColorListWidget::updateRow(int row) { react_to_changes = false; - const auto* color = map->getColor(row); + const auto* color = map->getColorByPrio(row); auto color_with_opacity = colorWithOpacity(*color); // Color preview diff --git a/src/tools/fill_tool.cpp b/src/tools/fill_tool.cpp index d9050486da..d8314c27db 100644 --- a/src/tools/fill_tool.cpp +++ b/src/tools/fill_tool.cpp @@ -323,10 +323,10 @@ void FillTool::drawObjectIDs(Map* map, QPainter* painter, const RenderConfig &co auto part = map->getCurrentPart(); auto num_objects = qMin(part->getNumObjects(), int(RGB_MASK)); - auto num_colors = map->getNumColors(); + auto num_colors = map->getNumColorPrios(); for (auto c = num_colors-1; c >= MapColor::Reserved; --c) { - auto map_color = map->getColor(c); + auto map_color = map->getColorByPrio(c); for (int o = 0; o < num_objects; ++o) { auto object = part->getObject(o); diff --git a/test/file_format_t.cpp b/test/file_format_t.cpp index 99d98bb405..4ead039c67 100644 --- a/test/file_format_t.cpp +++ b/test/file_format_t.cpp @@ -237,13 +237,13 @@ namespace QCOMPARE(actual.isBaselineViewEnabled(), expected.isBaselineViewEnabled()); // Colors - if (actual.getNumColors() != expected.getNumColors()) + if (actual.getNumColorPrios() != expected.getNumColorPrios()) { - QCOMPARE(actual.getNumColors(), expected.getNumColors()); + QCOMPARE(actual.getNumColorPrios(), expected.getNumColorPrios()); } - else for (int i = 0; i < actual.getNumColors(); ++i) + else for (int i = 0; i < actual.getNumColorPrios(); ++i) { - QCOMPARE(*actual.getColor(i), *expected.getColor(i)); + QCOMPARE(*actual.getColorByPrio(i), *expected.getColorByPrio(i)); } // Symbols @@ -448,8 +448,8 @@ namespace QCOMPARE(actual_georef.toGeographicCoords(actual_point), expected_georef.toGeographicCoords(actual_point)); // Colors - QVERIFY2(actual.getNumColors() >= expected.getNumColors(), qPrintable(test_label.arg(actual.getNumColors()).arg(expected.getNumColors()))); - QVERIFY2(actual.getNumColors() <= 2 * expected.getNumColors(), qPrintable(test_label.arg(actual.getNumColors()).arg(expected.getNumColors()))); + QVERIFY2(actual.getNumColorPrios() >= expected.getNumColorPrios(), qPrintable(test_label.arg(actual.getNumColorPrios()).arg(expected.getNumColorPrios()))); + QVERIFY2(actual.getNumColorPrios() <= 2 * expected.getNumColorPrios(), qPrintable(test_label.arg(actual.getNumColorPrios()).arg(expected.getNumColorPrios()))); // Symbols // Combined symbols may be dropped (split) on export. diff --git a/test/map_t.cpp b/test/map_t.cpp index 9a1b6e8f19..0adab91e35 100644 --- a/test/map_t.cpp +++ b/test/map_t.cpp @@ -144,32 +144,32 @@ void MapTest::specialColorsTest() Map map; // non-const const Map& cmap(map); // const - QCOMPARE(map.getMapColor(MapColor::CoveringWhite), static_cast(nullptr)); - QCOMPARE(cmap.getMapColor(MapColor::CoveringWhite), static_cast(nullptr)); - QCOMPARE(map.getColor(MapColor::CoveringWhite), Map::getCoveringWhite()); - QCOMPARE(cmap.getColor(MapColor::CoveringWhite), Map::getCoveringWhite()); - QCOMPARE(map.getMapColor(MapColor::CoveringRed), static_cast(nullptr)); - QCOMPARE(cmap.getMapColor(MapColor::CoveringRed), static_cast(nullptr)); - QCOMPARE(map.getColor(MapColor::CoveringRed), Map::getCoveringRed()); - QCOMPARE(cmap.getColor(MapColor::CoveringRed), Map::getCoveringRed()); - QCOMPARE(map.getMapColor(MapColor::Undefined), static_cast(nullptr)); - QCOMPARE(cmap.getMapColor(MapColor::Undefined), static_cast(nullptr)); - QCOMPARE(map.getColor(MapColor::Undefined), Map::getUndefinedColor()); - QCOMPARE(cmap.getColor(MapColor::Undefined), Map::getUndefinedColor()); - QCOMPARE(map.getMapColor(MapColor::Registration), static_cast(nullptr)); - QCOMPARE(cmap.getMapColor(MapColor::Registration), static_cast(nullptr)); - QCOMPARE(map.getColor(MapColor::Registration), Map::getRegistrationColor()); - QCOMPARE(cmap.getColor(MapColor::Registration), Map::getRegistrationColor()); - - QCOMPARE(map.getMapColor(MapColor::Reserved), static_cast(nullptr)); - QCOMPARE(cmap.getMapColor(MapColor::Reserved), static_cast(nullptr)); - QCOMPARE(map.getColor(MapColor::Reserved), static_cast(nullptr)); - QCOMPARE(cmap.getColor(MapColor::Reserved), static_cast(nullptr)); - - QCOMPARE(map.getMapColor(map.getNumColors()), static_cast(nullptr)); - QCOMPARE(cmap.getMapColor(cmap.getNumColors()), static_cast(nullptr)); - QCOMPARE(map.getColor(map.getNumColors()), static_cast(nullptr)); - QCOMPARE(cmap.getColor(cmap.getNumColors()), static_cast(nullptr)); + QCOMPARE(map.getMapColorByPrio(MapColor::CoveringWhite), static_cast(nullptr)); + QCOMPARE(cmap.getMapColorByPrio(MapColor::CoveringWhite), static_cast(nullptr)); + QCOMPARE(map.getColorByPrio(MapColor::CoveringWhite), Map::getCoveringWhite()); + QCOMPARE(cmap.getColorByPrio(MapColor::CoveringWhite), Map::getCoveringWhite()); + QCOMPARE(map.getMapColorByPrio(MapColor::CoveringRed), static_cast(nullptr)); + QCOMPARE(cmap.getMapColorByPrio(MapColor::CoveringRed), static_cast(nullptr)); + QCOMPARE(map.getColorByPrio(MapColor::CoveringRed), Map::getCoveringRed()); + QCOMPARE(cmap.getColorByPrio(MapColor::CoveringRed), Map::getCoveringRed()); + QCOMPARE(map.getMapColorByPrio(MapColor::Undefined), static_cast(nullptr)); + QCOMPARE(cmap.getMapColorByPrio(MapColor::Undefined), static_cast(nullptr)); + QCOMPARE(map.getColorByPrio(MapColor::Undefined), Map::getUndefinedColor()); + QCOMPARE(cmap.getColorByPrio(MapColor::Undefined), Map::getUndefinedColor()); + QCOMPARE(map.getMapColorByPrio(MapColor::Registration), static_cast(nullptr)); + QCOMPARE(cmap.getMapColorByPrio(MapColor::Registration), static_cast(nullptr)); + QCOMPARE(map.getColorByPrio(MapColor::Registration), Map::getRegistrationColor()); + QCOMPARE(cmap.getColorByPrio(MapColor::Registration), Map::getRegistrationColor()); + + QCOMPARE(map.getMapColorByPrio(MapColor::Reserved), static_cast(nullptr)); + QCOMPARE(cmap.getMapColorByPrio(MapColor::Reserved), static_cast(nullptr)); + QCOMPARE(map.getColorByPrio(MapColor::Reserved), static_cast(nullptr)); + QCOMPARE(cmap.getColorByPrio(MapColor::Reserved), static_cast(nullptr)); + + QCOMPARE(map.getMapColorByPrio(map.getNumColorPrios()), static_cast(nullptr)); + QCOMPARE(cmap.getMapColorByPrio(cmap.getNumColorPrios()), static_cast(nullptr)); + QCOMPARE(map.getColorByPrio(map.getNumColorPrios()), static_cast(nullptr)); + QCOMPARE(cmap.getColorByPrio(cmap.getNumColorPrios()), static_cast(nullptr)); } void MapTest::importTest_data() @@ -194,19 +194,19 @@ void MapTest::importTest() QVERIFY(map.getNumSymbols() > 0); auto original_size = map.getNumObjects(); - auto original_num_colors = map.getNumColors(); + auto original_num_colors = map.getNumColorPrios(); Map empty_map; map.importMap(empty_map, Map::CompleteImport); QCOMPARE(map.getNumObjects(), original_size); - QCOMPARE(map.getNumColors(), original_num_colors); + QCOMPARE(map.getNumColorPrios(), original_num_colors); map.importMap(map, Map::ColorImport); QCOMPARE(map.getNumObjects(), original_size); - QCOMPARE(map.getNumColors(), original_num_colors); + QCOMPARE(map.getNumColorPrios(), original_num_colors); map.importMap(map, Map::CompleteImport); QCOMPARE(map.getNumObjects(), 2*original_size); - QCOMPARE(map.getNumColors(), original_num_colors); + QCOMPARE(map.getNumColorPrios(), original_num_colors); QString imported_path = examples_dir.absoluteFilePath(imported_file); Map imported_map; @@ -239,7 +239,7 @@ void MapTest::hasAlpha() auto* color = [&map, symbol]() -> MapColor* { auto* color = symbol->guessDominantColor(); - return color ? map.getMapColor(map.findColorIndex(color)) : nullptr; + return color ? map.getMapColorByPrio(map.findColorPrio(color)) : nullptr; }(); QVERIFY(color); diff --git a/test/symbol_set_t.cpp b/test/symbol_set_t.cpp index 13877b0520..6dc5572bad 100644 --- a/test/symbol_set_t.cpp +++ b/test/symbol_set_t.cpp @@ -100,10 +100,10 @@ void processTranslation(const Map& map, TranslationEntries& translation_entries) { auto const id = map.symbolSetId(); - auto num_colors = map.getNumColors(); + auto num_colors = map.getNumColorPrios(); for (int i = 0; i < num_colors; ++i) { - auto* color = map.getColor(i); + auto* color = map.getColorByPrio(i); auto const& source = color->getName(); auto comment = QString{QLatin1String("Color ") + QString::number(color->getPriority())}; addSource(translation_entries, id, source, comment); @@ -435,11 +435,11 @@ void mergeISOM(Map& target, const QDir& symbol_set_dir) // Delete some colors, and mark the symbols which use these colors auto num_colors_deleted = 0; - auto const num_colors = map.getNumColors(); + auto const num_colors = map.getNumColorPrios(); for (auto i = num_colors; i > 0; --i) { auto const index = i - 1; - auto* const color = map.getColor(index); + auto* const color = map.getColorByPrio(index); if (color->getSpotColorName() == QStringLiteral("GREEN 100, BLACK 50") || color->getSpotColorName() == QStringLiteral("GREEN 60") ) { @@ -818,9 +818,9 @@ void SymbolSetTool::processSymbolSet() map.resetPrinterConfig(); map.undoManager().clear(); - for (int i = 0; i < map.getNumColors(); ++i) + for (int i = 0; i < map.getNumColorPrios(); ++i) { - auto color = map.getMapColor(i); + auto color = map.getMapColorByPrio(i); if (color->getSpotColorMethod() == MapColor::CustomColor) { color->setCmykFromSpotColors(); From ba1fec32d9452581530368e8767d553b3a9217ae Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:45:17 +0200 Subject: [PATCH 2/8] MapColor, XMLFile: Introduce color id The color id value will help us reference the map color in file format context. Currently colors are addressed by its rendering priority. The goal is to address colors by its stable id in a future file format. For now, we do not compare the color id in MapColor::equals() as there is no use for it. The id is not part of the color identity, much like its priority says nothing about the color itself. --- src/core/map.cpp | 21 ++++++ src/core/map.h | 17 ++++- src/core/map_color.cpp | 11 +++ src/core/map_color.h | 12 ++++ src/fileformats/xml_file_format.cpp | 32 +++++++++ src/fileformats/xml_file_format.h | 2 + test/data/colors/color-id.omap | 29 ++++++++ test/file_format_t.cpp | 106 ++++++++++++++++++++++++++++ test/file_format_t.h | 7 ++ 9 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 test/data/colors/color-id.omap diff --git a/src/core/map.cpp b/src/core/map.cpp index 0df3495089..7bf1029016 100644 --- a/src/core/map.cpp +++ b/src/core/map.cpp @@ -139,11 +139,32 @@ Map::MapColorSet::~MapColorSet() void Map::MapColorSet::insert(int pos, MapColor* color) { colors.insert(colors.begin() + pos, color); + if (color->getId() >= 0 && !ids[color->getId()]) + { + ids[color->getId()] = color; + } + else + { + // Prefer ids starting at one. + auto free_spot = std::find(begin(ids) + 1, end(ids), nullptr); + if (free_spot == end(ids)) + { + color->setId(ids.size()); + ids.push_back(color); + } + else + { + auto const n = free_spot - begin(ids); + color->setId(n); + ids[n] = color; + } + } adjustColorPriorities(pos + 1, colors.size()); } void Map::MapColorSet::erase(int pos) { + ids[colors[pos]->getId()] = nullptr; colors.erase(colors.begin() + pos); adjustColorPriorities(pos, colors.size()); } diff --git a/src/core/map.h b/src/core/map.h index 189ac7886f..f2e17c0330 100644 --- a/src/core/map.h +++ b/src/core/map.h @@ -428,6 +428,15 @@ friend class XMLFileExporter; * Returns nullptr if the color is not defined. */ const MapColor* getColorByPrio(int i) const; + + /** Returns a pointer to the const MapColor with id id. + * + * Mind that the id's are not guaranteed to be contigous and cannot be + * used for iteration over colors. + * + * \return nullptr in case the id is not present in the set. + */ + const MapColor* getMapColorById(int id) const; /** * Replaces the color with priority prio with the given color, and updates @@ -1549,7 +1558,8 @@ protected slots: class MapColorSet : public QSharedData { public: - ColorVector colors; + ColorVector colors; ///< Colors ordered by priority. Compact vector. + ColorVector ids; ///< Colors indexed by their id. Sparse with nullptrs. MapColorSet(); @@ -1711,6 +1721,11 @@ const MapColor* Map::getColorByPrio(int i) const } } +inline +const MapColor* Map::getMapColorById(int id) const +{ + return (id >= 0 && id < static_cast(color_set->ids.size())) ? color_set->ids[id] : nullptr; +} inline diff --git a/src/core/map_color.cpp b/src/core/map_color.cpp index 94255c876d..0967f71487 100644 --- a/src/core/map_color.cpp +++ b/src/core/map_color.cpp @@ -101,6 +101,7 @@ MapColor::MapColor(const QString& name, int priority) MapColor* MapColor::duplicate() const { MapColor* copy = new MapColor(name, priority); + copy->color_id = color_id; copy->cmyk = cmyk; copy->rgb = rgb; copy->opacity = opacity; @@ -116,6 +117,16 @@ MapColor* MapColor::duplicate() const return copy; } +int MapColor::getId() const +{ + return color_id; +} + +void MapColor::setId(int id) +{ + color_id = id; +} + bool MapColor::isBlack() const { return rgb.isBlack() && cmyk.isBlack(); diff --git a/src/core/map_color.h b/src/core/map_color.h index b4ef8be1d9..11c8068bcc 100644 --- a/src/core/map_color.h +++ b/src/core/map_color.h @@ -242,6 +242,14 @@ class MapColor * Normally you don't want to call this directly. */ void setPriority(int priority); + + /** The color id value helps us reference the map color in file format + * context. In the native XML it helps to maintain stable link between + * symbols and colors. */ + int getId() const; + + /** \see getId() */ + void setId(int id); /** @deprecated Returns the color's opacity. */ float getOpacity() const; @@ -469,6 +477,10 @@ class MapColor QString name; + /** The color_id is a map-dependent value managed by the map context. + * Still it is stored in the MapColor class much like the priority value. + */ + int color_id = -1; int priority; MapColorCmyk cmyk; diff --git a/src/fileformats/xml_file_format.cpp b/src/fileformats/xml_file_format.cpp index 2a2f29e52b..bb38adc4dc 100644 --- a/src/fileformats/xml_file_format.cpp +++ b/src/fileformats/xml_file_format.cpp @@ -327,6 +327,8 @@ void XMLFileExporter::exportColors() MapColor* color = map->color_set->colors[i]; const MapColorCmyk &cmyk = color->getCmyk(); XmlElementWriter color_element(xml, literal::color); + if (color->getId() >= 0) + color_element.writeAttribute(literal::id, color->getId()); color_element.writeAttribute(literal::priority, color->getPriority()); color_element.writeAttribute(literal::name, color->getName()); color_element.writeAttribute(literal::c, cmyk.c, 3); @@ -769,6 +771,8 @@ void XMLFileImporter::importColors() auto color = std::make_unique( color_element.attribute(literal::name), color_element.attribute(literal::priority) ); + if (color_element.hasAttribute(literal::id)) + color->setId(color_element.attribute(literal::id)); if (color_element.hasAttribute(literal::opacity)) color->setOpacity(color_element.attribute(literal::opacity)); @@ -870,6 +874,34 @@ void XMLFileImporter::importColors() } } + // Given that we access the color vector directly, we got to handle the id + // assignment as well. + auto& ids(map->color_set->ids); + std::vector colors_to_fix; + auto const max_element = std::max_element(begin(colors), end(colors), [](auto a, auto b) { return a->getId() < b->getId(); }); + auto const max_id = (max_element == end(colors)) ? -1 : (*max_element)->getId(); + ids.resize(std::max(max_id, static_cast(colors.size())) + 1, nullptr); + + for (auto* color : colors) + { + if (color->getId() >= 0) + ids[color->getId()] = color; + else + colors_to_fix.push_back(color); + } + + for (auto* color : colors_to_fix) + { + // Assign new ids starting from one. + auto free_spot = std::find(begin(ids) + 1, end(ids), nullptr); + auto assigned_id = free_spot - begin(ids); + color->setId(assigned_id); + if (assigned_id < static_cast(ids.size())) + ids[assigned_id] = color; + else + ids.push_back(color); + } + if (num_colors > 0 && num_colors != colors.size()) addWarning(tr("Expected %1 colors, found %2."). arg(num_colors). diff --git a/src/fileformats/xml_file_format.h b/src/fileformats/xml_file_format.h index f7b45f3cb7..6e754539e6 100644 --- a/src/fileformats/xml_file_format.h +++ b/src/fileformats/xml_file_format.h @@ -96,6 +96,7 @@ class XMLFileFormat : public FileFormat - For writing, drop compatibility with Mapper versions before 0.9. - Use the streaming variant when writing `barrier` elements. - Stop writing text object box sizes to the coordinates stream. +- Use the stable color id to reference colors from symbols. \subsection version-9 Version 9 @@ -108,6 +109,7 @@ class XMLFileFormat : public FileFormat - 2019-10-02 Added a text symbol `rotatable` property. This must be exported as `true` now when the text symbol is rotatable, but default to `true` when reading previous versions of the format. +- 2023-03-23 Added a stable id number to colors. \subsection version-8 Version 8 diff --git a/test/data/colors/color-id.omap b/test/data/colors/color-id.omap new file mode 100644 index 0000000000..ab29c5c496 --- /dev/null +++ b/test/data/colors/color-id.omap @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/file_format_t.cpp b/test/file_format_t.cpp index 4ead039c67..b3b01e014a 100644 --- a/test/file_format_t.cpp +++ b/test/file_format_t.cpp @@ -1302,6 +1302,112 @@ void FileFormatTest::ocdTextImportTest() } +void FileFormatTest::colorTest_data() +{ + QTest::addColumn("filepath"); + QTest::addColumn("format_id"); + + QTest::newRow(QByteArray {"Load color table - XML"}) + << QStringLiteral("data:colors/color-id.omap") + << QByteArray {"XML"}; +} + + +namespace QTest { + // Helper for textual MapColor output in QCOMPARE(). + template<> + char *toString(const MapColor& color) + { + auto color_method_name = [](MapColor::ColorMethod m) { + auto value = static_cast(m); + if (!value) + return QString::fromLatin1("UndefinedMethod"); + + QStringList flag_names; + for (auto& method_name : { "CustomColor", "SpotColor", + "CmykColor", "RgbColor", "Knockout" }) + { + if (value & 1) + flag_names.push_back(QString::fromLatin1(method_name)); + value >>= 1; + } + return flag_names.join(QChar::fromLatin1('|')); + }; + + auto ret = QString::fromLatin1("n:'%1' id:%2 p:%3 scm:%4 ccm:%5 rcm:%6 ko:%7 cmyk:(%8,%9,%10,%11) rgb:%12 scn:'%13' sf:%14 sa:%15 o:%16 sep:[") + .arg(color.getName()) + .arg(color.getId()) + .arg(color.getPriority()) + .arg(color_method_name(color.getSpotColorMethod()), + color_method_name(color.getCmykColorMethod()), + color_method_name(color.getRgbColorMethod())) + .arg(color.getKnockout()) + .arg(color.getCmyk().c) + .arg(color.getCmyk().m) + .arg(color.getCmyk().y) + .arg(color.getCmyk().k) + .arg(QColor(color.getRgb()).name(), + color.getSpotColorName()) + .arg(color.getScreenFrequency()) + .arg(color.getScreenAngle()) + .arg(color.getOpacity()); + QStringList component_descs; + for (auto const& component : color.getComponents()) + component_descs.push_back(QString::fromLatin1("%1/%2") + .arg(component.spot_color->getName()) + .arg(component.factor)); + ret.append(component_descs.join(QString::fromLatin1(", "))); + ret.append(QLatin1String("]")); + return QTest::toString(ret); + } +} // namespace QTest + + +void FileFormatTest::colorTest() +{ + QFETCH(QString, filepath); + QFETCH(QByteArray, format_id); + QVERIFY(QFileInfo::exists(filepath)); + + { + auto original = std::make_unique(); + + QVERIFY(original->loadFrom(filepath)); + + // Save the sample to the format identified by format_id and load it + // again to see if the target format maintains the color number as + // well. + std::unique_ptr copy; + auto const* format = FileFormats.findFormat(format_id); + QVERIFY(format); + copy = saveAndLoadMap(*original, format); + QVERIFY(copy); + + // Failure here means that we failed to persist the color properties in + // the target file format. + for (auto i = 0; i < original->getNumColorPrios(); ++i) + QCOMPARE(*original->getColorByPrio(i), *copy->getColorByPrio(i)); + + // Failure here means that we failed to persist the link between + // symbols and colors. + for (auto i = 0; i < original->getNumSymbols(); ++i) + { + auto* symbol_in_original = original->getSymbol(i); + for (auto color_prio = 0; color_prio < original->getNumColorPrios(); ++color_prio) + { + auto const* color_in_original = original->getColorByPrio(color_prio); + if (symbol_in_original->containsColor(color_in_original)) + { + auto const* symbol_in_copy = copy->getSymbol(i); + auto const* color_in_copy = copy->getColorByPrio(color_prio); + QVERIFY(symbol_in_copy->containsColor(color_in_copy)); + QCOMPARE(*color_in_original, *color_in_copy); + } + } + } + } +} + /* * We don't need a real GUI window. diff --git a/test/file_format_t.h b/test/file_format_t.h index 1eb8cec017..03d047e910 100644 --- a/test/file_format_t.h +++ b/test/file_format_t.h @@ -121,6 +121,13 @@ private slots: */ void ocdTextImportTest_data(); void ocdTextImportTest(); + + /** + * Test color traits persistence and the link to symbols. + */ + void colorTest_data(); + void colorTest(); + }; #endif // OPENORIENTEERING_FILE_FORMAT_T_H From 78b2b95db321a5f6337eebce1d74b45687bd886c Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:49:40 +0200 Subject: [PATCH 3/8] Update examples and symbol sets with color id The introduction of color id makes the tests alter example and symbol set sources. --- examples/complete map.omap | 78 +++++++++++----------- examples/forest sample.omap | 44 ++++++------ examples/overprinting.omap | 44 ++++++------ examples/src/complete map.xmap | 78 +++++++++++----------- examples/src/forest sample.xmap | 44 ++++++------ examples/src/overprinting.xmap | 44 ++++++------ symbol sets/10000/Course_Design_10000.omap | 12 ++-- symbol sets/10000/ISMTBOM_10000.omap | 50 +++++++------- symbol sets/10000/ISOM 2017-2_10000.omap | 70 +++++++++---------- symbol sets/10000/ISSkiOM 2019_10000.omap | 72 ++++++++++---------- symbol sets/12500/ISSkiOM 2019_12500.omap | 72 ++++++++++---------- symbol sets/15000/Course_Design_15000.omap | 12 ++-- symbol sets/15000/ISMTBOM_15000.omap | 50 +++++++------- symbol sets/15000/ISOM 2017-2_15000.omap | 70 +++++++++---------- symbol sets/15000/ISSkiOM 2019_15000.omap | 72 ++++++++++---------- symbol sets/20000/ISMTBOM_20000.omap | 50 +++++++------- symbol sets/4000/Course_Design_4000.omap | 12 ++-- symbol sets/4000/ISSprOM 2019_4000.omap | 64 +++++++++--------- symbol sets/5000/Course_Design_5000.omap | 12 ++-- symbol sets/5000/ISMTBOM_5000.omap | 50 +++++++------- symbol sets/5000/ISSkiOM 2019_5000.omap | 72 ++++++++++---------- symbol sets/7500/ISMTBOM_7500.omap | 50 +++++++------- symbol sets/7500/ISSkiOM 2019_7500.omap | 72 ++++++++++---------- symbol sets/src/Course_Design_10000.xmap | 12 ++-- symbol sets/src/ISMTBOM_15000.xmap | 50 +++++++------- symbol sets/src/ISOM 2017-2_15000.xmap | 70 +++++++++---------- symbol sets/src/ISOM2000_15000.xmap | 44 ++++++------ symbol sets/src/ISOM2017_15000.xmap | 56 ++++++++-------- symbol sets/src/ISSOM_5000.xmap | 72 ++++++++++---------- symbol sets/src/ISSkiOM 2019_15000.xmap | 72 ++++++++++---------- symbol sets/src/ISSkiOM_15000.xmap | 48 ++++++------- symbol sets/src/ISSprOM 2019_4000.xmap | 64 +++++++++--------- test/data/templates/world-file.xmap | 2 +- 33 files changed, 842 insertions(+), 842 deletions(-) diff --git a/examples/complete map.omap b/examples/complete map.omap index 77df4f5f21..e5ad5a7133 100644 --- a/examples/complete map.omap +++ b/examples/complete map.omap @@ -3,45 +3,45 @@ +proj=utm +datum=WGS84 +zone=3232 N+proj=latlong +datum=WGS84 -PURPLE - -BLACK -RED - - - - - - - - - - - - - - -BLUE - - - - -BROWN - - - - - - -GREEN - - - -YELLOW - - - - +PURPLE + +BLACK +RED + + + + + + + + + + + + + + +BLUE + + + + +BROWN + + + + + + +GREEN + + + +YELLOW + + + + diff --git a/examples/forest sample.omap b/examples/forest sample.omap index 32c8d2813f..81e2ff7abc 100644 --- a/examples/forest sample.omap +++ b/examples/forest sample.omap @@ -3,28 +3,28 @@ -PURPLE -BLACK - - - -BLUE - -BROWN - - - - - - -GREEN - - - -YELLOW - - - +PURPLE +BLACK + + + +BLUE + +BROWN + + + + + + +GREEN + + + +YELLOW + + + diff --git a/examples/overprinting.omap b/examples/overprinting.omap index 9041b015d2..04b2c97ff4 100644 --- a/examples/overprinting.omap +++ b/examples/overprinting.omap @@ -3,28 +3,28 @@ -PURPLE -BLACK - - - -BLUE - -BROWN - - - - - - -GREEN - - - -YELLOW - - - +PURPLE +BLACK + + + +BLUE + +BROWN + + + + + + +GREEN + + + +YELLOW + + + diff --git a/examples/src/complete map.xmap b/examples/src/complete map.xmap index 0e190ffbfe..08389d404c 100644 --- a/examples/src/complete map.xmap +++ b/examples/src/complete map.xmap @@ -14,77 +14,77 @@ - + PURPLE - + - + BLACK - + RED - + - + - + - + - + - + - + @@ -92,21 +92,21 @@ - + - + - + @@ -114,77 +114,77 @@ - + - + - + - + - + BLUE - + - + - + - + - + BROWN - + @@ -192,14 +192,14 @@ - + - + @@ -207,14 +207,14 @@ - + - + @@ -222,70 +222,70 @@ - + - + GREEN - + - + - + - + YELLOW - + - + - + - + diff --git a/examples/src/forest sample.xmap b/examples/src/forest sample.xmap index ff7c283dc2..6ccaad4fdb 100644 --- a/examples/src/forest sample.xmap +++ b/examples/src/forest sample.xmap @@ -5,63 +5,63 @@ - + PURPLE - + BLACK - + - + - + - + BLUE - + - + BROWN - + @@ -69,14 +69,14 @@ - + - + @@ -84,77 +84,77 @@ - + - + - + - + GREEN - + - + - + - + YELLOW - + - + - + diff --git a/examples/src/overprinting.xmap b/examples/src/overprinting.xmap index 97756e6476..5e0b1ce995 100644 --- a/examples/src/overprinting.xmap +++ b/examples/src/overprinting.xmap @@ -5,63 +5,63 @@ - + PURPLE - + BLACK - + - + - + - + BLUE - + - + BROWN - + @@ -69,14 +69,14 @@ - + - + @@ -84,77 +84,77 @@ - + - + - + - + GREEN - + - + - + - + YELLOW - + - + - + diff --git a/symbol sets/10000/Course_Design_10000.omap b/symbol sets/10000/Course_Design_10000.omap index 6dd1f38abb..4650589e7c 100644 --- a/symbol sets/10000/Course_Design_10000.omap +++ b/symbol sets/10000/Course_Design_10000.omap @@ -3,12 +3,12 @@ -PURPLE - - - - -BLACK +PURPLE + + + + +BLACK diff --git a/symbol sets/10000/ISMTBOM_10000.omap b/symbol sets/10000/ISMTBOM_10000.omap index cbb624bec6..552a28ad65 100644 --- a/symbol sets/10000/ISMTBOM_10000.omap +++ b/symbol sets/10000/ISMTBOM_10000.omap @@ -3,31 +3,31 @@ -PURPLE -BLACK - - - - -BROWN - - -BLUE - - - - - - - -GREEN - - - -YELLOW - - - +PURPLE +BLACK + + + + +BROWN + + +BLUE + + + + + + + +GREEN + + + +YELLOW + + + diff --git a/symbol sets/10000/ISOM 2017-2_10000.omap b/symbol sets/10000/ISOM 2017-2_10000.omap index a1c8268427..9caf785d4b 100644 --- a/symbol sets/10000/ISOM 2017-2_10000.omap +++ b/symbol sets/10000/ISOM 2017-2_10000.omap @@ -3,41 +3,41 @@ -PURPLE - -BLACK -GREEN - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - - - - - -YELLOW - - +PURPLE + +BLACK +GREEN + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + + + + + +YELLOW + + diff --git a/symbol sets/10000/ISSkiOM 2019_10000.omap b/symbol sets/10000/ISSkiOM 2019_10000.omap index 6a3b998c00..5675bfbd94 100644 --- a/symbol sets/10000/ISSkiOM 2019_10000.omap +++ b/symbol sets/10000/ISSkiOM 2019_10000.omap @@ -3,42 +3,42 @@ -PURPLE - - -BLACK -GREEN - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - - - - - -YELLOW - - +PURPLE + + +BLACK +GREEN + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + + + + + +YELLOW + + diff --git a/symbol sets/12500/ISSkiOM 2019_12500.omap b/symbol sets/12500/ISSkiOM 2019_12500.omap index d35849daa5..d1906265b6 100644 --- a/symbol sets/12500/ISSkiOM 2019_12500.omap +++ b/symbol sets/12500/ISSkiOM 2019_12500.omap @@ -3,42 +3,42 @@ -PURPLE - - -BLACK -GREEN - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - - - - - -YELLOW - - +PURPLE + + +BLACK +GREEN + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + + + + + +YELLOW + + diff --git a/symbol sets/15000/Course_Design_15000.omap b/symbol sets/15000/Course_Design_15000.omap index 5cda9d8350..19861b3207 100644 --- a/symbol sets/15000/Course_Design_15000.omap +++ b/symbol sets/15000/Course_Design_15000.omap @@ -3,12 +3,12 @@ -PURPLE - - - - -BLACK +PURPLE + + + + +BLACK diff --git a/symbol sets/15000/ISMTBOM_15000.omap b/symbol sets/15000/ISMTBOM_15000.omap index 7b6fc50ca9..bec74e00f0 100644 --- a/symbol sets/15000/ISMTBOM_15000.omap +++ b/symbol sets/15000/ISMTBOM_15000.omap @@ -3,31 +3,31 @@ -PURPLE -BLACK - - - - -BROWN - - -BLUE - - - - - - - -GREEN - - - -YELLOW - - - +PURPLE +BLACK + + + + +BROWN + + +BLUE + + + + + + + +GREEN + + + +YELLOW + + + diff --git a/symbol sets/15000/ISOM 2017-2_15000.omap b/symbol sets/15000/ISOM 2017-2_15000.omap index fc3b436b3d..f6a6804888 100644 --- a/symbol sets/15000/ISOM 2017-2_15000.omap +++ b/symbol sets/15000/ISOM 2017-2_15000.omap @@ -3,41 +3,41 @@ -PURPLE - -BLACK -GREEN - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - - - - - -YELLOW - - +PURPLE + +BLACK +GREEN + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + + + + + +YELLOW + + diff --git a/symbol sets/15000/ISSkiOM 2019_15000.omap b/symbol sets/15000/ISSkiOM 2019_15000.omap index a8b1de2901..e6603370e9 100644 --- a/symbol sets/15000/ISSkiOM 2019_15000.omap +++ b/symbol sets/15000/ISSkiOM 2019_15000.omap @@ -3,42 +3,42 @@ -PURPLE - - -BLACK -GREEN - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - - - - - -YELLOW - - +PURPLE + + +BLACK +GREEN + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + + + + + +YELLOW + + diff --git a/symbol sets/20000/ISMTBOM_20000.omap b/symbol sets/20000/ISMTBOM_20000.omap index b25d99a7b6..a37c55c1b8 100644 --- a/symbol sets/20000/ISMTBOM_20000.omap +++ b/symbol sets/20000/ISMTBOM_20000.omap @@ -3,31 +3,31 @@ -PURPLE -BLACK - - - - -BROWN - - -BLUE - - - - - - - -GREEN - - - -YELLOW - - - +PURPLE +BLACK + + + + +BROWN + + +BLUE + + + + + + + +GREEN + + + +YELLOW + + + diff --git a/symbol sets/4000/Course_Design_4000.omap b/symbol sets/4000/Course_Design_4000.omap index 2c4aba8204..a8373e83c2 100644 --- a/symbol sets/4000/Course_Design_4000.omap +++ b/symbol sets/4000/Course_Design_4000.omap @@ -3,12 +3,12 @@ -PURPLE - - - - -BLACK +PURPLE + + + + +BLACK diff --git a/symbol sets/4000/ISSprOM 2019_4000.omap b/symbol sets/4000/ISSprOM 2019_4000.omap index c9a1ab519f..3e33a8d3ce 100644 --- a/symbol sets/4000/ISSprOM 2019_4000.omap +++ b/symbol sets/4000/ISSprOM 2019_4000.omap @@ -3,38 +3,38 @@ -PURPLE - -BLACK -GREEN - - - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - -YELLOW - +PURPLE + +BLACK +GREEN + + + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + +YELLOW + diff --git a/symbol sets/5000/Course_Design_5000.omap b/symbol sets/5000/Course_Design_5000.omap index 2d09e22632..71f6894315 100644 --- a/symbol sets/5000/Course_Design_5000.omap +++ b/symbol sets/5000/Course_Design_5000.omap @@ -3,12 +3,12 @@ -PURPLE - - - - -BLACK +PURPLE + + + + +BLACK diff --git a/symbol sets/5000/ISMTBOM_5000.omap b/symbol sets/5000/ISMTBOM_5000.omap index c40e4e89ce..a081e5dcc6 100644 --- a/symbol sets/5000/ISMTBOM_5000.omap +++ b/symbol sets/5000/ISMTBOM_5000.omap @@ -3,31 +3,31 @@ -PURPLE -BLACK - - - - -BROWN - - -BLUE - - - - - - - -GREEN - - - -YELLOW - - - +PURPLE +BLACK + + + + +BROWN + + +BLUE + + + + + + + +GREEN + + + +YELLOW + + + diff --git a/symbol sets/5000/ISSkiOM 2019_5000.omap b/symbol sets/5000/ISSkiOM 2019_5000.omap index 8dc9acb425..1abdfe8430 100644 --- a/symbol sets/5000/ISSkiOM 2019_5000.omap +++ b/symbol sets/5000/ISSkiOM 2019_5000.omap @@ -3,42 +3,42 @@ -PURPLE - - -BLACK -GREEN - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - - - - - -YELLOW - - +PURPLE + + +BLACK +GREEN + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + + + + + +YELLOW + + diff --git a/symbol sets/7500/ISMTBOM_7500.omap b/symbol sets/7500/ISMTBOM_7500.omap index ca948c5359..88762f74f9 100644 --- a/symbol sets/7500/ISMTBOM_7500.omap +++ b/symbol sets/7500/ISMTBOM_7500.omap @@ -3,31 +3,31 @@ -PURPLE -BLACK - - - - -BROWN - - -BLUE - - - - - - - -GREEN - - - -YELLOW - - - +PURPLE +BLACK + + + + +BROWN + + +BLUE + + + + + + + +GREEN + + + +YELLOW + + + diff --git a/symbol sets/7500/ISSkiOM 2019_7500.omap b/symbol sets/7500/ISSkiOM 2019_7500.omap index cb3444989c..52e87b4a3d 100644 --- a/symbol sets/7500/ISSkiOM 2019_7500.omap +++ b/symbol sets/7500/ISSkiOM 2019_7500.omap @@ -3,42 +3,42 @@ -PURPLE - - -BLACK -GREEN - -BLUE -BROWN - - - - - - - - - - - - - - - - - - - - - - - - - -YELLOW - - +PURPLE + + +BLACK +GREEN + +BLUE +BROWN + + + + + + + + + + + + + + + + + + + + + + + + + +YELLOW + + diff --git a/symbol sets/src/Course_Design_10000.xmap b/symbol sets/src/Course_Design_10000.xmap index 177ad555ca..0fa8b1717b 100644 --- a/symbol sets/src/Course_Design_10000.xmap +++ b/symbol sets/src/Course_Design_10000.xmap @@ -5,42 +5,42 @@ - + PURPLE - + - + - + - + - + BLACK diff --git a/symbol sets/src/ISMTBOM_15000.xmap b/symbol sets/src/ISMTBOM_15000.xmap index ec0091995c..55b41ac4b7 100644 --- a/symbol sets/src/ISMTBOM_15000.xmap +++ b/symbol sets/src/ISMTBOM_15000.xmap @@ -5,56 +5,56 @@ - + PURPLE - + BLACK - + - + - + - + - + BROWN - + @@ -62,35 +62,35 @@ - + - + BLUE - + - + - + @@ -98,84 +98,84 @@ - + - + - + - + - + GREEN - + - + - + - + YELLOW - + - + - + diff --git a/symbol sets/src/ISOM 2017-2_15000.xmap b/symbol sets/src/ISOM 2017-2_15000.xmap index 99eaabaddc..3972af28a8 100644 --- a/symbol sets/src/ISOM 2017-2_15000.xmap +++ b/symbol sets/src/ISOM 2017-2_15000.xmap @@ -5,133 +5,133 @@ - + PURPLE - + - + BLACK - + GREEN - + - + BLUE - + BROWN - + - + - + - + - + - + - + - + - + - + - + - + @@ -139,35 +139,35 @@ - + - + - + - + - + @@ -175,14 +175,14 @@ - + - + @@ -190,63 +190,63 @@ - + - + - + - + - + - + - + YELLOW - + - + diff --git a/symbol sets/src/ISOM2000_15000.xmap b/symbol sets/src/ISOM2000_15000.xmap index a99f28f2d9..584d3ded14 100644 --- a/symbol sets/src/ISOM2000_15000.xmap +++ b/symbol sets/src/ISOM2000_15000.xmap @@ -5,63 +5,63 @@ - + PURPLE - + BLACK - + - + - + - + BLUE - + - + BROWN - + @@ -69,14 +69,14 @@ - + - + @@ -84,77 +84,77 @@ - + - + - + - + GREEN - + - + - + - + YELLOW - + - + - + diff --git a/symbol sets/src/ISOM2017_15000.xmap b/symbol sets/src/ISOM2017_15000.xmap index 179bc29713..de86017a42 100644 --- a/symbol sets/src/ISOM2017_15000.xmap +++ b/symbol sets/src/ISOM2017_15000.xmap @@ -5,84 +5,84 @@ - + PURPLE - + BLACK - + - + - + - + - + - + - + BLUE - + - + BROWN - + @@ -90,14 +90,14 @@ - + - + @@ -105,42 +105,42 @@ - + GREEN - + - + - + - + - + @@ -148,56 +148,56 @@ - + - + - + - + - + - + - + YELLOW - + diff --git a/symbol sets/src/ISSOM_5000.xmap b/symbol sets/src/ISSOM_5000.xmap index ab543b7d2d..83b9afe82e 100644 --- a/symbol sets/src/ISSOM_5000.xmap +++ b/symbol sets/src/ISSOM_5000.xmap @@ -5,70 +5,70 @@ - + PURPLE - + - + BLACK - + - + - + - + - + - + - + @@ -76,21 +76,21 @@ - + - + - + @@ -98,70 +98,70 @@ - + - + - + - + - + BLUE - + - + - + - + BROWN - + @@ -169,14 +169,14 @@ - + - + @@ -184,14 +184,14 @@ - + - + @@ -199,63 +199,63 @@ - + - + GREEN - + - + - + - + YELLOW - + - + - + diff --git a/symbol sets/src/ISSkiOM 2019_15000.xmap b/symbol sets/src/ISSkiOM 2019_15000.xmap index f89a8f29ea..3705e01473 100644 --- a/symbol sets/src/ISSkiOM 2019_15000.xmap +++ b/symbol sets/src/ISSkiOM 2019_15000.xmap @@ -5,21 +5,21 @@ - + PURPLE - + - + @@ -27,119 +27,119 @@ - + BLACK - + GREEN - + - + BLUE - + BROWN - + - + - + - + - + - + - + - + - + - + - + - + @@ -147,35 +147,35 @@ - + - + - + - + - + @@ -183,14 +183,14 @@ - + - + @@ -198,63 +198,63 @@ - + - + - + - + - + - + - + YELLOW - + - + diff --git a/symbol sets/src/ISSkiOM_15000.xmap b/symbol sets/src/ISSkiOM_15000.xmap index 35a5503c9d..b3ee285d97 100644 --- a/symbol sets/src/ISSkiOM_15000.xmap +++ b/symbol sets/src/ISSkiOM_15000.xmap @@ -5,56 +5,56 @@ - + - + PURPLE - + UPPER GREEN - + BLACK - + - + - + BROWN - + @@ -62,35 +62,35 @@ - + - + BLUE - + - + - + @@ -98,77 +98,77 @@ - + - + - + - + GREEN - + - + - + - + YELLOW - + - + - + diff --git a/symbol sets/src/ISSprOM 2019_4000.xmap b/symbol sets/src/ISSprOM 2019_4000.xmap index fcadc9e3f6..83e684c933 100644 --- a/symbol sets/src/ISSprOM 2019_4000.xmap +++ b/symbol sets/src/ISSprOM 2019_4000.xmap @@ -5,147 +5,147 @@ - + PURPLE - + - + BLACK - + GREEN - + - + - + - + BLUE - + BROWN - + - + - + - + - + - + - + - + - + - + - + - + @@ -153,14 +153,14 @@ - + - + @@ -168,63 +168,63 @@ - + - + - + - + - + - + - + - + YELLOW - + diff --git a/test/data/templates/world-file.xmap b/test/data/templates/world-file.xmap index 745e0a6c89..2c5e80f277 100644 --- a/test/data/templates/world-file.xmap +++ b/test/data/templates/world-file.xmap @@ -13,7 +13,7 @@ - + PURPLE From 8b38150baafe9d3f4a3fff541f8f39c40c890414 Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:51:16 +0200 Subject: [PATCH 4/8] OcdFileImport/Export: Persist OCD color numbers Use the color id to store the OCD color number. We know that the registration black may move in the table, get a new number, or might be dropped. The thing is that Mapper has registration black as a special color that cannot get an OCD color number, and we merely guess whether we need the color in the table in the export path. Closes GH-1245. --- src/core/map.cpp | 13 ++- src/fileformats/ocd_file_export.cpp | 76 ++++++++++------- src/fileformats/ocd_file_export.h | 2 +- src/fileformats/ocd_file_import.cpp | 7 ++ test/data/colors/fractional-cmyk.omap | 26 ++++++ test/data/colors/no-spot-colors.omap | 24 ++++++ test/data/colors/semi-opaque-color.omap | 24 ++++++ test/data/colors/spot-color-overprint.omap | 26 ++++++ test/file_format_t.cpp | 95 ++++++++++++++++++++-- 9 files changed, 254 insertions(+), 39 deletions(-) create mode 100644 test/data/colors/fractional-cmyk.omap create mode 100644 test/data/colors/no-spot-colors.omap create mode 100644 test/data/colors/semi-opaque-color.omap create mode 100644 test/data/colors/spot-color-overprint.omap diff --git a/src/core/map.cpp b/src/core/map.cpp index 7bf1029016..6fb9f4f51f 100644 --- a/src/core/map.cpp +++ b/src/core/map.cpp @@ -139,9 +139,16 @@ Map::MapColorSet::~MapColorSet() void Map::MapColorSet::insert(int pos, MapColor* color) { colors.insert(colors.begin() + pos, color); - if (color->getId() >= 0 && !ids[color->getId()]) + auto const color_id = color->getId(); + if (color_id >= 0) { - ids[color->getId()] = color; + // Current Mapper symbol sets have about 36 colors. Make room for + // at least 48 items and reallocate the vector in 16-item increments + // to keep the reallocation count low. + if (color_id >= static_cast(ids.size())) + ids.resize(std::max((color_id | 0xF) + 1, 48), nullptr); + Q_ASSERT(!ids[color_id]); + ids[color_id] = color; } else { @@ -348,6 +355,7 @@ MapColorMap Map::MapColorSet::importSet(const Map::MapColorSet& other, std::vect // Solve selected conflict item auto new_color = new MapColor(*selected_item->dest_color); + new_color->setId(-1); // Force assignment of a new id selected_item->dest_color = new_color; out_pointermap[selected_item->src_color] = new_color; std::size_t insertion_index = (selected_item->lower_errors == 0) ? selected_item->upper_bound : (selected_item->lower_bound+1); @@ -395,6 +403,7 @@ MapColorMap Map::MapColorSet::importSet(const Map::MapColorSet& other, std::vect MapColor* new_color = it->dest_color; if (new_color) { + new_color->setId(-1); // Invalidate the color id so that the color gets a new one. if (new_color->getSpotColorMethod() == MapColor::CustomColor) { SpotColorComponents components = new_color->getComponents(); diff --git a/src/fileformats/ocd_file_export.cpp b/src/fileformats/ocd_file_export.cpp index d363cdf8b8..61508c2de7 100644 --- a/src/fileformats/ocd_file_export.cpp +++ b/src/fileformats/ocd_file_export.cpp @@ -708,7 +708,25 @@ bool OcdFileExport::exportImplementation() // Check for a necessary offset (and add related warnings early). area_offset = calculateAreaOffset(); - uses_registration_color = map->isColorUsedByASymbol(map->getRegistrationColor()); + + if (map->isColorUsedByASymbol(map->getRegistrationColor())) + { + // A heuristic to find *some* free id for the Registration black color. + // Imported OCD maps usually have number 1 as the All color separations + // but this color gets dropped during import in favor of the built-in + // Mapper Registration black. Chances are that the original id is still + // free. If that id is in use, blindly search beyond the maximum number + // of colors. + if (!map->getMapColorById(1)) + registration_color_id = 1; + else + for (auto id = map->getNumColorPrios(); ; ++id) + if (!map->getMapColorById(id)) + { + registration_color_id = id; + break; + } + } setupFileHeaderGeneric(ocd_version, *file.header()); exportSetup(file); // includes colors @@ -843,25 +861,21 @@ void OcdFileExport::exportSetup(OcdFile& file) throw FileFormatException(::OpenOrienteering::OcdFileExport::tr("The map contains more than 24 spot colors which is not supported by OCD version 8.")); auto begin_of_spot_colors = beginOfSpotColors(map); - if (uses_registration_color) - ++begin_of_spot_colors; // in ocd output (ocd_number below) if (begin_of_spot_colors > 256) throw FileFormatException(::OpenOrienteering::OcdFileExport::tr("The map contains more than 256 colors which is not supported by OCD version 8.")); using std::begin; using std::end; auto separation_info = symbol_header.separation_info; auto color_info = symbol_header.color_info; - quint16 ocd_number = 0; - if (uses_registration_color) + if (registration_color_id >= 0) { - color_info->number = 0; + color_info->number = registration_color_id; color_info->name = toOcdString(Map::getRegistrationColor()->getName()); color_info->cmyk = { 200, 200, 200, 200 }; // OC*D stores CMYK values as integers from 0-200. std::fill(begin(color_info->separations), begin(color_info->separations) + symbol_header.num_separations, 200); std::fill(begin(color_info->separations) + symbol_header.num_separations, end(color_info->separations), 255); ++color_info; - ++ocd_number; } for (int i = 0; i < num_colors; ++i) @@ -884,7 +898,7 @@ void OcdFileExport::exportSetup(OcdFile& file) separation_info->raster_angle = quint16(qRound(10 * color->getScreenAngle())); ++separation_info; - if (ocd_number >= begin_of_spot_colors) + if (i >= begin_of_spot_colors) continue; // Just a spot color, not a regular map color. std::fill(begin(color_info->separations), end(color_info->separations), 255); @@ -908,13 +922,12 @@ void OcdFileExport::exportSetup(OcdFile& file) } } - color_info->number = ocd_number; + color_info->number = color->getId(); color_info->name = toOcdString(color->getName()); color_info->cmyk = ocd_cmyk; ++color_info; - ++ocd_number; } - symbol_header.num_colors = ocd_number; + symbol_header.num_colors = color_info - symbol_header.color_info; } } @@ -1001,9 +1014,9 @@ void OcdFileExport::exportSetup() // Map colors auto num_colors = map->getNumColorPrios(); auto begin_of_spot_colors = beginOfSpotColors(map); - auto ocd_number = 0; auto spot_number = 0; - if (uses_registration_color) + + if (registration_color_id >= 0) { SpotColorComponents all_spot_colors; for (int i = 0; i < num_colors; ++i) @@ -1018,7 +1031,7 @@ void OcdFileExport::exportSetup() MapColor registration_color{*Map::getRegistrationColor()}; registration_color.setSpotColorComposition(all_spot_colors); registration_color.setCmyk(all_cmyk); - addParameterString(9, stringForColor(ocd_number++, registration_color)); + addParameterString(9, stringForColor(registration_color_id, registration_color)); } for (int i = 0; i < num_colors; ++i) @@ -1027,10 +1040,15 @@ void OcdFileExport::exportSetup() if (color->getSpotColorMethod() == MapColor::SpotColor) { addParameterString(10, stringForSpotColor(spot_number++, *color)); - if (ocd_number >= begin_of_spot_colors) + if (i >= begin_of_spot_colors) continue; // Just a spot color, not a regular map color. } - addParameterString(9, stringForColor(ocd_number++, *color)); + + auto ocd_number = color->getId(); + if (ocd_number < 0) + addWarning(tr("Color %1 lacks an id in OCD export. Please report this incident to the developers.") + .arg(color->getName())); + addParameterString(9, stringForColor(ocd_number, *color)); } } @@ -1130,11 +1148,10 @@ void OcdFileExport::setupSymbolColors(const Symbol* symbol, O auto bitpos = std::begin(ocd_base_symbol.colors); auto last = std::end(ocd_base_symbol.colors); - if (uses_registration_color) + if (registration_color_id >= 0) { if (symbol->containsColor(map->getRegistrationColor())) - *bitpos |= bitmask; - bitmask <<= 1; + ocd_base_symbol.colors[registration_color_id / 8] |= 1 << (registration_color_id % 8); } for (int c = 0; c < map->getNumColorPrios(); ++c) { @@ -1169,26 +1186,25 @@ void OcdFileExport::setupSymbolColors(const Symbol* symbol, Counter& num_colors, num_colors = 0; auto it = first; - auto registration_offset = 0; - if (uses_registration_color) + if (registration_color_id >= 0) { - registration_offset = 1; if (symbol->containsColor(map->getRegistrationColor())) { ++num_colors; - *it = IndexType(0); + *it = IndexType(registration_color_id); ++it; } } for (int c = 0; c < map->getNumColorPrios(); ++c) { - if (!symbol->containsColor(map->getColorByPrio(c))) + auto const* color = map->getColorByPrio(c); + if (!symbol->containsColor(color)) continue; ++num_colors; - *it = IndexType(c + registration_offset); + *it = IndexType(color->getId()); if (++it == last) { @@ -2898,12 +2914,10 @@ void OcdFileExport::exportExtras() quint16 OcdFileExport::convertColor(const MapColor* color) const { - auto index = map->findColorPrio(color); - if (index >= 0) - { - return quint16(uses_registration_color ? (index + 1) : index); - } - return 0; + if (color == map->getRegistrationColor()) + return registration_color_id; + else + return color ? color->getId() : 0; } diff --git a/src/fileformats/ocd_file_export.h b/src/fileformats/ocd_file_export.h index f0aa70cb31..2d85b6fb6a 100644 --- a/src/fileformats/ocd_file_export.h +++ b/src/fileformats/ocd_file_export.h @@ -345,7 +345,7 @@ class OcdFileExport : public Exporter quint16 ocd_version; - bool uses_registration_color = false; + int registration_color_id = -1; }; diff --git a/src/fileformats/ocd_file_import.cpp b/src/fileformats/ocd_file_import.cpp index c045a5279f..d068481a75 100644 --- a/src/fileformats/ocd_file_import.cpp +++ b/src/fileformats/ocd_file_import.cpp @@ -471,6 +471,7 @@ void OcdFileImport::importColors(const OcdFile& file) const QString name = convertOcdString(color_info.name); int color_pos = map->getNumColorPrios(); auto color = new MapColor(name, color_pos); + color->setId(color_info.number); // OC*D stores CMYK values as integers from 0-200. MapColorCmyk cmyk; @@ -543,8 +544,13 @@ void OcdFileImport::importColors(const OcdFile< F >& file) return a->getPriority() < b->getPriority(); }); // Insert the spot colors into the map after (below) the regular colors. + auto non_conflicting_id = 0; + for (auto prio = 0; prio < map->getNumColorPrios(); ++prio) + non_conflicting_id = std::max(non_conflicting_id, map->getColorByPrio(prio)->getId()); + non_conflicting_id += 512; // Provide breathing room to the regular colors for (auto spot_color : spot_colors) { + spot_color->setId(non_conflicting_id++); map->addColor(spot_color, map->getNumColorPrios()); } } @@ -718,6 +724,7 @@ void OcdFileImport::importColor(const QString& param_string) int color_pos = map->getNumColorPrios(); auto color = new MapColor(name, color_pos); + color->setId(number); color->setCmyk(cmyk); color->setOpacity(opacity); map->addColor(color, color_pos); diff --git a/test/data/colors/fractional-cmyk.omap b/test/data/colors/fractional-cmyk.omap new file mode 100644 index 0000000000..ccdec36380 --- /dev/null +++ b/test/data/colors/fractional-cmyk.omap @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/data/colors/no-spot-colors.omap b/test/data/colors/no-spot-colors.omap new file mode 100644 index 0000000000..d69e66431f --- /dev/null +++ b/test/data/colors/no-spot-colors.omap @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/data/colors/semi-opaque-color.omap b/test/data/colors/semi-opaque-color.omap new file mode 100644 index 0000000000..12d484c3ed --- /dev/null +++ b/test/data/colors/semi-opaque-color.omap @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/data/colors/spot-color-overprint.omap b/test/data/colors/spot-color-overprint.omap new file mode 100644 index 0000000000..a2a994ce0e --- /dev/null +++ b/test/data/colors/spot-color-overprint.omap @@ -0,0 +1,26 @@ + + + + + + +Spot color 1 +Unused spot color w/o K.O. + + + + + + + + + + + + + + + + + + diff --git a/test/file_format_t.cpp b/test/file_format_t.cpp index b3b01e014a..b10556b353 100644 --- a/test/file_format_t.cpp +++ b/test/file_format_t.cpp @@ -24,6 +24,7 @@ #include #include #include +#include // IWYU pragma: no_include #include @@ -1307,9 +1308,33 @@ void FileFormatTest::colorTest_data() QTest::addColumn("filepath"); QTest::addColumn("format_id"); - QTest::newRow(QByteArray {"Load color table - XML"}) - << QStringLiteral("data:colors/color-id.omap") - << QByteArray {"XML"}; + std::vector format_ids_list; + format_ids_list.reserve(5); + format_ids_list.push_back("XML"); + +#ifndef MAPPER_BIG_ENDIAN + // In attempt to be future-proof, we shamelessly rip out all OCD-like + // formats from the registry. + FileFormats.findFormat([&format_ids_list](auto format) { + if (qstrlen(format->id()) > 3 && !qstrncmp(format->id(), "OCD", 3)) + format_ids_list.push_back(format->id()); + return false; + }); +#endif + + for (auto& format_id : format_ids_list) + for (auto* test_file : { "data:colors/color-id.omap", + "data:colors/fractional-cmyk.omap", + "data:colors/semi-opaque-color.omap", + "data:colors/spot-color-overprint.omap", + "data:colors/no-spot-colors.omap", + }) + { + QTest::newRow(QByteArray {"Load color table - "}.append(format_id) + .append(" - ").append(test_file)) + << QString::fromUtf8(test_file) + << QByteArray {format_id}; + } } @@ -1386,7 +1411,62 @@ void FileFormatTest::colorTest() // Failure here means that we failed to persist the color properties in // the target file format. for (auto i = 0; i < original->getNumColorPrios(); ++i) - QCOMPARE(*original->getColorByPrio(i), *copy->getColorByPrio(i)); + { + // >>>>>>>>> Temporary handling of OCD defects + auto const orig_cmyk = original->getColorByPrio(i)->getCmyk(); + auto const components_sum = orig_cmyk.c + orig_cmyk.m + orig_cmyk.y + orig_cmyk.k; + if (*format_id == 'O' && format_id[3] != '8' && components_sum * 100 != std::floor(components_sum * 100)) + QEXPECT_FAIL("", "Fractional CMYK values are not yet handled in OCD format", Continue); + if (*format_id == 'O' && original->getColorByPrio(i)->getKnockout() != copy->getColorByPrio(i)->getKnockout()) + QEXPECT_FAIL("", "Overprint is not yet reliably maintained in OCD format", Continue); + if (*format_id == 'O' && i == 0 && original->getColorByPrio(0)->getName() != copy->getColorByPrio(0)->getName()) + QEXPECT_FAIL("", "Registration black is not properly detected in absence of spot colors", Continue); + // <<<<<<<<< Temporary handling of OCD defects + + // Gratiously handle missing v8 features (opacity, limited name + // length, CMYK color resolution). + auto* color_in_original = original->getColorByPrio(i); + auto color_in_copy = std::unique_ptr(copy->getColorByPrio(i)->duplicate()); + if(!qstrcmp(format_id, "OCD8")) + { + // No opacity in v8 format. + if (color_in_original->getOpacity() != color_in_copy->getOpacity()) + color_in_copy->setOpacity(color_in_original->getOpacity()); + // Shorter color names. + if (color_in_original->getName().startsWith(color_in_copy->getName())) + color_in_copy->setName(color_in_original->getName()); + // Shorter spot color names. + if (color_in_original->getSpotColorMethod() == MapColor::SpotColor + && color_in_original->getSpotColorName().startsWith(color_in_copy->getSpotColorName())) + color_in_copy->setSpotColorName(color_in_original->getSpotColorName()); + // Lower resolution of CMYK component values. + auto const orig_cmyk = original->getColorByPrio(i)->getCmyk(); + auto copy_cmyk = MapColorCmyk { color_in_copy->getCmyk() }; + bool do_set_color = false; + for (auto f : { std::make_tuple(&orig_cmyk.c, ©_cmyk.c), + std::make_tuple(&orig_cmyk.m, ©_cmyk.m), + std::make_tuple(&orig_cmyk.y, ©_cmyk.y), + std::make_tuple(&orig_cmyk.k, ©_cmyk.k) }) + { + if (qFuzzyCompare(*std::get<0>(f), *std::get<1>(f))) + continue; + if (qFuzzyCompare(std::floor(*std::get<0>(f) * 200), std::floor(*std::get<1>(f) * 200))) + { + *std::get<1>(f) = *std::get<0>(f); + do_set_color = true; + } + else + { + do_set_color = false; + break; + } + } + if (do_set_color) + color_in_copy->setCmyk(copy_cmyk); + } + + QCOMPARE(*color_in_copy, *color_in_original); + } // Failure here means that we failed to persist the link between // symbols and colors. @@ -1398,10 +1478,15 @@ void FileFormatTest::colorTest() auto const* color_in_original = original->getColorByPrio(color_prio); if (symbol_in_original->containsColor(color_in_original)) { + // v8 does not have combined symbols. + if (symbol_in_original->getType() == Symbol::Combined + && !qstrcmp(format_id, "OCD8")) + continue; + auto const* symbol_in_copy = copy->getSymbol(i); auto const* color_in_copy = copy->getColorByPrio(color_prio); QVERIFY(symbol_in_copy->containsColor(color_in_copy)); - QCOMPARE(*color_in_original, *color_in_copy); + QCOMPARE(color_in_copy->getId(), color_in_original->getId()); } } } From 4b2b4f4a31a31d6c48c8ff8404ba819563ec6ff6 Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:52:52 +0200 Subject: [PATCH 5/8] OcdFileExport: Export non-integer CMYK components CMYK color component values can include fractional parts. The v8 format expresses the component values as a number in the range 0-200 that maps to percentages therefore component values are recorded with 0.5 % precision. The higher format versions use strings for the color definition, and the numbers can include one decimal place. This patch makes Mapper export the fractional part as needed. --- src/fileformats/ocd_file_export.cpp | 8 ++++---- test/file_format_t.cpp | 4 ---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/fileformats/ocd_file_export.cpp b/src/fileformats/ocd_file_export.cpp index 61508c2de7..a63875940f 100644 --- a/src/fileformats/ocd_file_export.cpp +++ b/src/fileformats/ocd_file_export.cpp @@ -491,10 +491,10 @@ QString stringForColor(int i, const MapColor& color) QTextStream out(&string_9, QIODevice::Append); out << color.getName() << "\tn" << i - << "\tc" << qRound(cmyk.c * 100) - << "\tm" << qRound(cmyk.m * 100) - << "\ty" << qRound(cmyk.y * 100) - << "\tk" << qRound(cmyk.k * 100) + << "\tc" << qRound(cmyk.c * 1000)/10.0 + << "\tm" << qRound(cmyk.m * 1000)/10.0 + << "\ty" << qRound(cmyk.y * 1000)/10.0 + << "\tk" << qRound(cmyk.k * 1000)/10.0 << "\to" << (color.getKnockout() ? '0' : '1') << "\tt" << qRound(color.getOpacity() * 100); if (color.getSpotColorMethod() == MapColor::CustomColor) diff --git a/test/file_format_t.cpp b/test/file_format_t.cpp index b10556b353..e05ce54911 100644 --- a/test/file_format_t.cpp +++ b/test/file_format_t.cpp @@ -1413,10 +1413,6 @@ void FileFormatTest::colorTest() for (auto i = 0; i < original->getNumColorPrios(); ++i) { // >>>>>>>>> Temporary handling of OCD defects - auto const orig_cmyk = original->getColorByPrio(i)->getCmyk(); - auto const components_sum = orig_cmyk.c + orig_cmyk.m + orig_cmyk.y + orig_cmyk.k; - if (*format_id == 'O' && format_id[3] != '8' && components_sum * 100 != std::floor(components_sum * 100)) - QEXPECT_FAIL("", "Fractional CMYK values are not yet handled in OCD format", Continue); if (*format_id == 'O' && original->getColorByPrio(i)->getKnockout() != copy->getColorByPrio(i)->getKnockout()) QEXPECT_FAIL("", "Overprint is not yet reliably maintained in OCD format", Continue); if (*format_id == 'O' && i == 0 && original->getColorByPrio(0)->getName() != copy->getColorByPrio(0)->getName()) From 7036522caf8b29a5c9a942aacfa2bf3e2fe82fa2 Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:54:29 +0200 Subject: [PATCH 6/8] OcdFileImport/Export: Handle v8 color overprint Although the official docs do not mention the color overprint property, the program dialog contains this setting. A quick experiment shows that the checkbox controls a reserved struct member. This patch implements color overprint import and export in OCD v8 format. --- src/fileformats/ocd_file_export.cpp | 1 + src/fileformats/ocd_file_import.cpp | 1 + src/fileformats/ocd_types_v8.h | 2 +- test/file_format_t.cpp | 2 -- 4 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fileformats/ocd_file_export.cpp b/src/fileformats/ocd_file_export.cpp index a63875940f..950cc038b6 100644 --- a/src/fileformats/ocd_file_export.cpp +++ b/src/fileformats/ocd_file_export.cpp @@ -924,6 +924,7 @@ void OcdFileExport::exportSetup(OcdFile& file) color_info->number = color->getId(); color_info->name = toOcdString(color->getName()); + color_info->overprint = color->getKnockout() ? -1 : 0; color_info->cmyk = ocd_cmyk; ++color_info; } diff --git a/src/fileformats/ocd_file_import.cpp b/src/fileformats/ocd_file_import.cpp index d068481a75..83418dbb91 100644 --- a/src/fileformats/ocd_file_import.cpp +++ b/src/fileformats/ocd_file_import.cpp @@ -501,6 +501,7 @@ void OcdFileImport::importColors(const OcdFile& file) // The color's CMYK was customized. color->setCmyk(cmyk); } + color->setKnockout(color_info.overprint != 0); if ((i == 0 && color->isBlack() && color->getName() == QLatin1String("Registration black")) || (!components.empty() && components.size() == num_separations diff --git a/src/fileformats/ocd_types_v8.h b/src/fileformats/ocd_types_v8.h index 46c8405394..fdcc4000c7 100644 --- a/src/fileformats/ocd_types_v8.h +++ b/src/fileformats/ocd_types_v8.h @@ -64,7 +64,7 @@ namespace Ocd struct ColorInfoV8 { quint16 number; - quint16 RESERVED_MEMBER; + qint16 overprint; // undocumented member, overprint: 0 = no, -1 = yes CmykV8 cmyk; PascalString<31> name; quint8 separations[32]; diff --git a/test/file_format_t.cpp b/test/file_format_t.cpp index e05ce54911..2563ab6fea 100644 --- a/test/file_format_t.cpp +++ b/test/file_format_t.cpp @@ -1413,8 +1413,6 @@ void FileFormatTest::colorTest() for (auto i = 0; i < original->getNumColorPrios(); ++i) { // >>>>>>>>> Temporary handling of OCD defects - if (*format_id == 'O' && original->getColorByPrio(i)->getKnockout() != copy->getColorByPrio(i)->getKnockout()) - QEXPECT_FAIL("", "Overprint is not yet reliably maintained in OCD format", Continue); if (*format_id == 'O' && i == 0 && original->getColorByPrio(0)->getName() != copy->getColorByPrio(0)->getName()) QEXPECT_FAIL("", "Registration black is not properly detected in absence of spot colors", Continue); // <<<<<<<<< Temporary handling of OCD defects From c8a7ea37b2320c85c17750cd691b6e2190a7592c Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:56:02 +0200 Subject: [PATCH 7/8] OcdFileImport: Reliably detect Registration black Mapper is unable to detect Registration black in absence of spot colors. This patch fixes the defect, loosens the name matching criterion and aligns the matching criteria along the supported OCD format versions. --- src/fileformats/ocd_file_import.cpp | 12 ++++++------ test/file_format_t.cpp | 5 ----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/fileformats/ocd_file_import.cpp b/src/fileformats/ocd_file_import.cpp index 83418dbb91..0c9729981e 100644 --- a/src/fileformats/ocd_file_import.cpp +++ b/src/fileformats/ocd_file_import.cpp @@ -503,12 +503,12 @@ void OcdFileImport::importColors(const OcdFile& file) } color->setKnockout(color_info.overprint != 0); - if ((i == 0 && color->isBlack() && color->getName() == QLatin1String("Registration black")) - || (!components.empty() && components.size() == num_separations - && color_info.cmyk.cyan == 200 + if ((color->isBlack() && color->getName().startsWith(QLatin1String("Registration black"))) + || (color_info.cmyk.cyan == 200 && color_info.cmyk.magenta == 200 && color_info.cmyk.yellow == 200 && color_info.cmyk.black == 200 + && components.size() == num_separations && std::all_of(color_info.separations, color_info.separations + num_separations, [](const auto& s) { return s == 200; }) ) ) { @@ -710,12 +710,12 @@ void OcdFileImport::importColor(const QString& param_string) if (!number_ok) return; - if ((cmyk.isBlack() && name == QLatin1String("Registration black")) - || (!components.empty() && components.size() == spot_colors.size() - && qFuzzyCompare(cmyk.c, 1) + if ((cmyk.isBlack() && name.startsWith(QLatin1String("Registration black"))) + || (qFuzzyCompare(cmyk.c, 1) && qFuzzyCompare(cmyk.m, 1) && qFuzzyCompare(cmyk.y, 1) && qFuzzyCompare(cmyk.k, 1) + && components.size() == spot_colors.size() && std::all_of(begin(components), end(components), [](const auto& c) { return qFuzzyCompare(c.factor, 1); }) ) ) { diff --git a/test/file_format_t.cpp b/test/file_format_t.cpp index 2563ab6fea..e178fb44dd 100644 --- a/test/file_format_t.cpp +++ b/test/file_format_t.cpp @@ -1412,11 +1412,6 @@ void FileFormatTest::colorTest() // the target file format. for (auto i = 0; i < original->getNumColorPrios(); ++i) { - // >>>>>>>>> Temporary handling of OCD defects - if (*format_id == 'O' && i == 0 && original->getColorByPrio(0)->getName() != copy->getColorByPrio(0)->getName()) - QEXPECT_FAIL("", "Registration black is not properly detected in absence of spot colors", Continue); - // <<<<<<<<< Temporary handling of OCD defects - // Gratiously handle missing v8 features (opacity, limited name // length, CMYK color resolution). auto* color_in_original = original->getColorByPrio(i); From 30a443e0a0e00f9f7af3f4979833e40998165f29 Mon Sep 17 00:00:00 2001 From: Libor Pechacek Date: Fri, 28 Jun 2024 23:57:35 +0200 Subject: [PATCH 8/8] OcdFileExport: Warn when exporting transparent colors to v8 format Transparent (semi-opaque) colors are supported only in later format versions. This patch makes Mapper write out a warning when the color opacity value gets dropped. --- src/fileformats/ocd_file_export.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/fileformats/ocd_file_export.cpp b/src/fileformats/ocd_file_export.cpp index 950cc038b6..6a96406eab 100644 --- a/src/fileformats/ocd_file_export.cpp +++ b/src/fileformats/ocd_file_export.cpp @@ -881,6 +881,9 @@ void OcdFileExport::exportSetup(OcdFile& file) for (int i = 0; i < num_colors; ++i) { const auto* color = map->getColorByPrio(i); + if (color->getOpacity() < 1) + addWarning(::OpenOrienteering::OcdFileExport::tr("Variable color opacity in \"%1\" is not supported by OCD version 8. Exporting as a fully opaque color.") + .arg(color->getName())); const auto& cmyk = color->getCmyk(); // OC*D stores CMYK values as integers from 0-200. auto ocd_cmyk = Ocd::CmykV8 {