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

Disable manual mipmap generation (and decrease atlas size again) #5765

Merged
merged 3 commits into from
May 1, 2023

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Apr 29, 2023

As per today's discussion in discord, this disables the meaningful changes made in #5508 to restore operation on configurations that have been negatively affected by the change (primarily appears to be mobile).

This is not a direct revert, and the goal was to keep the mipmap generation code in the tree, just in a disabled state, to permit revisiting the issue at a later date if necessary.

#5757 is a possible avenue for improvement in this stage, but it is currently not ready and @frenzibyte has no throughput to address the issues therein.

bdach added 2 commits April 29, 2023 15:37
With the custom mipmap generation logic disabled, the maximum texture
atlas size needs to be decreased to 1024 again to mitigate
`glMipmap()`'s bad performance characteristics.
@bdach bdach added area:graphics next-release Pull requests which are almost there labels Apr 29, 2023
@bdach bdach requested a review from a team April 29, 2023 15:33
@bdach bdach self-assigned this Apr 29, 2023
@bdach
Copy link
Collaborator Author

bdach commented Apr 29, 2023

Before/after screenshots from devices I own:

last release this PR
Samsung J7 (2016) Screenshot_20230429-161357 Screenshot_20230429-171122
OnePlus 8T Screenshot_2023-04-29-16-09-39-72_25a1a32208bbcdc1d450b7aa854bb161 Screenshot_2023-04-29-17-00-10-32_25a1a32208bbcdc1d450b7aa854bb161

Change is most apparent on the toolbar icons.

@Joehuu
Copy link
Member

Joehuu commented Apr 29, 2023

Can confirm on my Pixel 6a that ppy/osu#23240 and ppy/osu#23318 are fixed.

@peppy peppy self-requested a review May 1, 2023 06:11
@peppy peppy enabled auto-merge May 1, 2023 06:17
@peppy peppy merged commit 7da56ae into ppy:master May 1, 2023
@bdach bdach deleted the disable-manual-mipmap-generation branch May 1, 2023 07:02
@Shirou27
Copy link

That means the stuttering will come back? With high atlas size seems reduced the stuttering on my Redmi Note 9 phone.

For me, I don't mind having text weirdness in expense for performance or less stuttering.

@peppy
Copy link
Member

peppy commented May 11, 2023

That's correct. Please read the OP.

@Shirou27
Copy link

Shirou27 commented May 11, 2023

I'm sorry but I don't know what is "OP" you're referring.

But I'm not gonna install the next updates for now until it come back or something replaced it that can reduce the stuttering while playing. What important to me is performance as it affects the whole game for me.

If I just have money, I would buy a high-end android phone.

@bdach
Copy link
Collaborator Author

bdach commented May 11, 2023

@Shirou27 OP = opening post

This revert is a temporary measure. We're hoping to get these changes back in again as they do end up improving performance on a wide range of devices, but not at the expense of the game looking as bad as it does on some devices in the previous state. You're of course well within your rights to skip installing updates until then. Thanks for your understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:graphics next-release Pull requests which are almost there size/XL
Projects
Archived in project
4 participants