Skip to content
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

allow setting a different extension than .jpg for router pictures #155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maurerle
Copy link
Contributor

This makes it possible to set a different router pictures extension in the config - needed for #154

@maurerle maurerle force-pushed the feature/pictures_ext branch from 58b32a8 to b052bdf Compare August 19, 2023 10:57
@maurerle maurerle force-pushed the feature/pictures_ext branch from b052bdf to 41c489d Compare August 19, 2023 11:16
@maurerle
Copy link
Contributor Author

As we do have vectorgraphics for everything now in the upstream pictures repository https://github.com/freifunk/device-pictures - it would be needed to merge this to make use of svg or png extensions

@belzebub40k @blocktrron
Can I help somehow to get this merged? :)

@blocktrron
Copy link
Member

@maurerle For supporting multiple extensions, I'd prefer a map for model <--> picture.

@maurerle
Copy link
Contributor Author

As modern devices can render svg quite fast and SVGs have a smaller filesize, I would like to have the option to directly use the pictures from here https://github.com/freifunk/device-pictures
This would remove the need to make sure that new pictures are added in both repositories.

This also includes all pictures of current existing routers.
So a map per model is not needed in anyway.

If this is merged, I can simply change the picture source here:
https://github.com/ffac/ff-supernode/blob/ec32d7eef3e1b8374e243e85f846121de1cec2d6/playbooks/roles/ff.firmware_selector/templates/config.js#L72

to https://map.aachen.freifunk.net/pictures-svg/ and use the same svg source as the current meshviewer is using:
https://github.com/freifunk/meshviewer/blob/94d1502d351a3d36b1f918c38e526d5ae8d41b52/config.example.json#L69

@herbetom
Copy link
Member

I'm not sure if it's worth offering/switching to SVGs. Sure, in theory they look nicer. But in practice it's about four times the traffic from a quick glance and not all clients render them correct (take a look at the top cover at https://github.com/freifunk/device-pictures/blob/1a231609e51803edfef517f4df1700a313ab78a3/pictures-svg/zte-mf281.svg and https://github.com/freifunk-darmstadt/gluon-firmware-selector/blob/279d269eec91d6de9fd3ae40b0aa15f596a4c838/pictures/zte-mf281.jpg).

@maurerle
Copy link
Contributor Author

@herbetom what do you mean with "four times the traffic"? Aren't the svgs less than half the size?

9.2M	pictures-jpg
8.4M	pictures-png
4.2M	pictures-svg

for the currently 371 pictures

Regarding your concern with the white space around the svg - it is not shown when using the picture itself:
https://map.aachen.freifunk.net/pictures-svg/zte-mf281.svg

and the firmware selector looks fine too:
https://firmware.freifunk-aachen.de/?q=zte (need to click the unrecommended checkbox to see the zte devices as they are broken)

@belzebub40k
Copy link
Collaborator

@maurerle For supporting multiple extensions, I'd prefer a map for model <--> picture.

I don't think we should add this as a setting on a per model basis. As we have the sources for all pictures we can easily convert them all to the same type. But as we already allow to change the folder for the pictures I think it should also be possible to change the used extension. I don't see a reason why we would prevent this.

@herbetom
Copy link
Member

@maurerle You're comparing "full size" JPGs with SVGs from what i can tell. Hoewer, the gluon-firmware-selector uses JPGs with a resolution of 256x256.

Our pictures dir has 2.9MB and it seems like about 1.6MB are served to / requested by a client currrently.
So yes, the x4 claim doesn't seem to hold up. But it's still less.

Regarding the rendering diffrences i mentioned:

The 256x256 JPG:
image

The SVG in a Browser:
image

With a higher resolution JPG it would be more obvious, but the boundaries of the top cover are for some reason not rendered with the correct width.

In addition have i exported the writing for the SVGs i created as Curves and not as text. That does increase the file size not insignifikantly. But this way there are no problems with missing fonts. But that's AFAIK not the case for all SVGs in the repo.

It's nothing majaor, but the advantage of JPGs is that you don't really have to worry about compatibily. I would definitely prefer just using SVGs. But it does come with it's drawbacks.

Copy link
Collaborator

@belzebub40k belzebub40k left a comment

Choose a reason for hiding this comment

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

This PR misses the possibility to change the license text for the pictures. This should be adjustable to give appropriate credit to all creators if necessary.

@maurerle
Copy link
Contributor Author

maurerle commented Sep 1, 2024

As downsized jpgs are superior to the use of the source svgs for now, I am closing this.

@maurerle
Copy link
Contributor Author

maurerle commented Dec 9, 2024

Allowing transparency and therefore dark mode is another advantage of the usage of other extensions than jpg

Sign up for free to 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.

4 participants