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

Add profile photo #36

Closed
hesterbruikman opened this issue Aug 6, 2020 · 37 comments
Closed

Add profile photo #36

hesterbruikman opened this issue Aug 6, 2020 · 37 comments
Assignees

Comments

@hesterbruikman
Copy link

#4

Add profile photo

Source: Roadmap planning workshop item status-im/status-react#48 and Discuss

User stories

  • As a user, I want to define a name and profile photo to display my contacts without having to change the ETH address, so that it is inviting to non-technical users and cryptocurrency enthusiasts and even inexperienced with encryption.

Use cases

  • View photo in Profile
  • View photo in other user profile
  • View photo in chat
  • View photo for 1:1 chat in chat list
  • View photo in notification
  • Update photo when registering ENS, in Profile or both (TBD)
    • Select photo from gallery
    • Take photo
    • Crop photo
    • Show photo is updating (if ca >8 sec)

Designs


Progress Summary

@hesterbruikman hesterbruikman changed the title Profile photo Add profile photo Aug 6, 2020
@Samyoul
Copy link
Member

Samyoul commented Aug 12, 2020

Team Comments

See feedback


Doubt

  • "I personally doubt it will improve retention."

Praise

  • "Would love to see this, will make it much more engaging - seems like it could be technically challenging tho with decentralised file storage?"
  • "crucial"
  • "Makes sense to implememnt and pretty straightforward"
  • "(+++)"

Implementation Considerations

  • "We need to have a good grasp on what syncinc looks like."
  • "will it be a part of ens ?"
  • "I'd like to be kept abreast of how this is stored and exposed to people on the network. Also, sanitize it before setting it in the applications."
  • "ENS contracts being migrated, not sure if it is effected by adding profile photos if handled through ENS"
  • "Via whisper or something like IPFS? Former would re-send stuff all the time"
  • "needs clarity where pics would live: on ENS or Waku"
  • "Persist with IPFS, or store locally and distribute via an image attachment to the contact update protobuf?"

Request 

 

  • "Can we call it avatar/persona or is that weird?"

@Samyoul
Copy link
Member

Samyoul commented Aug 12, 2020

Potential Approaches

ENS

An ENS approach could take advantage of ENS public resolver get-text-data also see EIP 634. Works well for users with a usable ENS name, but not usable for other users.


Waku

Local storage and contact detail sync.

  • Store photo on device.
  • User device sends contact detail message to chat they join
  • Chat participants make contact detail request for any other participant that they don't know.
    • Any chat participant that knows the contact photo can send a response
    • Need to know who should send contact photo and when to prevent network spamming.

IPFS

  • Upload image to IPFS
  • Attach IPFS address to chat message payload
  • Receiving device(s) parse the contact photo payload attach to related contact detail.

We could mix approaches. Perhaps defer to the ENS avatar.

@Samyoul
Copy link
Member

Samyoul commented Aug 13, 2020

Notes from yesterday's meeting

  • 14:00 UCT - Kick off 'Profile photo'
    • https://meet.google.com/edn-eonp-csm
    • Notes :
      • Profile Photos
      • Discussion of user display names:
        • The group agreed that adding only profile photos would be seen as delivering half a feature, and that the other half of this feature is allowing the user the ability to chose their own display name.
        • Need to write document detailing
          • Threat modeling for display name
          • Show rationale for why a display name would be good
          • Add an element of uniqueness, <display_name> + <first_8_chars_of_pubkey>, "Jupiter King 0xeb18d99c..."
    • Action Points
      • @Samyoul to:
        • Draft specs for implementing profile photos
        • Being collection of arguments and counter points for user display names

@errorists
Copy link

errorists commented Aug 17, 2020

for your consideration, here you'll find designs for the combination of display names and profile pics, and more specifically what would need to be changed to accommodate those 👉 Figma

@hesterbruikman
Copy link
Author

@Samyoul regarding draft specs. I'd be most curious how propagating a display name through the network could work. I don't know how this was handled in the past.

A concern I have is that we end up with either:

  • sending display name along with chat key in a way that they are or can be linked on mailservers (NO GO)
  • store chat key/name associations locally for specific users, bringing back the when do I see a random name versus display name confusion because we can e.g. only offer this through contact requests (Acceptable, but different from design which sets display name publicly regardless of contact status)

@Samyoul
Copy link
Member

Samyoul commented Aug 18, 2020

@errorists Thanks for the designs. They look cool.

So in chat a display name would look like the following:

Profile-–-Figma

And ENS name would look like the following:

Profile-–-Figma (1)

Is it possible to make the ENS name a little more premium looking? Maybe a badge or another colour for the name. Just something that makes that ENS name look like something you would want to spend money on.

@Samyoul
Copy link
Member

Samyoul commented Aug 18, 2020

@hesterbruikman thanks for these concerns.

sending display name along with chat key in a way that they are or can be linked on mailservers (NO GO)

No, the approach would involve the display name being included in the encrypted payload, so it would not be possible to even read the display name unless the message was publicly decryptable.

store chat key/name associations locally for specific users, bringing back the when do I see a random name versus display name confusion because we can e.g. only offer this through contact requests (Acceptable, but different from design which sets display name publicly regardless of contact status)

I think that the only feasible method of broadcasting a user's display name would be to include it with each message they send, ENS names works like this. This would mean that each message would have the display name of the sender attached. If a user changes her display name then the devices receiving new messages from the user would need to perform a backend update on the chat key > display name association or we have a chat where the messages using a particular display name show using that display name.

We don't need to have formal contacts, we can just have basic chat key > identity mappings that exist in the device's local storage for each identity that the user has communication with.

@Samyoul
Copy link
Member

Samyoul commented Aug 18, 2020

I've completed most of the work on the specification . Could @flexsurfer, @andremedeiros and / or @cammellos give your thoughts on the process proposed in my draft document?

@errorists
Copy link

@Samyoul re: ENS names more premium looking, we could do that if we wanted to, here's an example with an '@' badge, I tried the ENS logo too but at that size it's unrecognisable

Screenshot 2020-08-19 at 09 54 42

@cammellos
Copy link
Contributor

cammellos commented Aug 19, 2020

Thanks @Samyoul
I had a look at the docs, my observations would be:

Potential breaking change

// ChatMessageIdentity represents the user defined identity associated with their messages
message ChatMessageIdentity {

  // ens_name is the valid ENS name associated with the chat key
  string ens_name = 1;
  
  // display_name is the user's chosen display name
  string display_name = 2;
  
  // profile_image is the data associated with the user's profile image
  ProfileImage profile_image = 3;
}

This is a potential breaking change, as we already have a ens_name field in protobuf.
So we either populate both, at the cost of extra bytes (until we are happy to drop compatibility, but I would not break for something like this), or re-organize the fields in a way that is non breaking (i.e keep ens_name top level, and the rest is the same, or move display_image profile_image top level, which I think is probably neater, but either would work, or any other option).
Although I like the way they are organized, I think not having to deal with a potential breaking change is easier.

Bandwidth consumption

Sending images with each message (actual images, not url/ipfs hash) has a potential large impact on bandwidth. Effectively it would be like any message is an image message.
I am not sure this will cut it, roughly an image size could be up to 100KB, meaning joining an active public chat might end up consuming a considerable amount of bw (say 50 messages in a day, which is fairly low, might endup consuming ~5MB).
To me this is a bit of a show-stopper when it comes to sending images through waku with each message, unless we are able to keep the payload below a certain threshold.

Security consideration of pulling images from IPFS or other domains

I would want a whitelist for this, and not allowed by default, otherwise is trivial to detect a given user IP address, if they are online, etc. Otherwise it would invalidate many of the choices we have made before (waku, images, audio, push notifications).
All the consideration made here status-im/specs#69 (comment) applies basically.

Storing images

Images should be stored in the database, not on the device filesystem (or at least not in cleartext), but that's easy to change (we do this for normal images and profile pictures, which we had before and never fully removed).

Caching images

In this case I would assume that a given URL is unique to its content, so I would put the burden on the user changing picture to make sure that is the case, so we don't need to refresh the cache.
I.e. http://test.com/x will always be pointing to the same content and is unchangeable, we re-fetch only if we see that the url has changed to http://test.com/y. (x and y can be content-hash, but don't have to be).
It seems like it's not too much to ask for a client and we don't have to deal with identity_hash.
As a side note:

The identity_hash is a sha256 hash of the parsed ChatMessageIdentity protobuf, used to make checking for identity changes simple.

This might be a bit tricky, as adding a new field might have some issues as it would invalidate previous caches or we'd have to exclude it, but I think we can just get rid of identity_hash and check that the URL is different.

Mixing of UX and vanity features

This is more of a UX point,
I think if we market ourselves as a secure messenger, we need to be a bit more careful not to mix vanity and security features.
I believe this requires more careful thinking.
Currently we have two security features:

  1. 3 random words name
  2. Automatically generated profile image

(1) can already be overridden by using an ENS name, and will be overridden by using user-set profile names (ENS name are mutable, so it can be exploited)
(2) Will be overridden by setting a user-set profile image

I understand that in the designs we add a public key next to the name, which is good.
But the problem is that we are mixing 2 "security features" with vanity features, and it's not clear to the user what they should rely on to identify a user, or they might rely on photo/name which are not secure in fact:

photo -> no good as I can just set an identicon of someone else
name -> no good as I can just set any name

In my opinion we should not mix the two, as it looks bad for a security focused messenger, what's for security should be clear it's for security and what's vanity.

I think this needs a solution, some that comes to mind:

  • We always display both (identicon/profile-picture if set, 3 words name / user-set name)
  • We make the distinction very clear. A profile picture will have some indication that is user-set, a username will have some indication that is user set, not like https://www.figma.com/file/TNCyHKtR3sx5EL6YznFWUa4O/Profile?node-id=5102%3A19274 where "Teenage Ninja Turtle" is indistinguishable from a user set username (and therefore might or might not be user set, which can be exploited). This is a bit of a slippery solution, as you teach the user to rely on those, until they change.
  • We remove the security features and we settle on a single one (pubkey). For example we could replace generated profile pictures to something a bit more like discord, where there's some obvious duplication, so that users understand that they are not supposed to rely on them. 3 words name is harder to do, you could force users to set one and propagate it with messages, but we need to think about compatibility.

A mix of the above or other can be possible of course, but the point is that we should be very careful not to mix them.
Also if we display a pk, pk should always be displayed, even if a ENS name is set (ens names are mutable).

Conclusion

sorry for the long post :)
Unfortunately I don't see a neat way to implement this without making some concession, either in terms of bandwidth (images through waku) or privacy (ipfs,url).

I am not opposed to the idea on principle, although I would not implement profile pictures given the tradeoffs if it was my call. Sharing only with contacts is a bit easier and we were doing it already, but given the state of contacts now it would just sink the feature immediately, but would solve the bw problem and at that point we could just pass them along in waku.

I think if anything, "I set your picture" fits better the product, but gives a much worse UX admittedly.

Whatsapp does "set your own picture", but "I set your name", and I would think that's better, given that verifying identities in our product is already challenging.

In any case, if we want to go forward with this, the above is my suggestion on what needs to be addressed, very through specs @Samyoul , that's a definitely something we should take as an example!

@errorists
Copy link

@cammellos thanks for the writeup, really well thought out 🙏

re:

We remove the security features and we settle on a single one (pubkey) (…)

Yes, this was my thinking. There should really only be one source of truth to distinguish unique users and imo public key is the best one we got. 3w random name is as much a burden as it's a feature as we're finding out listening to people using the product. Leaving last call I was under the impression that we'll list those somewhere too and Jonny will share gathered community insights. I'm very much open to ideas how to make this transition better, like the one you shared about generated profile images.

@cammellos
Copy link
Contributor

cammellos commented Aug 19, 2020

@errorists
I agree, using the pk as a single way to identify a user is what we should go for.
3 random words name is also fine, as effectively equivalent, for now I would just use pk, but they are interchangeable.

my 2p are:

  1. Make pk more prominent, it should be the first thing a user see in a post (on its own line, or before the username)
  2. Drop automatically generate profile pictures and replace with something more like discord, so there's no confusion (also helps with branding, thinking about how popular facebook empty profile, discord alien, twitter egg are)
  3. As part of on-boarding, force the user to chose a username (this should be set even if they have an ENS name, as ENS name might change). This will help with also opening the accounts, as 3 words name is gone, and remembering pk might not be so easy for a user.

So a post would look something like:

(Profile picture 
/                          (pk) - User set name / ens name (this might be blank for compatibility reasons) (this might be two separate lines)
discord picture ) 

In terms of pk vs 3 random words user name, I don't know what's best, I like pks better, but I am a geek, and def. 3 words names are easier to remember.
But that's something easy to A/B test, it's trivial to get two builds one displaying always pks and one 3 word names, and we can let the community decide/user research etc.

@Samyoul
Copy link
Member

Samyoul commented Aug 19, 2020

@Samyoul re: ENS names more premium looking, we could do that if we wanted to, here's an example with an '@' badge, I tried the ENS logo too but at that size it's unrecognisable

Screenshot 2020-08-19 at 09 54 42

@errorists Yes, I like the "@" badge

@Samyoul
Copy link
Member

Samyoul commented Aug 19, 2020

@cammellos thank you very much for your feedback, I'm grateful for your perspective.

Potential breaking change

Yes, removing ens_name from the top level payload is a breaking change, but for future development having a dedicated Identity protobuf component would be more easily extensible.

So we either populate both, at the cost of extra bytes (until we are happy to drop compatibility, but I would not break for something like this)

To me this would be the solution. Keep the top level ens_name and populate both. The cost is only a few bytes like you say, so the proposed approach would compromise on not creating a breaking change for little downside.

Bandwidth consumption / full image payload attachment

To be honest with this option I was throwing things against the wall to see what we could make work.

I have another idea that uses dedicated identity messages that are not attached to the chat message payload. I suggested it in the original call, but I wanted to keep the identity management inline with pre-existing flows.

These messages would be sent to a chat and not be visible, but would be parsed effectively the same as an attached ChatMessageIdentity protobuf. This approach would only require the user to send the message once a month / once a week, frequent enough to keep the identity data accessible to any new participants. And if the user no longer participates in a chat then the identity message won't be resent after the "refresh" period expires.

Security consideration of pulling images from IPFS or other domains

otherwise is trivial to detect a given user IP address,

This is something that I didn't consider. That's a major issue. Hmmm. I think this is actually a supporting factor for my "other idea".

Storing images

Images should be stored in the database

Ok, that's fine.

Caching images

In this case I would assume that a given URL is unique to its content, so I would put the burden on the user changing picture to make sure that is the case, so we don't need to refresh the cache.

I actually assumed the opposite that the url for any given image is not reflective of the underlying payload. I could swap out a photo on my server keeping the same url.

An example is Gravatar, they use the same url for a centrally managed profile image. The link stays the same but the image may change at any time.

But I don't think that using URLs is going to be a viable option, at least not without major security precautions.

This might be a bit tricky, as adding a new field might have some issues as it would invalidate previous caches or we'd have to exclude it, but I think we can just get rid of identity_hash and check that the URL is different.

In my mind adding a new field would necessitate a contact update anyway.

ENS Avatars

ENS Avatars are a string, they can be anything. They can be a raw base64 encoded image or a URL, or a completely random string of nonsense.

ENS has been floated as a solution for user set profile photos, because ENS is decentralised, but the avatar may not be a decentralised solution is could be a link to https://supershadywebsite.ru/tracking_images/virus4u.png .

Identity hash

The identity_hash is not just for checking if the image has changed it is used to detect if anything in the identity has changed. It just means we only need to check a single field instead of checking the ENS name, the display name and the image data and any future parameter.

Mixing of UX and vanity features

I basically agree with you on what you say here, I'll defer to @errorists's opinion and judgement with regards what the solution to the presentation is.

Conclusion

I think IPFS is off the table, I think that URLs (Including those via ENS avatars) are off the table, but I do believe that there is still a path forward using Waku only. Images via URLs could be downloaded on the device of the user that is setting the image (therefore any risk associated with a untrustworth URL is assumed entirely by the user of the URL) and propagated using Waku.

Let me rework the specs and I'll propose a more robust solution.

sorry for the long post :)

Haha, I was actually happy to see your intimidating wall of text 😄 , I knew I was going to get some schooling.

@Samyoul
Copy link
Member

Samyoul commented Aug 20, 2020

@cammellos. @errorists I've completed spec draft v2.

The main differences between v1 and v2

  • Handling of IPFS, ENS Avatar and URL profile image sources are done only by the device of the user that is setting the profile image.
  • All ChatIdentity data is propagated via Waku
  • No other device is required to make an http(s) call to get image data
  • ChatIdentity is sent periodically to chats the user is active in and not per message.

Example chat

@Samyoul
Copy link
Member

Samyoul commented Aug 25, 2020

I've done a little research on Vector based profile images. We can get very high quality images in pretty small sizes.

We can attach things like:

  • Status Logo for 216 bytes
  • Ethereum Logo for 195 bytes
  • Google Logo for 748 bytes
  • Docker Logo for 2487 bytes

@Samyoul
Copy link
Member

Samyoul commented Sep 2, 2020

Rationale against the use of profile images

After analysis and thorough discussion, the team's consensus is that the use of images is not feasible for profile representation. The main concern is that for the optimum use of profile data propagation the data should be attached to each message the user sends, this creates serious bandwidth concerns.

Considered solutions with rejection reasons

  • IPFS
    • No guarantee that the IPFS gateway can be trusted not to log all IP addresses of Status client devices requesting profile images. Major security risk, would lead to the mass profiling of all status clients that show profile images.
  • URL
    • No guarantee that the image server can be trusted not to log all IP addresses of Status client devices requesting profile images. Major security risk, would lead to the mass profiling of all status clients that show profile images.
  • Waku
    • Raw data
    • IPFS (See above)
    • URL (See above)
  • ENS Avatar
    • Raw data
      • Gas cost to store the image data may likely exceed the gas limits of transactions.
    • IPFS (See above)
    • URL (See above)

Most feasible solution

Of the solutions considered the most feasible is detailed in the draft specs v2. This solution basically involves periodically (every 24 hours) sending profile data for active users of a topic to the topic.

This solution reduces major bandwidth consumption concerns at the cost of reducing the availability of profile data. The profile data may or may not be received by all parties in a Waku topic and users of a topic may have inconsistent experiences (such as receiving profile data of other users many hours after first interacting with them, or not at all).

Another potential solution is to publish the profile identity data detailed in draft spec v2 to a dedicated chat topic for the chat key. The down side of this solution is that the topic listen limit is only 10,000 and could quickly be overloaded with profile data only topics.

Most feasible alternative

Profile avatars can be in the range of 20 to 50 bytes in size and could be reasonably attached to every message sent by the user. Additionally avatar data could be easily stored in ENS avatar fields.

@errorists
Copy link

@Samyoul have you read Jarrad's response here, just asking if maybe we should reevaluate the profile images on waku proposal?

@Samyoul
Copy link
Member

Samyoul commented Sep 3, 2020

Thanks @errorists I've now caught up with that discussion.

@Samyoul
Copy link
Member

Samyoul commented Sep 9, 2020

I've submitted my proposal to update the Status specs here status-im/specs#150 . Also see the nice view of the proposal if git diffs are not your thing.

Because Jarrad has expressed his support of profile images, I've reworked my draft spec into something that I believe is feasible and uses as little extra bandwidth as possible, and can be realistically used in all chat contexts (1-1, group, public etc)

You'll notice that in the specs I've left room for avatars to be supported 😎 , even via ENS

@hesterbruikman
Copy link
Author

Thanks for adding the nice view link ;) As we know this will have an impact on bandwidth. Do you see any ways in which we might be able to test or project the impact? Not sure of we have Waku simulations that can be fed assumptions based frequency of profile image sharing.

Also, what's the current thinking on the connection with display names? Will profile images propagate at the same frequency of profile images? I'd like to prevent scenarios we had in the past where it was unclear what the 'status' of a user was (contact or not) as sometimes profile image with be visible, sometimes not.

Lastly, what would be research questions we might need to validate with users?
e.g.

  • to what extend do people understand how to set a profile image and display name?
  • to what extend do people understand how they appear to others at any given time?

@Samyoul
Copy link
Member

Samyoul commented Sep 10, 2020

TLDR

  • Get the answers to the current use questions in the Research Questions section
  • I'm going to make a simulation we can all play with to estimate profile image impact
  • Users either get all the identity data at the same time or you don't get any identity data

Simulating the impact of profile images

Do you see any ways in which we might be able to test or project the impact? Not sure of we have Waku simulations that can be fed assumptions based frequency of profile image sharing.

We can make some simulations, I'm not aware of any pre-existing ones that we could feed assumptions to. But, if we want to have numbers, I could take some averages of number of users in a chat topic and then calculate the worst case scenario and best case and maybe some kind of random mix.

1-1 chat

1-1 chat would have very low bandwidth usage, maybe about an extra 5kb per identity change. Presuming each user only updates their profile data once a month, that would be 5kb per active 1-1 chat.

  • Need to know: How to estimate the number of 1-1 chats a user has to estimate what the global total would be. We could survey users to see how many active 1-1 chats they have.

Group chat

Group chat would have a similarly low bandwidth, maybe about an extra 2.5kb per group member per identity change.

  • Need to know: How to estimate the average number of group chats users are active in and the average number of users in a group chat. Again surveys.

Public chat

This is the killer, this one will increase the bandwidth by quite a bit. Roughly 2.5kb per average active member per day per all
public chats.

Pulling numbers from the chats I know about, we have about 10 active public chats, with about an average of about 12 active users. So 10 x 12 x 2.5kb x 30 days = 9mb a month

  • Need to know What public chats are active and the average number of users based on this.

Let me create a simulation and put in my assumptions, I'll share it here and then we can change numbers and get better output.

Profile image propagation

Also, what's the current thinking on the connection with display names? Will profile images propagate at the same frequency of profile images? I'd like to prevent scenarios we had in the past where it was unclear what the 'status' of a user was (contact or not) as sometimes profile image with be visible, sometimes not.

So my specification creates a new concept of a ChatMessageIdentity which covers everything related to users identity data, profile image, ens name and display name. The ChatMessageIdentity is also extensible, meaning that we can add other features to the user ID, things like avatars, DIDs etc.

What this means is that the whole identity is broadcast as a single entity, so there is no risk of only getting a display name and no profile image etc. This means you either get all the identity data at the same time or you don't get any identity data, which in my opinion is the best user experience.

Research Questions

Lastly, what would be research questions we might need to validate with users?
e.g.

  • to what extend do people understand how to set a profile image and display name?
  • to what extend do people understand how they appear to others at any given time?

Not sure I understand this. are we talking about users of other products, such as WhatsApp? Sorry UX research is very alien to me.

There are also surveys of the current Status user base to answer my questions above:

  • How many active 1-1 chats does the average user have?
  • How many group chats users does the average user have?
  • What is the average number of users in a group chat?
  • How many public chats are active?
  • What is the average number of users of a public chat?

@Samyoul
Copy link
Member

Samyoul commented Sep 17, 2020

File Sizes

I've done some research work on cropping, resizing and compressing image data and I'm able to get pretty consistent results.

I've put together a repo dedicated to the findings, please click here to see lots of images https://github.com/status-im/image-resizer (there's even some goats).

Summary

Thumbnails

If profile images are 64px x 64px in size we can easily have file sizes comfortably below 2.5kb without major quality loss.

Image Size (px^2) Image Quality (%) Size (bytes)
64 90 2,455
64 90 2,236
64 90 2,170
64 90 1,752
64 80 2,540
64 90 1,911
64 90 1,082
64 90 2,512
64 90 1,746

We can even set a goal image size and write an algorithm that will find the correct quality to match the image size.

In my experimentation I've been able to select a target file size of 1.5kb without major quality loss in most cases. See the samples.

Larger Images

I've been able to get reasonable quality images of 256px x 256px in size around the 10kb mark.

Image Size (px^2) Image Quality (%) Size (bytes)
256 50 10,113
256 80 10,039
256 80 10,111
256 90 9,070
256 20 9,212
256 70 9,841
256 100 5,151
256 50 10,015
256 90 10,200

You'll notice that image quality varies wildly relative to file size at larger image sizes. If we target ~10kb we get the above results, the more complex images become the most noticeably degraded, but not horribly so.

I would recommend using the larger image size only for contacts. I don't know what our target size should be for this, but if we use larger payloads (25kb for example) for only contacts we can use 512px by 512px images with reasonable quality for users that wish to see the larger image of their contact's profile image See the samples.


My question to you @errorists @hesterbruikman and @cammellos is ... Is the quality of these images good enough?

@errorists
Copy link

errorists commented Sep 18, 2020

Nice! Profile thumbnails should be at 80x80 pixels in size, we display them at 40 points in the UI and that requires at least twice the pixels. Our local assets are stored at x3 times their size in points, but that would likely be overkill for photos.

For the larger guys, the highest we go is 120 points for these. so 240x240 pixels will be fine.

For compression, I'd limit it at no less than 60% jpg, in general 60-80% is the sweet spot.

@Samyoul
Copy link
Member

Samyoul commented Sep 18, 2020

Great, thank you for those numbers. I'll make an update to the prototype code and share the results.

@errorists
Copy link

also a random thought I have, we know those appear as circles in the UI so we likely could crop them as such and get rid of all of the data that's not shown in the UI, just replace it with white pixels or something. maybe it would make a small difference.

@hesterbruikman
Copy link
Author

Came for the goats, stayed for the numbers.

At face value, I think this is a very reasonable quality. @errorists could you please discuss the dimensions we use on Desktop with Simon? See if that changes anything? Also if we set a maximum, that needs to be known to Desktop so the UI can be adapted to it

@Samyoul
Copy link
Member

Samyoul commented Sep 18, 2020

I've updated the image sizes to only show 80px and 240px, I've also experimented with circle cropping like you suggested and the results were weird. Sometimes the file size is increased sometime the file size is decreased, on average the file size is increased. See below:

Image Properties Size (px^2) Image Quality (%) Size (bytes)
80 80 2,440
pre-render circle crop 80 80 2,802
post-render circle crop 80 80 2,798
80 80 2,217
pre-render circle crop 80 80 2,377
post-render circle crop 80 80 2,380
80 80 2,134
pre-render circle crop 80 80 2,439
post-render circle crop 80 80 2,440
80 90 2,137
pre-render circle crop 80 90 2,141
post-render circle crop 80 90 2,138
80 60 2,554
pre-render circle crop 80 60 2,503
post-render circle crop 80 60 2,479
80 80 1,931
pre-render circle crop 80 80 2,265
post-render circle crop 80 80 2,261
80 90 1,274
pre-render circle crop 80 90 2,480
post-render circle crop 80 90 2,483
80 80 2,544
pre-render circle crop 80 80 2,564
post-render circle crop 80 80 2,550
80 90 2,219
pre-render circle crop 80 90 1,996
post-render circle crop 80 90 2,005

@errorists
Copy link

lol, fuck logic amirite? 😅 let's do without cropping then. for compression quality, I think we could also max it at 80%, as at this small size going up will yield increasingly diminishing returns. thanks man, the baby elephant is cute.

@Samyoul
Copy link
Member

Samyoul commented Sep 18, 2020

The results for 240px images.

Basically the same but the margins of difference are much larger.

It think the reason for this counter intuitive result is because the compression algorithms seek to keep the image as close to the original as possible by matching colours, and adding none congruent black shapes makes it harder to do this. Also a circle is an unhelpful shape, it basically intersects with lots of the compression chunks and in most cases increases the work required to compress the image.

I tried just adding a black square in the corner and that did consistently and significantly reduce the file size. Not very nice to look at though.

See the 240px images
Image Properties Size (px^2) Image Quality (%) Size (bytes)
240 60 10,224
pre-render circle crop 240 60 10,295
post-render circle crop 240 60 10,248
240 80 9,138
pre-render circle crop 240 80 9,519
post-render circle crop 240 80 9,499
240 80 9,228
pre-render circle crop 240 80 10,267
post-render circle crop 240 80 10,269
240 90 8,218
pre-render circle crop 240 90 8,209
post-render circle crop 240 90 8,212
240 30 10,419
pre-render circle crop 240 30 9,413
post-render circle crop 240 30 9,406
240 70 8,793
pre-render circle crop 240 70 8,931
post-render circle crop 240 70 8,938
240 100 3,711
pre-render circle crop 240 100 14,517
post-render circle crop 240 100 14,535
240 60 10,274
pre-render circle crop 240 60 9,560
post-render circle crop 240 60 9,511
240 90 9,223
pre-render circle crop 240 90 7,754
post-render circle crop 240 90 7,726

@Samyoul
Copy link
Member

Samyoul commented Sep 18, 2020

I think we could also max it at 80%

Ok, I can write an algorithm that does the following:

  • Compress the image to 80%
    • if file size is < 2.5kb take this render
    • else render the image at 70%
      • if file size is < 2.5kb take this render
      • else render the image at 60% and take this render regardless of size.

Hopefully on average the file size will be below the 2.5kb mark.

@errorists
Copy link

you the man! sgtm, the quality is plenty enough. I'm pumped it's finally happening 🙌

@Samyoul
Copy link
Member

Samyoul commented Sep 18, 2020

File size limit justification

Thumbnail

The max size limit of 2.5kb for thumbnails was decided based on the following basis:

  • Images should be rendered at a quality between 60% and 80%
  • Images should be 80px x 80px
  • The largest file size at quality 80% should be <= the largest file size at 60% quality
Results :
Image Size (px^2) Image Quality (%) Size (bytes)
80 80 2,440
80 80 2,217
80 80 2,134
80 80 1,669
80 60 2,554
80 80 3,683
80 80 1,931
80 80 1,185
80 80 2,544
80 80 1,743

Findings

  • The largest file size at 80% was 3,683 bytes
  • The average file size at 80% is 2,172 bytes
  • The largest file size at 60% is 2,554 bytes
  • The average file size within the limits is 2,046 bytes

Profile Image

The max size limit of 16kb for thumbnails was decided based on the following basis:

  • Images should be rendered at a quality between 60% and 80%
  • Images should be 240 x 240px
  • The largest file size at quality 80% should be <= the largest file size at 60% quality
Results :
Image Size (px^2) Image Quality (%) Size (bytes)
240 80 15,276
240 80 9,138
240 80 9,228
240 80 5,718
240 60 16,143
240 80 24,290
240 80 11,033
240 80 2,982
240 80 15,321
240 80 6,603

Findings

  • The largest file size at 80% was 24,290 bytes
  • The average file size at 80% is 11,065 bytes
  • The largest file size at 60% is 16,143 bytes
  • The average file size within the limits is 10,159 bytes

@cammellos
Copy link
Contributor

@errorists / @hesterbruikman would you mind updating (or just confirming) the designs on this issue with an MVP that we would be happy to release? (or if you could list the points that still we need to clarify).
Thanks!

@errorists
Copy link

@cammellos sure, the full scope of the interface required to facilitate this is found here. It's quite simple, no need for any MVP. I'll also update the original link in the issue

@errorists
Copy link

actually, now that I think about it, it's not that different to what we had before removing Profile Pictures before v1 😅 if you guys still have that somewhere, it should take you at least halfway there with UI

@hesterbruikman hesterbruikman transferred this issue from status-im/status-mobile Dec 1, 2020
@hesterbruikman
Copy link
Author

Closing as this has been implemented

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

4 participants