Skip to content

Consider dash symbols in combined symbols - #2154

Merged
dg0yt merged 4 commits into
OpenOrienteering:masterfrom
dl3sdo:dash-points-combined-symbols
Dec 9, 2024
Merged

Consider dash symbols in combined symbols#2154
dg0yt merged 4 commits into
OpenOrienteering:masterfrom
dl3sdo:dash-points-combined-symbols

Conversation

@dl3sdo

Copy link
Copy Markdown
Member

When drawing line objects for symbols with dash symbols, no dash points were set automatically if the symbol was a combined symbol.
When editing line objects for symbols with dash symbols, additional points were not added as dash points if the symbol was a combined symbol.
These deviations were known and marked as TODO.
This commit treats line symbols with dash symbols and combined symbols containing line symbols with dash symbols equally.

@dl3sdodl3sdo mentioned this pull request Apr 27, 2023
@dl3sdo
dl3sdoforce-pushed the dash-points-combined-symbols branch from 676d238 to 044165aCompareJune 26, 2023 21:18
@dl3sdodl3sdo mentioned this pull request Jun 27, 2023
@dg0yt
dg0yt self-requested a review April 14, 2024 15:47
@dl3sdo
dl3sdoforce-pushed the dash-points-combined-symbols branch from 044165a to 165bf3dCompareAugust 22, 2024 12:43
Comment threadsrc/tools/tool_helpers.cpp Outdated
* Function is used by DrawPathTool::updateDashPointDrawing() and
* EditPointTool::addDashPointDefault() in relation to setting and changing dash points.
*/
bool symbolContainsDashSymbol(const Symbol* symbol)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should become a virtual member function Symbol::containsDashSymbol, with suitable overrides in subclasses.

@dl3sdo
dl3sdo marked this pull request as draft August 23, 2024 08:09
@dl3sdo
dl3sdoforce-pushed the dash-points-combined-symbols branch from 165bf3d to acaa4a5CompareAugust 23, 2024 15:43
@dl3sdo
dl3sdo marked this pull request as ready for review August 23, 2024 15:45
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

For line symbols and combined symbols determine if the symbol contains
a dash symbol.
When drawing line objects for symbols with dash symbols, no dash
points were set automatically if the symbol was a combined symbol.
When editing line objects for symbols with dash symbols, additional
points were not added as dash points if the symbol was a combined
symbol.
These deviations were known and marked as TODO.
This change treats line symbols with dash symbols and combined symbols
containing line symbols with dash symbols equally.
@dl3sdo
dl3sdoforce-pushed the dash-points-combined-symbols branch from acaa4a5 to f5011a4CompareNovember 29, 2024 15:50
@dl3sdo

Copy link
Copy Markdown
MemberAuthor

Rebased to solve merge conflicts.
@dg0yt: please review.

@dg0ytdg0yt self-assigned this Dec 8, 2024
There is no benefit from the inline definition,
and all other implementations are in the cpp files.
@dl3sdo

Copy link
Copy Markdown
MemberAuthor

@dg0yt: I agree to your proposed changes, thank you.

@dg0yt
dg0yt merged commit 340e506 into OpenOrienteering:masterDec 9, 2024
@dl3sdo
dl3sdo deleted the dash-points-combined-symbols branch December 10, 2024 18:07
@dl3sdodl3sdo added this to the v0.9.6 milestone May 26, 2025
lpechacek pushed a commit to lpechacek/mapper that referenced this pull request Oct 15, 2025
Dash symbols may occur in line symbols but also in combined symbols.
The editors only considered line symbols.
This change adds a virtual property to the Symbol class and
corresponding overrides in derived classes. DrawPathTool and
EditPointTool are updated to make use of the new property.
Co-authored-by: Kai Pastor <dg0yt@darc.de>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dl3sdo@dg0yt