Helium314 / SCEE

OpenStreetMap surveyor app for experienced OSM contributors
GNU General Public License v3.0
154 stars 12 forks source link

Added valves quest #644

Closed mcliquid closed 2 months ago

mcliquid commented 2 months ago

Fixes https://github.com/streetcomplete/StreetComplete/issues/4450

TODO:

mcliquid commented 2 months ago

Screenshot_20240909-101419~2.png

mcliquid commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

valve

mcliquid commented 2 months ago

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

mnalis commented 2 months ago

The author from Austria consents to their use with reference to the license, his name and his website

the text is:

Either the notice is given in accordance with the license conditions or as follows:: Ralf Roletschek / Roletschek.at with link to my homepage near to the photo or at a location that is common in your publication but preserving the association between image and credit. In any event, copyright and license are to be mentioned.

"Please review the full license requirements carefully before using this image. If you would like to clarify the terms of the license or negotiate less restrictive commercial licensing outside of the bounds of Licenses, please contact me by email ralf@roletschek.at"

If might be fine with "location that is common in your publication but preserving the association between image and credit." provided that "In any event, copyright and license are to be mentioned." in authors.txt

Or should I write to the author personally and ask?

As it seems unclear (or you wouldn't be asking :smiley: ) what would be fine with author, I think you should ask the author if your current authors.txt line would be fine with him (that is how we credit all other pics, in source only)?

+ valves_schrader.jpg     Copyright     Ralf Roletschek / Roletschek.at https://en.wikipedia.org/wiki/File:Schrader-ventil.jpg

Adding link in that existing one-line is fine for us; but we cannot add links (as that is plaintext file, not a HTML or MD - we can add leading "https://" text though, but that is technically not a link), and quoting whole license in separate HTML file is more annoying but possibly doable (it would be first of that kind); but I definitely don't think embedding name, website, license into the picture would be a good solution for us.

For us it would be easiest if current attribution (as you made in your file) is fine with them, or they agreed the pictures be licensed under CC-BY or CC-BY-SA (which would ensure they are mentioned in authors.txt alongside others)

If they don't want to, or do not respond, we'd probably need to find other images.

https://commons.wikimedia.org seems like it should have usable alternatives. (or you could look there anyway, some pics there looks even clearer to me, and it might be like less work than clarifying / renegotiating license)

mcliquid commented 2 months ago

@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 commented 2 months ago

New screenshot with the new icon and the new pictures: Screenshot_20240909-164705.png

HolgerJeromin commented 2 months ago

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

mcliquid commented 2 months ago

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

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

@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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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

Helium314 commented 2 months ago

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

mcliquid commented 2 months ago

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!

mnalis commented 2 months ago

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 valves_dunlop_gimp_q50_arith.jpg. 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 commented 2 months ago

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?

image
mnalis commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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