-
-
Notifications
You must be signed in to change notification settings - Fork 17
Signals: support multiple icons per feature #657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
SDF icon merging is not working well yet 👀 OSM Americana Shieldlib also does not implement this (https://github.com/osm-americana/openstreetmap-americana/blob/d9dbb3a6e635976885e7ec3ea730267de7a449bf/shieldlib/src/screen_gfx.ts#L33) |
CONTRIBUTING.md
Outdated
| - { all: ['IT:AVA', 'IT:AVV'], value: 'it/AVV-AVA', description: 'Avvio & Avanzamento' } | ||
| - { exact: 'IT:AVA', value: 'it/AVA', description: 'Avanzamento' } | ||
| - { exact: 'IT:AVV', value: 'it/AVV', description: 'Avvio' } | ||
| offset: { x: -1, y: 9 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is very cool! i have a question about the offset – currently it uses absolute pixels (not relative values). this means it won't work for the case from #595
- could we change
offsetto use relative pixels or percentages? for example0to1, 1 means 100% of the previous image's height? - or could we support both absolute and relative? for example
y: truemeans 100% of the previous image's height?
I briefly explored this in 3fc516c - it's an extremely crude solution to make the tram signals case work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review, good point.
I agree that stacking becomes harder specifying pixel offsets.
We could also add something like
position:
under: ...and then somehow specify under what this icon needs to be positioned.
Or maybe
position: 'center-bottom'or
position: 'bottom'(and always position the icon in the center)
And then the offset can still be useful to offset the icon itself a little bit, either up to create a bit of overlap, or down to create a small gap between the preceding icons.
🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, something like that would be convenient – I don't have any specific suggestions for the syntax
or if you prefer to keep it simple: I think the italian example would still work if offset.x and offset.y used percentages instead of pixels? then the syntax is the same, only the units are different
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the Italian example the percentage offset would be cumbersome. The icon itself is 10px, and the icon below 12px, so the offset would be 8.3333333% (1 pixel) to get it centered.
But actually the goal is just to center the icon, the offset itself is not as important as I thought.
I am considering dropping offset entirely for now, to keep it simple, and only have a position specified where the next icon will be placed.
So making 2 icons overlap: position: center (the default)
Putting the next icon under the current icons (Italian example): position: bottom
Putting the next icon on top of the existing icon (tram signals example): position: top
An icon left of the rest of the signal (https://wiki.openstreetmap.org/wiki/DE:OpenRailwayMap/Tagging_in_Germany#Mastschilder, https://www.dampflok7.de/bilder/signale/12_oz/mastschilder/mas/mastschild_mas_1.JPG): position: left
The icon will be centered to the other axis (so specifying top/bottom will center it horizontally, left/right vertically, center centers it fully.
One question remains at the center/top/bottom/left of "what" the next icon is placed. The whole stack, or the last placed icon. I think the whole stack makes most sense.
I will try some things.
Also none of the choices are set in stone, if it does not work out we can just change it to accommodate for more use cases :)
|
The layout with |
…to multiple-icons
|
Legend works, feature popup works, SDF icons work. This should be ready for merging. |
This reverts commit 05d4b9c.
Followup on #657 Features with multiple icons did not output their icon height correctly for non-center-positioned icons. Now the feature icon height is calculated as ``` max(max(centered) + sum(top_bottom), left_right) ``` with centered the height of centered icons, `top_bottom` the height of the top/bottom positioned icons, and `left_right` the left/right positioned icons. Before After
Followup on #657 Features with multiple icons did not output their icon height correctly for non-center-positioned icons. Now the feature icon height is calculated as ``` max(max(centered) + sum(top_bottom), left_right) ``` with centered the height of centered icons, `top_bottom` the height of the top/bottom positioned icons, and `left_right` the left/right positioned icons. Before After
Followup on #657 Features with multiple icons did not output their icon height correctly for non-center-positioned icons. Now the feature icon height is calculated as ``` max(max(centered) + sum(top_bottom), left_right) ``` with centered the height of centered icons, `top_bottom` the height of the top/bottom positioned icons, and `left_right` the left/right positioned icons. Tested on http://localhost:8000/#view=18.5/45.5409841/11.5417528&style=signals: Before: <img width="1275" height="574" alt="image" src="https://github.com/user-attachments/assets/d99aa4fb-2f50-4ddd-86ae-00037074cc28" /> After: <img width="1196" height="586" alt="image" src="https://github.com/user-attachments/assets/5e6f79b9-99bc-4ced-8e20-7cda5f8e06ad" />
Application of #657 for simplifying the IT icons. Related to #639 cc @MatCr90 this PR simplifies the icon setup for the IT main/combined/distant signals. (http://localhost:8000/#view=16.99/45.666447/11.936039&style=signals): <img width="1624" height="699" alt="image" src="https://github.com/user-attachments/assets/48a3e332-68df-4f73-a658-e08e47824f5a" /> (http://localhost:8000/#view=16.62/45.541332/11.539128&style=signals): <img width="1752" height="839" alt="image" src="https://github.com/user-attachments/assets/0cabc0ea-6044-4dd2-b39b-410da1583a48" /> (http://localhost:8000/#view=18.96/45.5404914/11.5740117&style=signals): <img width="757" height="502" alt="image" src="https://github.com/user-attachments/assets/e0e43b6e-71ab-4a4f-b4fa-e1916c223434" /> TODO: - [x] example icons for preset/taginfo
using #657, we can now properly render multiple stop positions on the same node. and we can implement light-rail signals now, which was discussed in #595 (comment) (the german system seems to very similar) stop positions: <img width="99" height="98" alt="image" src="https://github.com/user-attachments/assets/692ff89f-3ba5-4d28-b979-3b622c1971f8" /> <img width="110" height="119" alt="image" src="https://github.com/user-attachments/assets/b899171c-0505-4017-a385-c3f5b9d55b93" /> light-rail signals: <img width="125" height="127" alt="image" src="https://github.com/user-attachments/assets/90cd353a-d975-46b4-930b-b9ef7fd3f85f" /> <img width="148" height="286" alt="image" src="https://github.com/user-attachments/assets/b27d9aeb-5951-432f-8d26-cf5ebd0d8671" /> --------- Co-authored-by: Hidde Wieringa <[email protected]>


Part of #545
Requires decoupling the icon from the signal "feature".
Goal: instead of duplicating the same signal features because they have similar tags but need different icons:
instead, allow multiple icons, composed with each other:
(this example is only combining 2 signals, the Italian signals will have more variants composed together)
For a full example, see #639 which has huge duplication because of the large variety in tagging. (A good thing, but currently the YAML configuration does not support such use cases well.)
This requires changes in many different places: