D13202: Circular user avatar for Kickoff

[Nathaniel Graham]

ngraham abandoned this revision.
ngraham added a comment.


  Let's continue in D13415 .

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: ngraham, hein, #plasma, sharvey
Cc: mart, apol, davidedmundson, plasma-devel, abetts, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, ahiemstra

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  I rewrote this patch using an `OpacityMask` and a second rounded `Rectangle` 
to draw the ring. It's a separate diff - D13415: Reshape Kickoff avatar to a 
circle with Opacity Mask .
  
  Please help me test and offer any suggestions.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  **#$%$@$!!!**
  
  I thought I had the blank circle problem fixed, but it appears I was wrong - 
must've done something incomplete in my testing.
  
  Will someone Plasma-related please have a look at this and see if you can 
offer any suggestions? I'm at a loss. All the icon/face files change when 
they're supposed to, but the [CENSORED] `ShaderEffect` circle still goes blank 
until plasmashell is restarted.
  
  Is there some way I can force a refresh of Kickoff?
  
  I'm at a loss on this and humbly request guru assistance.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  In D13202#275279 , @davidedmundson 
wrote:
  
  > Worth noting that some values were tweaked here because of the smaller 
circle size.
  
  
  For the record, the values that I tweaked were:
  
highp float blend = 0.03;// was 0.01
highp float innerRadius = 0.46;  // was 0.47
highp float outerRadius = 0.48;  // was 0.49
  
  It **was** trial and error, but the goal was to get a slightly thicker border 
for a smaller circle to smooth out the aliasing. Reusing this elsewhere may 
need different values. `blend + innerRadius` can't exceed `0.49` or the 
cropped-away corners become visible.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  @ngraham - This now appears to function properly with no blank circles.  Will 
you please confirm?
  
  If Nate is happy, will someone from #plasma 
 please approve so I can land it and 
get it filed under "finished"?
  
  Thanks.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35770.
sharvey added a comment.


  - Remove explicit `visible: true`, as default == `true`

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35648&id=35770

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[David Edmundson]

davidedmundson added a comment.


  As for this patch, IMHO it's good to go. If things do become libraries we can 
always migrate it.
  
  Worth noting that some values were tweaked here because of the smaller circle 
size.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[David Edmundson]

davidedmundson added a comment.


  > btw... i was thinking about copypasting such thing to have an avatar 
component in kirigami... would there be interest on this?
  
  Not sure, I think an Avatar component would be far too use-case specific for 
Kirigami.
  Having a round effect (like 
https://docs.ubuntu.com/phone/en/apps/api-qml-development/Ubuntu.Components.UbuntuShape)
 might make sense.

INLINE COMMENTS

> apol wrote in Header.qml:104
> I tried using fragment shader in discover and it ported poorly on different 
> hardware. Maybe OpacityMask or Rectangle + clip works too.

FWIW, in future use the new Plasma Renderer  KCM to set "Force GLC Core 
Profile" for testing.
It's a lot closer to the GSL you'd get on the different hardware.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[Marco Martin]

mart added a comment.


  In D13202#270416 , @davidedmundson 
wrote:
  
  > > The bordered avatars are done in a different manner than this (mine 
involves less math!). I'll see if there's a way I can sort it out.
  >
  > If you copy and paste the ShaderEffect from UserDelegate.qml   colorBorder 
is a property you can change
  
  
  btw... i was thinking about copypasting such thing to have an avatar 
component in kirigami... would there be interest on this?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[Marco Martin]

mart added a comment.


  In D13202#270391 , @ngraham wrote:
  
  > Nice work! Will test it out later today.
  >
  > There's no reason to have the appearance user-selectable, IMHO. Part of the 
goal here is a consistent visual appearance. I vote for always showing the 
round avatar and marking this as fixing Bug 386656.
  >
  > The one visual improvement I could think of is adding a thin light gray 
outline around the circle to help separate it from the background. This is done 
on the login and lock screens; might be able to mine those implementations for 
ideas, if it's not too hard. But even without that, this is looking good. :)
  
  
  yeah, shouldn't be configurable

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: mart, apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  @apol : My first pass at this used `OpacityMask`. It worked well. See below.
  
  In D13202#270387 , @sharvey wrote:
  
  > F5875550: Screenshot_20180529_113110.png 

  
  
  Two things, though... multiple people recommended I use Dave's shader routine 
from SDDM instead. Plus, with Opacity Mask, I don't think I can reproduce the 
much-desired "ring" effect around the image.
  
  Comments, folks?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added inline comments.

INLINE COMMENTS

> apol wrote in Header.qml:139
> No need to set it to true, just drop the line.

Stupid defaults. :D

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, mart

D13202: Circular user avatar for Kickoff

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> Header.qml:104
> +//but in SDDM that's a bit pointless
> +fragmentShader: "
> +varying highp vec2 qt_TexCoord0;

I tried using fragment shader in discover and it ported poorly on different 
hardware. Maybe OpacityMask or Rectangle + clip works too.

> sharvey wrote in Header.qml:139
> Was `faceIcon.status !== Image.Ready`, which I believe was an old timing test 
> when machines were slower. Doesn't seem necessary on modern hardware.

No need to set it to true, just drop the line.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: apol, davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35648.
sharvey added a comment.


  - Reset QtQuick version to 2.4

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35647&id=35648

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  Let's try this...

INLINE COMMENTS

> Header.qml:139
> +source: squareFace
> +visible: true
>  width: units.gridUnit * 3

Was `faceIcon.status !== Image.Ready`, which I believe was an old timing test 
when machines were slower. Doesn't seem necessary on modern hardware.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35647.
sharvey added a comment.


  - Force icon to be visible, avoiding blank circle

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35151&id=35647

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  @davidedmundson  - could use your input here, please. Your circle shader 
effect detects a user avatar change, partially - it goes blank. But it doesn't 
reload `squareFace` despite the fact that all three avatar locations 
(`~.face`,` ~.face.icon`, and `/var/lib/AccountServices/icon/$USER` all update 
immediately. Any idea why it doesn't refresh, or how to force it to refresh?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey marked 2 inline comments as done.
sharvey added a comment.


  I don't know if I need to edit `user-manager` to emit a signal of some kind, 
or if some trigger needs to go over the D-Bus... I'm wading into deep water 
here.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Nathaniel Graham]

ngraham added a comment.


  Yes, we had a report of this: https://bugs.kde.org/show_bug.cgi?id=384107
  
  However the issue I've seen is a genuine regression from your patch: changing 
my avatar from a file on disk to an image from gallery works without your 
patch, but fails with it.
  
  It looks like the stuff in here may be interacting poorly the pre-existing 
bug. I can tell you from submitting some user-manager patches to fix avatars in 
the past that the code there is sadly pretty fragile.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  F5878497: Screenshot_20180530_121625.png 

  It appears Kickoff had this problem prior to the circles. I changed my 
avatar, but un-patched Kickoff now shows the default avatar.
  
  I'm not clear why `user-manager` removes the previous avatar, but the new one 
isn't immediately available.
  
  I obviously need to force a refresh somewhere, somehow - but I don't know how.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  In D13202#270585 , @ngraham wrote:
  
  > Looks great now! Quite pleasing to the eye.
  >
  > I've got a few blockers though:
  >
  > - When I change the icon in the user-manager KCM and re-open Kickoff, the 
new icon isn't displayed; instead I just see a blank circle with no image in it 
until I quit and restart plasmashell.
  
  
  That’s odd. It shows us something *is* updating, just not correctly or 
completely. What’s strange is that it seems to be removing the old avatar but 
not yet updating with the new one.

INLINE COMMENTS

> ngraham wrote in Header.qml:19
> This version bump makes Kickoff require Qt 5.11. I wasn't are that we were 
> going to make Plasma 5.14 depend on Qt 5.11. At the minimum, it makes this 
> hard to test without compiling your own Qt. Is there anything we're actually 
> depending on with this patch that was added with 2.11, or can we reduce the 
> minimum version? I manually changed it to 2.10 and didn't notice any issues...

No, that version bump was likely from an earlier attempt using a different 
technique  I’ll get it dialed back to the correct minimum and will repost.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Looks great now! Quite pleasing to the eye.
  
  I've got a few blockers though:
  
  - When I change the icon in the user-manager KCM and re-open Kickoff, the new 
icon isn't displayed; instead I just see a blank circle with no image in it 
until I quit and restart plasmashell.
  - See the below comment:

INLINE COMMENTS

> Header.qml:19
>  
> -import QtQuick 2.0
> +import QtQuick 2.11
>  import org.kde.plasma.core 2.0 as PlasmaCore

This version bump makes Kickoff require Qt 5.11. I wasn't are that we were 
going to make Plasma 5.14 depend on Qt 5.11. At the minimum, it makes this hard 
to test without compiling your own Qt. Is there anything we're actually 
depending on with this patch that was added with 2.11, or can we reduce the 
minimum version? I manually changed it to 2.10 and didn't notice any issues...

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35151.
sharvey added a comment.


  - Whitespace once more

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35150&id=35151

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey marked 2 inline comments as done.
sharvey added a comment.


  F5876367: Screenshot_20180529_180924.png 

  Best yet, I think.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey marked 4 inline comments as done.
sharvey added inline comments.

INLINE COMMENTS

> davidedmundson wrote in Header.qml:76
> needed?

Not anymore. I thought your ShaderEffect just drew circles. I didn't catch on 
that it rounded off an image and then added a border. This probably helped the 
antialiasing.

> davidedmundson wrote in Header.qml:121
> decreasing the border width but increasing this might make it more rounded

A bit of trial and a bit of error... and I think we have a good result.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35150.
sharvey added a comment.


  - Remove redundant `OpacityMask`; tweaked antialiasing

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35139&id=35150

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> Header.qml:76
> +
> +   OpacityMask {
> +   anchors.fill: squareFace

needed?

> Header.qml:102
> +hideSource: true
> +live: false
> +}

because this is false, we need to force an update every time  squareFace 
changes

> Header.qml:113
> +
> +//if copying into another project don't forget to connect 
> themeChanged to update()
> +//but in SDDM that's a bit pointless

and when the theme changes

> Header.qml:121
> +uniform lowp vec4 colorBorder;
> +highp float blend = 0.01;
> +highp float innerRadius = 0.46;

decreasing the border width but increasing this might make it more rounded

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Andres Betts]

abetts added a comment.


  In D13202#270530 , @sharvey wrote:
  
  > In D13202#270529 , @abetts wrote:
  >
  > > It doesn't look any different to me. They are both the same. Maybe the 
screenshot is too small? I can see the pixels from far on both images.
  >
  >
  > Yeah, the same to me as well. The screenshots are natural size from my 
1920x1080 screen. I'll keep looking for options, features, settings, mystic 
spells...
  
  
  Thanks Scott, your work is very much appreciated!

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  In D13202#270529 , @abetts wrote:
  
  > It doesn't look any different to me. They are both the same. Maybe the 
screenshot is too small? I can see the pixels from far on both images.
  
  
  Yeah, the same to me as well. The screenshots are natural size from my 
1920x1080 screen. I'll keep looking for options, features, settings, mystic 
spells...

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Andres Betts]

abetts added a comment.


  In D13202#270528 , @sharvey wrote:
  
  > AA option not set
  >  F5875999: Screenshot_20180529_131810.png 

  >
  > AA option set
  >  F5876003: Screenshot_20180529_155151.png 

  >
  > Does the second option look dramatically better or different?
  
  
  It doesn't look any different to me. They are both the same. Maybe the 
screenshot is too small? I can see the pixels from far on both images.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  AA option not set
  F5875999: Screenshot_20180529_131810.png 

  
  AA option set
  F5876003: Screenshot_20180529_155151.png 

  
  Does the second option look dramatically better or different?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  In D13202#270496 , @ngraham wrote:
  
  > Cool! But in your latest screenshot, the circle is aliased pretty badly
  
  
  I noticed. Dave's code specifically says it'll be drawn antialiased. There's 
also an antialiasing property that you can explicitly set for the 
`ShaderEffect` element, but it didn't appear to make a difference whether on or 
off.
  
  Perhaps the author of said circles can offer some advice.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Nathaniel Graham]

ngraham added a comment.


  Cool! But in your latest screenshot, the circle is aliased pretty badly

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35139.
sharvey added a comment.


  - Whitespace (again...)

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35138&id=35139

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  I stuck with @davidedmundson 's use of `PlasmaCore.ColorScope.textColor` for 
the highlight ring. I tested it with a few different color themes and the whole 
thing comes out looking like a natural match. This shot is back to my personal 
choice of an openSUSE scheme.
  
  F5875763: Screenshot_20180529_131810.png 


REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  > If you copy and paste the ShaderEffect from UserDelegate.qml   colorBorder 
is a property you can change
  
  {icon thumbs-up} Very nice bit of code. Thanks.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35138.
sharvey added a comment.


  - Add theme-colored ring around avatar

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35132&id=35138

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[David Edmundson]

davidedmundson added a comment.


  > The bordered avatars are done in a different manner than this (mine 
involves less math!). I'll see if there's a way I can sort it out.
  
  If you copy and paste the ShaderEffect from UserDelegate.qml   colorBorder is 
a property you can change

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: davidedmundson, plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Andres Betts]

abetts added a comment.


  In D13202#270398 , @sharvey wrote:
  
  > > The one visual improvement I could think of is adding a thin light gray 
outline around the circle to help separate it from the background. This is done 
on the login and lock screens; might be able to mine those implementations for 
ideas, if it's not too hard. But even without that, this is looking good. :)
  >
  > The bordered avatars are done in a different manner than this (mine 
involves less math!). I'll see if there's a way I can sort it out.
  >
  > By the way, visual gurus: "light gray" or theme-specific color?
  
  
  If it is for a border, I would go with a semitransparent border using dark 
grey.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Nathaniel Graham]

ngraham added a comment.


  Yes, always a theme-specific color please! "Light gray" was just for 
illustration purposes. :)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  > The one visual improvement I could think of is adding a thin light gray 
outline around the circle to help separate it from the background. This is done 
on the login and lock screens; might be able to mine those implementations for 
ideas, if it's not too hard. But even without that, this is looking good. :)
  
  The bordered avatars are done in a different manner than this (mine involves 
less math!). I'll see if there's a way I can sort it out.
  
  By the way, visual gurus: "light gray" or theme-specific color?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Andres Betts]

abetts added a comment.


  In D13202#270391 , @ngraham wrote:
  
  > Nice work! Will test it out later today.
  >
  > There's no reason to have the appearance user-selectable, IMHO. Part of the 
goal here is a consistent visual appearance. I vote for always showing the 
round avatar and marking this as fixing Bug 386656.
  >
  > The one visual improvement I could think of is adding a thin light gray 
outline around the circle to help separate it from the background. This is done 
on the login and lock screens; might be able to mine those implementations for 
ideas, if it's not too hard. But even without that, this is looking good. :)
  
  
  Me too. +1

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, #plasma
Cc: plasma-devel, abetts, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Nathaniel Graham]

ngraham added a comment.


  Nice work! Will test it out later today.
  
  There's no reason to have the appearance user-selectable, IMHO. Part of the 
goal here is a consistent visual appearance. I vote for always showing the 
round avatar and marking this as fixing Bug 386656.
  
  The one visual improvement I could think of is adding a thin light gray 
outline around the circle to help separate it from the background. This is done 
on the login and lock screens; might be able to mine those implementations for 
ideas, if it's not too hard. But even without that, this is looking good. :)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, plasma-devel
Cc: abetts, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  I intentionally didn't tag Bug 386656 
, as it has some debate allowing 
the user to select between a round or square avatar. @ngraham voted for the 
circle, which is what I implemented. If we want it switchable, I'll add a 
toggle. If not, I'll add the `BUG:` tag.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, plasma-devel
Cc: abetts, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Andres Betts]

abetts added a comment.


  +1

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, plasma-devel
Cc: abetts, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey added a comment.


  F5875550: Screenshot_20180529_113110.png 


REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13202

To: sharvey, ngraham, hein, plasma-devel
Cc: ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey updated this revision to Diff 35132.
sharvey added a comment.


  - Remove whitespace

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13202?vs=35131&id=35132

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, plasma-devel
Cc: ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D13202: Circular user avatar for Kickoff

[Scott Harvey]

sharvey created this revision.
sharvey added reviewers: ngraham, hein, plasma-devel.
Restricted Application added a project: Plasma.
sharvey requested review of this revision.

REVISION SUMMARY
  Apply circular `OpacictyMask` to crop square user avatar into a circle

TEST PLAN
  - Download & apply patch
- Recompile `plasma-desktop` (or at least applets)
- Add a new Kickoff widget ("Application Launcher")
- Check for circular avatar (logout/login may be necessary)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  round-kickoff-avatar (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D13202

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: sharvey, ngraham, hein, plasma-devel
Cc: ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart