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

Icons specifications #2

Open
alessandro-saglimbeni opened this issue Mar 30, 2020 · 10 comments
Open

Icons specifications #2

alessandro-saglimbeni opened this issue Mar 30, 2020 · 10 comments

Comments

@alessandro-saglimbeni
Copy link
Collaborator

alessandro-saglimbeni commented Mar 30, 2020

Icons specification for Asset Registry

Format

  • .png

Size

  • 537x537 pixels

Transparency and colors

  • Make sure the logos within the icon are not transparent, otherwise they'll change color based on the various apps background colors
  • Make sure the borders are visible and result in the intended visual effect, irrespectively of the background color of the apps where the icon will be displayed (i.e. no molding in the background)

Padding

  • no padding on the longest edge,
  • icons should only have some padding on their shorter edge to have them fit within the 537x537 square. Inspect the two examples below with some graphic tool suite
    L-BTC has some padding to the left and right, because height>width
    USDt ha some padding above and below because width>height
@shesek
Copy link
Collaborator

shesek commented Mar 31, 2020

Is this actionable or just informative for issuers? Perhaps better to have this on the wiki and link to it from the README?

@greenaddress
Copy link

rather than README it may be good to have it shown on https://assets.blockstream.info/register.html ?

@alessandro-saglimbeni
Copy link
Collaborator Author

Not sure we want it public yet, currently we're allowing only few selected assets to have an icon. I'd have it as an issue template, but they changed format and I didn't have time to set it up.

Wiki is good for now, then when we'll have a larger flow of issuers asking to add their icons, we'll have to set up a vetting process anyway

@jgriffiths
Copy link

jgriffiths commented Jul 21, 2021

We should specify post-processing for icons to ensure they are as small as possible and adhere to the relevant standards.

The current icon set can be losslessly reduced in size by 17% (edit: see below), and contain artifacts like invalid color profiles which may prevent display with strict software. We should additionally specify that that EXIF data must be stripped.

I suggest we give a pngcrush command line for users to run before uploading, and reject icons if running that command on the icon again changes it.

@shesek
Copy link
Collaborator

shesek commented Jul 21, 2021

We should specify post-processing for icons to ensure they are as small as possible and adhere to the relevant standards.

Agreed.

I suggest we give a pngcrush command line for users to run before uploading, and reject icons if running that command on the icon again changes it.

AFAIU users won't be able to upload icons on their own (at least initially), someone from Blockstream will have to authorize and upload it through some management dashboard. So perhaps we could just apply pngcrush on our own?

(Note that unlike the other asset metadata, the asset icon isn't committed to in the issuance tx nor signed by the issuer public key. It is only signed in the assets registry git db repo with the registry PGP key.)

@jgriffiths
Copy link

jgriffiths commented Jul 21, 2021

So perhaps we could just apply pngcrush on our own?

@shesek This seems like a good idea, given we have issues with the download size in our wallets, especially over tor.

After experimenting with pngcrush a bit , the files can actually be reduced by 43% with no visual loss detectable using:

$ pngcrush --version
 pngcrush 1.8.13, uses libpng 1.6.36 and zlib 1.2.11
 Check http://pmt.sf.net/
 for the most recent version.
$ find . -type f -iname '*.png' -exec pngcrush -ow -rem iCCP -rem alla -reduce -m 0 -l 9 -brute {} \;

the asset icon isn't committed to in the issuance tx nor signed by the issuer public key

Seems like there is nothing stopping us then :D

@allenpiscitello
Copy link
Collaborator

We are embedding the icons in the application at the start and no longer downloading them until we can support downloading individual icons. We can certainly post this recommendation but the behavior in the future should mean the optimization will be less important since they won't need to download every icon.

@jgriffiths
Copy link

@allenpiscitello So far only iOS has implemented icon embedding I believe. Compiling in the compressed asset data has just been implemented in gdk rather than being re-implemented in all wallets and requiring other users to find their own solutions, it will be available in the next release 0.0.44.

The gdk solution has several advantages over per-wallet changes, but nonetheless the icon data maximally compressed adds ~1MB to the wallet with only 30 icons so far available in the registry (~2.5 MB for iOS, until it switches to the more efficient gdk implementation).

less important since they won't need to download every icon.

There are privacy issues with downloading a subset, the wallets will need to either download all icons or download more than they need to to create an anonymity set most likely. Optimising the data before committing it to the registry will reduce the impact of the privacy mitigation/allow twice the anonymity set. Given we have to vet icons before committing I think we should run the png optimisation ourselves before we commit the icon.

@allenpiscitello
Copy link
Collaborator

We plan to add thousands of icons, so the download all of them solution does not work. If someone is concerned about privacy, they can use Tor.

@jgriffiths
Copy link

We plan to add thousands of icons

Even more reason to optimise then!

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

No branches or pull requests

5 participants