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

User avatar in profile page changed from s=290 to s=580 while rendered at s=128 anyway #16287

Closed
strk opened this issue Jun 28, 2021 · 8 comments · Fixed by #17951
Closed

User avatar in profile page changed from s=290 to s=580 while rendered at s=128 anyway #16287

strk opened this issue Jun 28, 2021 · 8 comments · Fixed by #17951
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@strk
Copy link
Member

strk commented Jun 28, 2021

Gitea 1.14.3 is requesting Libravatar avatar images with size 580 in profile page while rendering them at 128x128.
Older versino 1.12.3 was requesting a size of 290 instead.

I think it would make sense to request s=128 instead, to reduce the download time

@zeripath
Copy link
Contributor

See #15453

@strk
Copy link
Member Author

strk commented Jun 29, 2021

That ticket shows that passing a size is what changed, but doesn't explain why we're passing a size of 580 when rendering the image at 128, is that an UI issue which went unnoticed before ?

@strk
Copy link
Member Author

strk commented Oct 18, 2021

It got worst, now the avatar image is requested at size 1160 !
See https://try.gitea.io/strk

I noted that models/avatars/avatar.go has a const AvatarRenderedSizeFactor = 4 which would then match the older version size of 290 (290 x 4 == 1160).

@strk
Copy link
Member Author

strk commented Oct 18, 2021

290 constant is still present in modules/avatar/avatar.go:

// AvatarSize returns avatar's size
const AvatarSize = 290

What was the AvatarRenderedSizeFactor introduced for ?

It looks like be745be doubled it from a factor of 2 to a factor of 4 to get better avatars for Hi-DPI displays but, shouldn't all of this be decided by the frontend code and based on actual displays ?

\cc @techknowlogick @zeripath ref #15941

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Oct 20, 2021
@wxiaoguang
Copy link
Contributor

It got worst, now the avatar image is requested at size 1160 ! See https://try.gitea.io/strk

I noted that models/avatars/avatar.go has a const AvatarRenderedSizeFactor = 4 which would then match the older version size of 290 (290 x 4 == 1160).

Although the requested size looks like 1160, but the real responded image is not that big, it isn't a big problem now.

But, I believe we should make the avatar size problem correct.

@strk
Copy link
Member Author

strk commented Oct 24, 2021

Although the requested size looks like 1160, but the real responded image is not that big, it isn't a big problem now.

The image is not big because my avatar service is being nice and refusing to serve such a big image, but what Gitea requests should match what it actually will want to render on the screen. The generated HTML looks like this:

<img class="ui avatar image" src="/avatar/fe2a9e759730ee64c44bf8901bf4ccc3?size=1160" title="strk" width="290" height="290">

Doesn't make sense, right ? It will render at 290x290 no matter what the /avatar link will return, but it asks the /avatar link for a 1160x1160 image. Doesn't make sense to me

@strk
Copy link
Member Author

strk commented Oct 24, 2021

The culprit seems to be the Avatar helper in modules/templates/helper.go which applies the RenderedSizeFactor to the size requested by the template

silverwind added a commit to silverwind/gitea that referenced this issue Dec 10, 2021
Save a bit of bandwidth by only requesting 3-times the rendered avatar
size. Factor 4 is only really beneficial on a handful of mobile phones
and I don't think they are the primary device we design for.

Fixes: go-gitea#17422
Fixes: go-gitea#16287
zeripath pushed a commit to zeripath/gitea that referenced this issue Dec 15, 2021
silverwind added a commit to silverwind/gitea that referenced this issue Dec 16, 2021
Save a bit of bandwidth by only requesting 3-times the rendered avatar
size. Factor 4 is only really beneficial on a handful of mobile phones
and I don't think they are the primary device we design for.

Configurability contributed by zeripath.

Fixes: go-gitea#17422
Fixes: go-gitea#16287
lunny pushed a commit that referenced this issue Dec 16, 2021
Save a bit of bandwidth by only requesting 3-times the rendered avatar
size. Factor 4 is only really beneficial on a handful of mobile phones
and I don't think they are the primary device we design for.

Configurability contributed by zeripath.

Fixes: #17422
Fixes: #16287

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@strk
Copy link
Member Author

strk commented Dec 16, 2021

Unfortunately the default size factor of 3 still breaks my avatar: https://try.gitea.io/strk (is requested at 870x870
I still think this should be based on actual user monitor rather than expecting a "one size fits all" as it is now.

Chianina pushed a commit to Chianina/gitea that referenced this issue Mar 28, 2022
…17951)

Save a bit of bandwidth by only requesting 3-times the rendered avatar
size. Factor 4 is only really beneficial on a handful of mobile phones
and I don't think they are the primary device we design for.

Configurability contributed by zeripath.

Fixes: go-gitea#17422
Fixes: go-gitea#16287

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
4 participants