Conversation
fa730df to
d2790d9
Compare
d2790d9 to
b5ef334
Compare
b5ef334 to
13a86c8
Compare
src/core/symbols/symbol.h
Outdated
| bool is_hidden; /// \see isHidden() | ||
| bool is_protected; /// \see isProtected() | ||
| bool is_rotatable = false; | ||
| QVariantHash auxiliary_properties; /// Auxiliary properties for import of objects (e.g., .ocd, .dxf files). Auxiliary properties are not saved in a map file. |
There was a problem hiding this comment.
QVariantHash means QHash<QString,QVariant>. Given that the use of this member is tightly scoped to the import of objects, i.e. to particular importers, I would prefer an int key type with named constants (from the actual importer) instant of arbitrary strings. This helps to detect typos at compile time and is cheaper than QString. And with QHash instead of any array-like structure, it should still be robust and convenient enough.
| QVariantHash auxiliary_properties; /// Auxiliary properties for import of objects (e.g., .ocd, .dxf files). Auxiliary properties are not saved in a map file. | |
| QHash<int,QVariant> auxiliary_properties; /// For temporary use during import. Not to be saved on export. |
src/fileformats/ocd_file_import.cpp
Outdated
| t->setText(getObjectText(ocd_object)); | ||
| t->setRotation(convertAngle(ocd_object.angle)); | ||
| t->setHorizontalAlignment(text_halign_map.value(symbol)); | ||
| t->setHorizontalAlignment((symbol->getAuxiliaryProperty(QLatin1String("HAlign")).value<TextObject::HorizontalAlignment>())); |
There was a problem hiding this comment.
With the suggested int keys, this becomes something like
| t->setHorizontalAlignment((symbol->getAuxiliaryProperty(QLatin1String("HAlign")).value<TextObject::HorizontalAlignment>())); | |
| t->setHorizontalAlignment(symbol->getAuxiliaryProperty(OcdFileImportProperties::HAlign).value<TextObject::HorizontalAlignment>()); |
Still longer than the original code... the generalization isn't beneficial here.
There was a problem hiding this comment.
Done, I put the enums in the source files. I'm not sure whether the anonymous namespaces around are superfluous.
| /** | ||
| * Returns and then removes an auxiliary property of this symbol. | ||
| */ | ||
| QVariant consumeAuxiliaryProperty(QString key); | ||
|
|
||
| /** | ||
| * Returns and then removes an auxiliary property of this symbol or returns a default value. | ||
| */ | ||
| QVariant consumeAuxiliaryProperty(QString key, QVariant default_value); | ||
|
|
There was a problem hiding this comment.
I tend to suggest a single clearAuxiliaryProperties() instead of the consume... pattern. This function could be called for all symbols at the end of an import if necessary and appropriate.
src/gdal/ogr_file_format.cpp
Outdated
| FILEFORMAT_ASSERT(split < length); | ||
|
|
||
| auto label = description.right(length - split - 1); | ||
| auto label = symbol->consumeAuxiliaryProperty(QLatin1String("label")).toString(); |
There was a problem hiding this comment.
I didn't test now, but I guess that this shows the potential and the missing pieces:
Putting the label into the description was a hack, but it helped to assign objects with the same label to a a single symbol. Probably also on a second and third import into the same map.
This could be covered with an automated test: a test vector file with to identical objects, imported twice. At the end there must be a map with four objects with one symbol.
There was a problem hiding this comment.
Due to my understanding of OgrFileImport::getSymbolForLabel() objects with the same color and font size are assigned to a single symbol, the label as well as anchor and angle are always overwritten.
My proposal would not change the current behaviour, IMO.
|
Test file: |
|
Adding an automated test as proposed by @dg0yt shows an interesting behaviour (maybe a bug in my version of OGR):
I assume that this is the root cause (if I understand "%.3g" correctly): |
|
Note: Code 71 due to AutoCAD 2012 DXF Reference Note: Revise |
e89b490 to
c5bdadc
Compare
c5bdadc to
39ebb69
Compare
|
Rebased to resolve merge conflicts. |
39ebb69 to
487d168
Compare
0a8367d to
37b09a5
Compare
|
I propose to merge #2421 first. |
When importing files, Mapper may encounter symbol properties that are not part of its native symbol properties but need to be applied when importing objects. For example, horizontal and vertical text alignment are symbol properties in OCAD but object properties in Mapper. Extend the Symbol base class to support auxiliary symbol properties that are neither saved to a map file nor loaded from it. Use auxiliary symbol properties for the GDAL importer and for importing .ocd files.
37b09a5 to
0562af4
Compare
Mapper sometimes needs to handle symbol properties that are not part of Mapper's own set of symbol properties and that need to be applied when importing objects.
Example: horizontal and vertical text alignment is a symbol property in OCAD but an object property in Mapper.
Mapper uses auxiliary data structures to temporarily store those symbol properties for later applying them on object import.
This change adds a QVariantHash element to the Symbol base class that allows a flexible and type safe storage of auxiliary symbol properties.
These auxiliary symbol properties are neither saved to a map file nor loaded from it.
The second commit applies the auxiliary symbol properties to the DXF import where text object properties like the text itself, alignment and rotation were previously imported by putting them into a string and transporting the string via the description property of the related text symbol.
Besides being an awkward design the approach suffers from multiple deficiencies: the auxiliary string was not removed from the description field, the rotation angle was rounded (e.g., -17.2 became 20) and the way of constructing and parsing the string was unnecessary complex.
Note that this commit replaces #2135.
The third commit applies the auxiliary symbol properties to the import of OCAD maps.
Auxiliary symbol properties are replacing the text_halign_map and text_valign_map hashes that were used to store OCAD's symbol properties for horizontal and vertical text alignment for being applied later to text object import.
This PR is a POC to demonstrate a possible way of preserving symbol/object related properties until object creation.
Auxiliary symbol properties could be considered with respect to #2082 and #1639.