feat: add `sdf` sprites via additional apis #1492

CommanderStorm · 2024-08-30T03:05:40Z

Resolves #1075

I am not quite right if I have made everything correctly.
The project has lots of places to configure things and it was not always clear where I need an Option.

Another thing which is likely wrong:
I'd assume as far as I read the blessing that should require new files...
Something is really odd here => need to debug that before public review makes sense
I fucked up the testcase a bit. This is now fixed

docs/src/sources-sprites.md

…g for this feature

nyurik · 2024-10-20T05:20:51Z

Thanks, this is an awesome feature to have. One thing I kept thinking while reviewing: would it be possible to NOT have any of the associated config values, and instead simply allow both sdf and non-sdf usage? It's not very difficult to run through the spreet crate twice and generate two spritesheets, and this would simplify usage -- either use /sprite or /sdf (or some other prefix)?

TBD:

does it makes any business sense to do this?
are there are any cases where sdf would fail to generate, while regular sprites would work?
do we ever want to support non-svg files for sprites, and how would that work?

CommanderStorm · 2024-10-20T11:01:41Z

Honestly:
I have not see a difference between the pngs and sdf images, except for the sdf: true metadata.

I need to look into if this is possible, but I think we could just activate this automatically if users are using monochrome icons.

Using a different url is fine too.
You likely don't want to mix the monochrome, but color-configurable sdf images and the regularly colored pngs, but that solution achieves the goal.
What do you think about /sprites/sdf/.. instead? They are still sprites, so having that under this url is likely better.

Will have to test if this would be a breaking change though ^^
=> Will rework this today

Answering your questions:

does it makes any business sense to do this?

Yes, thinking about it, I can see a reason why one would want both (multi colored and dynamically colored icons on the same map)

are there are any cases where sdf would fail to generate, while regular sprites would work?

I have not come across cases where this, fails to generate, but sdfs don't support multiple colors
=> the output here is black, unless colored in the map
do we ever want to support non-svg files for sprites, and how would that work?

generating sdf from pngs is not possible (we are missing the distances) => users would need to supply sdf+png images to have both functionality.
If we have such images, we should likely only support adding pngs to the pngs, not sdfs.
Reasoning: for an sdf to exist, you would need to have a svg file (or other vector format) at some point (unless I am missing something)
=> just add the svg instead.

In any case, I need to make this more clear in the docs, why one would choose the one or the other image format.

hiddewie · 2024-10-20T12:41:51Z

Hi 👋🏻, chipping into this nice PR because it would be very helpful for improving the symbol rendering one of my projects using the MapLibre stack (https://github.com/hiddewie/OpenRailwayMap-vector).

I have not see a difference between the pngs and sdf images, except for the sdf: true metadata.

Testing the output from the spreet commandline shows a difference for me when rendering the same directory

spreet symbols/general sprites

gives me

while

spreet symbols/general sprites --sdf

gives me

(colorless and with the distance field visible).

So the Martin SDF sprites should output something similar when using the spreet library.

One thing I kept thinking while reviewing: would it be possible to NOT have any of the associated config values, and instead simply allow both sdf and non-sdf usage?

It would be useful for me to have both normal and SDF sprites output and available for map rendering. The first thing I can think of is rendering the same symbol twice, one SDF with the icon outline, and the colorful icon on top to automatically generate a (fixed color) halo around a symbol.

And the other reason mentioned above, to render both colored and monochrome variants of the same image on the map is also useful.

The combination of a colorful image together with the SDF data would be ideal, but I don't think that is supported in a single sprite image by any of the libraries or SDF sprite consumers.

Maybe there could be a way to change the consuming map rendering libraries to accept two paths of the same sprite resource, one for a colorful image, and one for the SDF image, and the rendering library could combine the information to render colorful icons together with an icon halo.

CommanderStorm · 2024-10-20T18:51:06Z

I have not see a difference between the pngs and sdf images, except for the sdf: true metadata.

That was user error. Thanks for noticing, that feature would have not worked.

I have raised a PR documenting the behaviour upstream flother/spreet#85 and fixed the bug in my code ^^

CommanderStorm · 2024-10-20T18:58:49Z

martin/src/sprites/mod.rs


    Ok((name, sprite))
 }

 pub async fn get_spritesheet(
    sources: impl Iterator<Item = &SpriteSource>,
    pixel_ratio: u8,
+    as_sdf: bool,


Attention: There are 2-3 semver-breaking changes here.

Without reworking the API completely, I don't see how this can be avoided.

=> @nyurik would a major version bump be okay?

we are totally ok to keep bumping major for martin - no biggie

martin/src/sprites/mod.rs

docs/src/sources-sprites.md

nyurik

looks better with each pass!

WRT url - i'm still a bit on the fence on the API... /sprite/sdf/... does make sense, but then you have to handle if the user gives a sprite source named sdf (which probably is not that unusual)... On the other hand, a source called /sprite_sdf/... or /sdf/... or /sdf_sprite/... keeps naming collision resolution in one place (we already do this for sources), and keeps overall pattern consistent (no URL sub-branches). I would prefer to keep API flatter, just not sure of the naming.

martin/src/sprites/mod.rs

nyurik · 2024-10-21T16:00:14Z

martin/src/sprites/mod.rs


    Ok((name, sprite))
 }

 pub async fn get_spritesheet(
    sources: impl Iterator<Item = &SpriteSource>,
    pixel_ratio: u8,
+    as_sdf: bool,


we are totally ok to keep bumping major for martin - no biggie

martin/src/sprites/mod.rs

Co-authored-by: Frank Elsinga <[email protected]>

CommanderStorm · 2024-10-21T20:00:06Z

Have changed it to /sdf_sprite/.. as far as the choices prefered by you, that is the best imo.

The reason why I introduced it as /sprite/sdf/.. without any backwards compatibiltiy considerations is the following:

The default regular expression associated with a replacement marker [^/]+ matches one or more characters which are not a slash. For example, under the hood, the replacement marker {foo} can more verbosely be spelled as {foo:[^/]+}.

https://actix.rs/docs/url-dispatch/#generating-resource-urls

=> even if a sprite was previously named "sdf", it could not match the new routes

…ataformat

nyurik · 2024-10-22T00:34:22Z

Thx for changing - i do think it looks a bit better now, but if anyone has other thoughts, please chime in. @CommanderStorm could you add some tests to the test.sh? Make sure to run just bless afterwards.

CommanderStorm · 2024-10-22T11:21:02Z

could you add some tests to the test.sh?

Shure thing, have added it

nyurik

awesome work!!!

hiddewie · 2024-10-25T18:45:14Z

Great work @CommanderStorm, this gives nice results on my maps!

Uses maplibre/martin#1492 Unfortunately, SDF sprites cannot be layered right below each image, they have to be in their own layer below the icons. Symbols with outline: ![image](https://github.com/user-attachments/assets/da290ed7-441a-4c88-881a-88ee845d3acf) Hovering on rail lines in their native color: ![image](https://github.com/user-attachments/assets/79556b75-e6ab-4c37-95d6-87c9d22264ab) Railway direction including hover: ![image](https://github.com/user-attachments/assets/fe071904-2e9d-450f-8f63-e0613c8ac515)

CommanderStorm · 2024-11-03T20:51:16Z

We finally also had the time to finish the refactoring.
A bit of eye candy:

CommanderStorm force-pushed the sprite-dataformat branch from 4342a85 to 8fc96b4 Compare August 30, 2024 03:08

CommanderStorm marked this pull request as draft August 30, 2024 03:12

CommanderStorm changed the title ~~Implemented sdf file conversion~~ feat: implements sdf file conversion Aug 30, 2024

CommanderStorm commented Sep 25, 2024

View reviewed changes

docs/src/sources-sprites.md Outdated Show resolved Hide resolved

CommanderStorm commented Sep 25, 2024

View reviewed changes

docs/src/sources-sprites.md Outdated Show resolved Hide resolved

CommanderStorm added 5 commits September 26, 2024 00:59

implemented sdf

b295872

improve the wording in the docs

2691557

removed unnessesary change

8cade36

fixed the testcase having faulty handling of extensions

cf15c0f

fix an import error made earlier

b62ddd3

CommanderStorm marked this pull request as ready for review September 25, 2024 17:06

made sure that the docs are not all over the place in terms of wordin…

947bc6e

…g for this feature

CommanderStorm force-pushed the sprite-dataformat branch from 2db4897 to 947bc6e Compare September 25, 2024 17:13

hiddewie mentioned this pull request Oct 20, 2024

Generate outlines for symbols using SDF hiddewie/OpenRailwayMap-vector#84

Closed

CommanderStorm marked this pull request as draft October 20, 2024 13:02

CommanderStorm added 2 commits October 20, 2024 19:29

adapted the docs to a better way of configuring sdf files

cb8c1b8

ripped out the old way of configuring sdfs and added a new endpoint

1d8a8f8

CommanderStorm mentioned this pull request Oct 20, 2024

docs: made sure that a pitfall of make_sdf is documented flother/spreet#85

Merged

made sure that the sdf images are correctly generated

689cbb1

CommanderStorm changed the title ~~feat: implements sdf file conversion~~ feat: add sdf sprites via additional apis Oct 20, 2024

CommanderStorm added 2 commits October 20, 2024 20:56

reverted an unnecessary breaking change

c7d39c3

reworded parts of the docs

20e6519

CommanderStorm commented Oct 20, 2024

View reviewed changes

martin/src/sprites/mod.rs Outdated Show resolved Hide resolved

fixed markdown linting issues

54697b1

CommanderStorm marked this pull request as ready for review October 20, 2024 19:13

CommanderStorm commented Oct 20, 2024

View reviewed changes

docs/src/sources-sprites.md Show resolved Hide resolved

nyurik reviewed Oct 21, 2024

View reviewed changes

changed the url as requested

988c22d

CommanderStorm commented Oct 21, 2024

View reviewed changes

martin/src/sprites/mod.rs Outdated Show resolved Hide resolved

CommanderStorm and others added 2 commits October 21, 2024 21:56

fixed the service not being registered

bf00d85

Update martin/src/sprites/mod.rs

8ed9be0

Co-authored-by: Frank Elsinga <[email protected]>

CommanderStorm requested a review from nyurik October 21, 2024 20:14

Merge remote-tracking branch 'origin/sprite-dataformat' into sprite-d…

78f6814

…ataformat

added a test to test.sh

f425c4e

added the new endpoint to the endpoints

41eda74

nyurik approved these changes Oct 22, 2024

View reviewed changes

nyurik merged commit b134cb2 into maplibre:main Oct 22, 2024
18 of 19 checks passed

CommanderStorm deleted the sprite-dataformat branch October 22, 2024 16:50

nyurik mentioned this pull request Oct 22, 2024

Nominate @CommanderStorm maplibre/maplibre#418

Open

5 tasks

hiddewie mentioned this pull request Oct 25, 2024

Use SDF sprites to render icon halos hiddewie/OpenRailwayMap-vector#128

Merged

CommanderStorm mentioned this pull request Oct 29, 2024

Make icons ("sprites") colored TUM-Dev/NavigaTUM#1637

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `sdf` sprites via additional apis #1492

feat: add `sdf` sprites via additional apis #1492

CommanderStorm commented Aug 30, 2024 •

edited

Loading

nyurik commented Oct 20, 2024

CommanderStorm commented Oct 20, 2024 •

edited

Loading

hiddewie commented Oct 20, 2024

CommanderStorm commented Oct 20, 2024 •

edited

Loading

CommanderStorm Oct 20, 2024

nyurik Oct 21, 2024

nyurik left a comment

nyurik Oct 21, 2024

CommanderStorm commented Oct 21, 2024 •

edited

Loading

nyurik commented Oct 22, 2024

CommanderStorm commented Oct 22, 2024

nyurik left a comment

hiddewie commented Oct 25, 2024

CommanderStorm commented Nov 3, 2024

feat: add sdf sprites via additional apis #1492

feat: add sdf sprites via additional apis #1492

Conversation

CommanderStorm commented Aug 30, 2024 • edited Loading

nyurik commented Oct 20, 2024

CommanderStorm commented Oct 20, 2024 • edited Loading

hiddewie commented Oct 20, 2024

CommanderStorm commented Oct 20, 2024 • edited Loading

CommanderStorm Oct 20, 2024

Choose a reason for hiding this comment

nyurik Oct 21, 2024

Choose a reason for hiding this comment

nyurik left a comment

Choose a reason for hiding this comment

nyurik Oct 21, 2024

Choose a reason for hiding this comment

CommanderStorm commented Oct 21, 2024 • edited Loading

nyurik commented Oct 22, 2024

CommanderStorm commented Oct 22, 2024

nyurik left a comment

Choose a reason for hiding this comment

hiddewie commented Oct 25, 2024

CommanderStorm commented Nov 3, 2024

feat: add `sdf` sprites via additional apis #1492

feat: add `sdf` sprites via additional apis #1492

CommanderStorm commented Aug 30, 2024 •

edited

Loading

CommanderStorm commented Oct 20, 2024 •

edited

Loading

CommanderStorm commented Oct 20, 2024 •

edited

Loading

CommanderStorm commented Oct 21, 2024 •

edited

Loading