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

Make AvatarRenderedSizeFactor configurable and set it to 3 #17951

Merged
merged 2 commits into from
Dec 16, 2021

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented 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: #17422
Fixes: #16287

@silverwind silverwind added the performance/speed performance issues with slow downs label Dec 10, 2021
@silverwind silverwind added this to the 1.16.0 milestone Dec 10, 2021
@silverwind
Copy link
Member Author

For reference, the devices with pixel ratio > 3 from the link above:

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 10, 2021
@zeripath
Copy link
Contributor

zeripath commented Dec 15, 2021

@silverwind silverwind#1 will make this a configurable option which should make @strk happy

@strk
Copy link
Member

strk commented Dec 15, 2021

Yes I'm happy with configurable, and merged your PR to mine (#17422)

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 15, 2021
@silverwind
Copy link
Member Author

Merged and did some minor description adjustments. Default is 3 here and I think this is the ideal value.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 16, 2021
@silverwind silverwind changed the title Reduce AvatarRenderedSizeFactor to 3 Make AvatarRenderedSizeFactor configurable and set it to 3 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
@silverwind
Copy link
Member Author

PR title and commit description updated.

@lunny
Copy link
Member

lunny commented Dec 16, 2021

CI failed is unrelated.

@lunny lunny merged commit cc129d2 into go-gitea:main Dec 16, 2021
@silverwind silverwind deleted the avatar-factor-3 branch December 16, 2021 08:11
Chianina pushed a commit to Chianina/gitea that referenced this pull request 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 <[email protected]>
@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
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. performance/speed performance issues with slow downs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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