Page MenuHomePhabricator

Generate unique? profile images for each new user
Closed, ResolvedPublic

Description

There are enough Pokemon and colors available, we could build something automated for new users to get something unique.

Event Timeline

chad created this task.Feb 10 2016, 10:04 PM
avivey added a subscriber: avivey.

nux "select profile image" interstitial?

chad added a comment.Feb 10 2016, 11:26 PM

721 pokemon x lots of colors.

seriously though, I think this is really good idea; I'm not sure if it would help if registering via oauth (And getting the default oauth provider image).

Would anyone else use "Suggest a picture for another user" if we built it (T4104)? I think this feature is hilarious/social/fun/easy but maybe I'm alone in that.

Particularly, I like that it allows users who are feeling the most pain (e.g., triager or PM types) to gently pressure users to bother doing setup properly by suggesting awful pictures, or do the legwork and suggest good ones.

I would probably use that for every new team member that was still using the default/bad picture after some time.

I would use it to give Chad a new hand-drawn picture every day.

I would take great delight in using such a tool

chad triaged this task as Wishlist priority.Jun 12 2016, 7:55 PM
chad added a comment.Feb 26 2017, 5:41 PM

Shapes x Colors x Letters seems most clear, but will have some collisions. I can mock something up. The letters would be generated images like in Projects? Or can this be all GD? I think that assumes fonts.

We can draw letters in GD, but I suspect they won't look very good compared to letters from Photoshop (bad antialiasing, limited font choices, weird alignment/kerning issues), so I'd guess it's not worth bothering and we might as well just jump straight to using generated images like Projects.

chad added a comment.Feb 26 2017, 6:04 PM

Googling around I can find programmatically generated lists of 256 colors x 26 letters = 6600. We can also make background shapes. 4,5,6,7,8 sided. This would give us 33k uniques.

chad added a comment.Feb 26 2017, 6:06 PM

Honestly I'm fine with 6600 uniques out of the box. I think beyond that people can update their image. Mostly I'd like it for individual conversations (tasks with newbies), so I don't expect collision in those cases.

chad added a comment.Feb 26 2017, 6:42 PM

It'd look something like this:

chad added a subscriber: 0.Feb 26 2017, 6:49 PM

A-Z and 0-9 is all we need here? Gotta remember user @0

It looks like we currently allow usernames to begin with _, ., and - (PhabricatorUser->validateUsername()), but maybe just do a ? for anything-except-numbers-and-letters?

chad added a comment.Feb 27 2017, 7:30 PM

Ok, first part is checked in and seems reasonable. https://secure.phabricator.com/uiexample/view/PhabricatorFilesComposeAvatarExample/

Remaining steps:

  • Generate real images
  • Auto assign real images based on super secret recipe.
chad added a comment.Feb 28 2017, 12:26 AM

Since I'm not a real programmer, getting stuck on how to assign something by username. I presume we want every chad to get the same pre-built avatar if possible. I could just randomize it, but that seems not really beneficial.

So we have 2 different letters, 256 colors, and 2 borders. Is there something that can take a hash of chad and get a consistent (1,2)(1,256)(1,2)?

chad added a comment.Feb 28 2017, 12:28 AM

Actually, may have found something on "the google". bbs

diff --git a/src/infrastructure/util/PhabricatorHash.php b/src/infrastructure/util/PhabricatorHash.php
index 00d7cca63c..5dfbbe2617 100644
--- a/src/infrastructure/util/PhabricatorHash.php
+++ b/src/infrastructure/util/PhabricatorHash.php
@@ -78,6 +78,22 @@ final class PhabricatorHash extends Phobject {
     return $result;
   }
 
+  public static function digestToRange($string, $min, $max) {
+    if ($min > $max) {
+      throw new Exception(pht('Maximum must be larger than minimum.'));
+    }
+
+    if ($min == $max) {
+      return $min;
+    }
+
+    $hash = sha1($string, $raw_output = true);
+    $value = head(unpack('L', $hash));
+
+    return $min + ($value % ($max - $min));
+  }
+
+
 
   /**
    * Shorten a string to a maximum byte length in a collision-resistant way

Add some junk when hashing to get different values:

$color = PhabricatorHash::digestToRange($chad.'profile-color', 1, 256);
$border = PhabricatorHash::digestToRange($chad.'profile-borrder', 0, 1);
...
chad added a comment.Feb 28 2017, 1:10 AM

Oh is crc32 bad?

chad added a comment.Feb 28 2017, 1:59 AM

Works great!

crc32 would be fine in this use case too since the requirements are so lax. My version isn't particularly robust either, but should get the job done.

chad added a comment.Feb 28 2017, 2:18 AM

I'm going to get this to being able to build a real file, and hand it over unless you think the rest is trivial. Not sure how this would need to work in the NUX case. Generate an image when PhabricatorUser is triggered? What about old defaults?

chad added a comment.Feb 28 2017, 7:18 PM

I'm fumbling with GD, I did try solid color borders, which work fine, but are overkill (millions of options). I wanted to go back to RGBA, but not finding a good reference on converting rgba(255,255,255,.4); into a GD friendly color.

chad added a comment.Feb 28 2017, 7:18 PM

If it's not simple, I can probably fake border colors with builtin colors we use.

I think you can use imagecolorallocatealpha($img, $r, $g, $b, $a) to do it for you. Note that alpha is 0-255, not 0-1.

Otherwise it's something like this, I think, but this might be completely wrong and I didn't test it at all:

function rgba2gd($r, $g, $b, $a) {
  $a = (1 - $a) * 255;
  return ($a << 24) | ($r << 16) | ($g << 8) | $b;
}

I think the NUX case is fairly tricky. I need to look at the code, but I'm thinking something like:

  • Generate the image in the Query, not when the user is created (helps with old users and not having to update a ton of different ways users can get created, and not generating these as side effects during unit tests).
  • Don't break if gd is missing.
  • Maybe add some kind of like bin/auth bestow-avatar to make testing easier?
  • Maybe record which avatar version we've generated so we can automatically upgrade these avatars later (for example, if we fix a bug or pick different colors)? Right now, there would be no way to say "regenerate the avatars for all users who haven't selected an avatar of their own", but it would be nice to be able to do that.
chad added a comment.Mar 1 2017, 4:47 PM

Why not require GD outright? Seems like a drop in the bucket with all other requirements and leads to confusion later like with projects or macros

I think all the other requirements are usually builtin (we "require" them only because it's possible to remove them, and some distros occasionally do for kind-of-maybe-questionable reasons) but I think it's basically like requiring your car to have tires.

gd usually needs to be installed separately and can sometimes be hard to install since it depends on libpng/libjpeg -- more like requiring a bike rack, to stretch this analogy?

We already "require" it in the sense that we give you a setup warning which says "nothing related to images will work until you fix this", but we don't hard-fatal you like we do if you're missing mysql / mysqli.

A personal case here is that I occasionally build php from source to test or debug things, but building gd from source has usually been a big pain on OSX for me, so it's nice that I can just ignore the setup warning. I don't think we need to work well when gd is missing, it just shouldn't be a super confusing fatal. The amount of code required is like:

// Just use psyduck if there's no `gd`.
if (!function_exists('imagecreatefromstring')) {
  return get_builtin_file('avatar.png');
}

...so it's not very tricky, I just keep mentioning it because it's super easy to forget about.

(And, if our behavior is "confusing fatal", that could prevent new installs from getting far enough to even see the setup warning saying "install gd".)

chad added a comment.Mar 1 2017, 5:13 PM

Yeah, I just want to find a way product side to always ensure good profile images are used. I'm fine having some fallback, if 100% needed, but it makes me really uncomfortable having different experiences or lessened experiences if avoidable.

chad added a comment.Mar 1 2017, 5:16 PM

Yeah I think having some bin/profileimage script for pre-generation would be good for admins like here so you can pre-run the migration separate from enabling.

chad added a comment.Mar 1 2017, 5:17 PM

Macro psyduck: RIP PSYDUCK NEVER FORGET

Psyduck is still used if the username starts with a non-alphanumeric character.

chad added a comment.Mar 1 2017, 7:31 PM
  • Generate the image in the Query, not when the user is created (helps with old users and not having to update a ton of different ways users can get created, and not generating these as side effects during unit tests).

PhabricatorPeopleQuery ? So generate it the first time it's called for use?

chad added a comment.Mar 1 2017, 10:49 PM

Yeah everywhere I look at doing this seems exceedingly hacky. Writing a profile image the first time PhabricatorUser->getDefaultPictureURI doesn't seem right, but I don't seen any other lazy way of doing this outside of user creation.

chad added a comment.Mar 3 2017, 6:04 PM

@epriestley not sure what I need to do next here on this or D17430

chad added a comment.Mar 3 2017, 6:16 PM

I have a bad version that makes a file the first time getDefaultPictureURI is called and one doesn't exist. But it uses unguarded writes and feels hacky,

Something like:

  • Add defaultProfileImagePHID, a nullable phid?, to PhabricatorUser.
  • Add defaultProfileImageVersion, a text64, to PhabricatorUser.

To test this stuff:

  • Add a new bin/people maybe? This doesn't feel like it fits very well on any of the existing things there.
  • Add bin/people avatar or similar, maybe with flags like --user X, --all, and --force.
    • Behavior is: if the user has an old default avatar, or --force is provided, rebuild it and save the version.
    • Otherwise, say "User already has an up-to-date avatar, declining to regenerate."
    • Then, installs can pre-generate things with bin/people avatar --all without doing extra work, and you can test changes to generation with bin/people avatar --user dog --force.
  • Generation is:
    • Create a new file.
    • Save the PHID as defaultProfileImagePHID.
    • Save some version as defaultProfileImageVersion, so we know what code generated the avatar and can automatically upgrade avatars if we tweak the algorithm later.
  • Test by running the command and making sure the database values get written sensibly.

Then:

  • In PhabricatorUserProfileImageCacheType->newValueForUsers(), have the code use getProfileImagePHID() first, then fall back to getDefaultProfileImagePHID() if that isn't available and the version is the current version.
  • If neither is available (or the default avatar is an old version) and the user is logged in, try to generate a profile image, save the PHID and a version on the user, then use the newly-generated PHID. This will need unguarded writes.
  • If that stuff fails or the user isn't a real user, fall back to the current default pathway.
chad raised the priority of this task from Wishlist to High.Mar 4 2017, 5:38 AM
chad removed the point value for this task.
chad added a comment.Mar 5 2017, 12:16 AM

is there a simple way to reset the cache?

I think bin/cache purge --purge-user will do it (but for every user). There's no way to do it for just one user, I don't think.

chad added a comment.EditedMar 5 2017, 12:22 AM

awwyiss

fancy ๐Ÿ‘‘

๐Ÿž ๐Ÿฆ

chad added a comment.Mar 5 2017, 4:29 PM

I presume these will pop in as cache expires?

I dropped the cache, looks like they're working.

chad added a subscriber: jmeador.Mar 5 2017, 5:05 PM

True story, I built these because of @jmeador

mavit added a subscriber: mavit.Mar 16 2017, 5:08 PM

This would be totally great, if our corporate username policy didn't mandate that all usernames start with the same letter ๐Ÿ˜‚

avivey added a comment.Apr 7 2017, 9:12 PM

Wasn't this supposed to show up as an option in "Edit Profile Picture"?

I think it does, but only if you ever don't have a profile picture:

If your account had a profile picture before we shipped this, it will never normally generate a default picture, so the selectable default in "Edit Profile Picture" will be the default-default, the psyduck.

I think you can use bin/people profileimage ... to force generation of a default for a set of users.