Added valves quest #644

mcliquid · 2024-09-08T21:11:17Z

TODO:

Add custom icon (can't find a "valve" on SVG repo)
Enable multi selection of values

mcliquid · 2024-09-09T08:21:55Z

From my point of view, the Quest is ready for review. I have tested it myself and it works well.
I am now looking for a better / own Quest icon. If I can't find a good one, I would go with the air pump icon.

mcliquid · 2024-09-09T08:24:13Z

Almost forgotten: The three pictures with the dark blue background are copyrighted. The author from Austria consents to their use with reference to the license, his name and his website. Is this sufficient via the authors.txt? Or should the name be written on the image? Or should I write to the author personally and ask?

mcliquid · 2024-09-09T08:32:47Z

I tried to make my own icon with a valve. What do you think?
P.S.: It looks like a spark plug.

mcliquid · 2024-09-09T08:37:56Z

I would say it's okay once you know what it is because you activated the quest in the quest settings anyway.

…more unified)

mcliquid · 2024-09-09T14:40:31Z

@mnalis Thanks for the hint to search Wikimedia Commons! I immediately went to the pictures on the wiki without considering whether there were any alternatives. So great that Bohwaz photographed all four valves with the same technique and made them available under copyleft! PR has been updated.

mcliquid · 2024-09-09T14:47:43Z

New screenshot with the new icon and the new pictures:

HolgerJeromin · 2024-09-09T15:27:45Z

https://www.schwalbe.com/clik-valve-ventil/
Will be released in a few weeks/month :-)

app/src/main/res/authors.txt

Co-authored-by: Holger Jeromin <mailgithub@katur.de>

mcliquid · 2024-09-09T15:34:03Z

https://www.schwalbe.com/clik-valve-ventil/
Will be released in a few weeks/month :-)

the SCVs will be backwards compatible with normal SV valves anyway.
the new valves were announced months ago, but are still not available and therefore not yet available "on the ground".
do you have a suitable picture for the quest? :)

app/src/main/res/authors.txt

mcliquid · 2024-09-10T07:34:56Z

@HolgerJeromin (Off-Topic): I created the wiki page for valves. I could also include the new Schwalbe Clik valve, but I wanted to make sure that you would also have captured it with valves=clik? So far there is no usage and as soon as it is added to the wiki, we will give a direction. The official name is “Schwalbe Clik Valve” with the abbreviation “SCV”. Would schwalbe_clik_valve then be more correct?

HolgerJeromin · 2024-09-10T10:19:25Z

@mcliquid Good question.
The other values are single "vendor" names.
So perhaps schwalbe could work. But clik could work, too, as it is more iconic.
Wiki talk page or forum is probably a better place to discuss.

mnalis · 2024-09-10T11:54:38Z

Wiki talk page or forum is probably a better place to discuss.

Yes, I'd suggest making a poll at https://community.openstreetmap.org/c/general/tagging/70 what people would prefer

(and link to that community discussion here and on wiki talk page)

mcliquid · 2024-09-10T11:59:24Z

Let's see how things will develop. Not relevant for this PR right now :)

app/src/main/java/de/westnordost/streetcomplete/quests/valves/ValvesItem.kt

Helium314 · 2024-09-10T17:10:41Z

app/src/main/java/de/westnordost/streetcomplete/quests/valves/AddValves.kt

+        getMapData().filter("nodes, ways with amenity = compressed_air or service:bicycle:pump = yes or compressed_air = yes")
+
+    override fun applyAnswerTo(answer: List<Valves>, tags: Tags, geometry: ElementGeometry, timestampEdited: Long) {
+        tags["valves"] = answer.joinToString(";") { it.osmValue }


Do you think sorting the values could be useful?

Good question, there are already all possible combinations in taginfo. I guess a consuming app will have to extract the values anyway. As far as I can see there are only the Orchard Produce and the Sports Quest with multi-value answers, and neither does sort the values. Would it be easy to implement?

I too see no benefit to sorting...

Would it be easy to implement?

should be answer.sortedBy { it.osmValue }.joinToString(";") { it.osmValue }

There is no benefit for consumers checking for specific valves, but e.g. if SCEE ends up being the most used editor for this tag (and thus values being mostly sorted), it could make reading a list as posted by @mnalis a little easier.

Helium314 · 2024-09-10T17:17:03Z

Is it possible to make the images a little smaller, so this quest doesn't add another ~700 kB to apk size?

mcliquid · 2024-09-10T18:36:12Z

Is it possible to make the images a little smaller, so this quest doesn't add another ~700 kB to apk size?

Sure, I completely misunderstood the image sizes here: https://github.com/streetcomplete/StreetComplete/blob/master/CONTRIBUTING_A_NEW_QUEST.md#new-photos

I thought if it says 384px for three columns then it's 1152px for two columns, but it's for one. So instead of 1152px it should be 576px. Fixed that!

…by approximately 30%.

mnalis · 2024-09-10T20:11:21Z

I thought if it says 384px for three columns then it's 1152px for two columns, but it's for one. So instead of 1152px it should be 576px. Fixed that!

Also, passing images through some optimizer reduces the size significantly more, for basically no quality loss.
for example, random web search returned https://tinyjpg.com/
reduces drawable-xxdpi/valves_dunlop.jpg from 34989 to just 17927 bytes for no visible change, so that is the minimum which should be done. That is 48% size reduction with no visible changes to image quality and very little extra work!

Using gimp with jpeg quality set to 50% (and few advanced options like non-progressive arithmetic coding with 4:2:0 subsampling), can produces JPEG files of just 12138 bytes, with extremely tiny differences which do not impact the usability at all, like . That is over 65% size reduction compared to original images.

Can you @mcliquid save in such optimized format, or would you like me to preprocess those images for you?

mcliquid · 2024-09-10T20:35:49Z

for basically no quality loss.
with no visible changes to image quality

In my view, this is not correct. I always save JPEGs with a 60% quality index. In my view, anything below that generates very clear JPEG artifacts. Also visible in this comparison, especially on the thread on the right. This is now 60 % vs. 20 %.
But TBH, I don't really care in this case ;) Should I optimize all the images of the whole app at once?

mnalis · 2024-09-10T21:28:06Z

I always save JPEGs with a 60% quality index

Well, for this example in this PR, it seems iit slipped? e.g.:

% identify -verbose ~/Downloads/valves_presta.jpg | grep Quality
  Quality: 86

In my view, anything below that generates very clear JPEG artifacts

Usually yes, but in my example 50% was fine too, with only very tiny changes.

This is now 60 % vs. 20 %.

Wow, it looks quite good even at 20%. Even while differences are detectable; that is not what we care about here -- we care about images being equally good for their function, and both of those images look equally good to me even on a desktop screen; I doubt I'd even be able to see the artifacts on mobile screen.

So yes, I think this is much better, especially for 68% size reduction (to just 32% of original)!

Should I optimize all the images of the whole app at once?

Definitely not in this PR, I'd say. But I'd say please do for all images this PR introduces (taking care they don't degrade visually too much to become ugly, of course). And probably not in SCEE (Idea is to try to keep as much common base as possible with upstream, to reduce merge issues).

But I think it should be suggested in upstream as it would benefit equally well (assuming images there are equally unoptimized as ones in this PR); but the bigger issue with replacing all images is that each image should be checked whether there is noticeable degradation before replacing it.
Because, especially as app has grown from ~60MB to ~90MB, it is quite significant, and we should try to reduce size to more reasonable numbers.

And "how to optimize image size" should be mentioned in instructions at https://github.com/streetcomplete/StreetComplete/blob/master/CONTRIBUTING_A_NEW_QUEST.md#new-photos (but also as new PR at upstream)

HolgerJeromin · 2024-09-11T04:11:39Z

JFYI https://squoosh.app/
Is a very good tool which is a website but works locally on you files.
It has a cool preview feature to compare quality.

mcliquid · 2024-09-11T06:51:08Z

To get back to the topic and not get any further off-topic here:

The downsizing of images outside of this quest should be moved to another ticket.
Are there any changes left for this quest?

app/src/main/java/de/westnordost/streetcomplete/quests/valves/AddValvesForm.kt

from discussion at Helium314#644 (comment)

* general advice about reducing file size manually * mention online tools to simplify size/quality comparison from discussion at Helium314#644 (comment) * emphasize playing with quality, without recommending exact values * mention online SVGOMG * mention GIMP * do not mention values at all

Added valves quest

fe4b407

This comment was marked as outdated.

Sign in to view

mcliquid added 2 commits September 9, 2024 10:15

Optimized strings

25b2949

Restructured quest files, unified naming, added multi-selection

65f4744

mcliquid marked this pull request as ready for review September 9, 2024 08:20

Added own quest icon (valve icon)

8d351b6

This comment was marked as resolved.

Sign in to view

Changed images to copyleft versions (and all from the same author -> …

3e7df96

…more unified)

HolgerJeromin reviewed Sep 9, 2024

View reviewed changes

app/src/main/res/authors.txt Outdated Show resolved Hide resolved

Update app/src/main/res/authors.txt

2cc0870

Co-authored-by: Holger Jeromin <mailgithub@katur.de>

mnalis reviewed Sep 9, 2024

View reviewed changes

app/src/main/res/authors.txt Outdated Show resolved Hide resolved

Fixed license

a5a6866

mnalis approved these changes Sep 10, 2024

View reviewed changes

Helium314 reviewed Sep 10, 2024

View reviewed changes

app/src/main/java/de/westnordost/streetcomplete/quests/valves/ValvesItem.kt Outdated Show resolved Hide resolved

Helium314 reviewed Sep 10, 2024

View reviewed changes

app/src/main/java/de/westnordost/streetcomplete/quests/valves/ValvesItem.kt Outdated Show resolved Hide resolved

Helium314 reviewed Sep 10, 2024

View reviewed changes

mcliquid added 2 commits September 10, 2024 20:37

Image dimensions reduced by approximately 50% and image size reduced …

d9c55e5

…by approximately 30%.

Optimized code after feedback

8515da0

reduce image quality

9799247

reduce image quality

eed5386

Helium314 reviewed Sep 11, 2024

View reviewed changes

app/src/main/java/de/westnordost/streetcomplete/quests/valves/AddValvesForm.kt Outdated Show resolved Hide resolved

change not null behavior, values sorted by usage from taginfo

1dba914

Helium314 merged commit b116b81 into Helium314:modified Sep 12, 2024

This was referenced Sep 13, 2024

New quest: valve types on bicycle pumps streetcomplete/StreetComplete#4450

Closed

AddBicyclePump, for bicycle_repair_station. Adding only pump streetcomplete/StreetComplete#5870

Closed

mcliquid deleted the valves branch September 19, 2024 19:24

mnalis added a commit to mnalis/StreetComplete that referenced this pull request Oct 1, 2024

mention online tools to simplify size/quality comparison

a681c3a

from discussion at Helium314#644 (comment)

mnalis mentioned this pull request Oct 1, 2024

Docs: suggest reducing image size for new quests streetcomplete/StreetComplete#5937

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added valves quest #644

Added valves quest #644

mcliquid commented Sep 8, 2024 •

edited

Loading

This comment was marked as outdated.

mcliquid commented Sep 9, 2024

mcliquid commented Sep 9, 2024

mcliquid commented Sep 9, 2024 •

edited

Loading

mcliquid commented Sep 9, 2024

This comment was marked as resolved.

mcliquid commented Sep 9, 2024

mcliquid commented Sep 9, 2024

HolgerJeromin commented Sep 9, 2024

mcliquid commented Sep 9, 2024

mcliquid commented Sep 10, 2024

HolgerJeromin commented Sep 10, 2024

mnalis commented Sep 10, 2024

mcliquid commented Sep 10, 2024

Helium314 Sep 10, 2024

mcliquid Sep 10, 2024

mnalis Sep 10, 2024

Helium314 Sep 11, 2024

Helium314 commented Sep 10, 2024

mcliquid commented Sep 10, 2024

mnalis commented Sep 10, 2024 •

edited

Loading

mcliquid commented Sep 10, 2024

mnalis commented Sep 10, 2024 •

edited

Loading

HolgerJeromin commented Sep 11, 2024

mcliquid commented Sep 11, 2024

Added valves quest #644

Added valves quest #644

Conversation

mcliquid commented Sep 8, 2024 • edited Loading

This comment was marked as outdated.

mcliquid commented Sep 9, 2024

mcliquid commented Sep 9, 2024

mcliquid commented Sep 9, 2024 • edited Loading

mcliquid commented Sep 9, 2024

This comment was marked as resolved.

mcliquid commented Sep 9, 2024

mcliquid commented Sep 9, 2024

HolgerJeromin commented Sep 9, 2024

mcliquid commented Sep 9, 2024

mcliquid commented Sep 10, 2024

HolgerJeromin commented Sep 10, 2024

mnalis commented Sep 10, 2024

mcliquid commented Sep 10, 2024

Helium314 Sep 10, 2024

Choose a reason for hiding this comment

mcliquid Sep 10, 2024

Choose a reason for hiding this comment

mnalis Sep 10, 2024

Choose a reason for hiding this comment

Helium314 Sep 11, 2024

Choose a reason for hiding this comment

Helium314 commented Sep 10, 2024

mcliquid commented Sep 10, 2024

mnalis commented Sep 10, 2024 • edited Loading

mcliquid commented Sep 10, 2024

mnalis commented Sep 10, 2024 • edited Loading

HolgerJeromin commented Sep 11, 2024

mcliquid commented Sep 11, 2024

mcliquid commented Sep 8, 2024 •

edited

Loading

mcliquid commented Sep 9, 2024 •

edited

Loading

mnalis commented Sep 10, 2024 •

edited

Loading

mnalis commented Sep 10, 2024 •

edited

Loading