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

Switch mobile shader to use default mediump precision. #99767

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DarioSamo
Copy link
Contributor

@DarioSamo DarioSamo commented Nov 27, 2024

For now this is a pretty basic PR towards our goal of switching mobile to FP16. I'll be looking at the output in the mobile vendor shader compilers more in detail to see where we can edge out some gains.

Even as it is, without showing much in the way of visual regressions, this PR gives a small performance improvement (2 to 3%) with a very straightforward change.

Will keep this as a draft until I dive deeper into the compiled output and tweak the shader.


Contributed by W4 Games. 🍀

@Calinou
Copy link
Member

Calinou commented Nov 28, 2024

Should we have a project setting to force highp in core shaders? We had this in GLES2 in 3.x, which was occasionally used to fix rendering issues in projects running on mobile.

At the same time, it's a band-aid fix that negates the performance improvements from using mediump/lowp in the appropriate locations, so maybe it's better to start from a clean slate.

Also, does this PR affect shaders written by users, either GDShader or GLSL? See also #59990.

@DarioSamo
Copy link
Contributor Author

DarioSamo commented Nov 28, 2024

Also, does this PR affect shaders written by users, either GDShader or GLSL? See also #59990.

All user variables have the highp prefix to avoid issues in user code (if not specified).

@clayjohn clayjohn modified the milestones: 4.x, 4.4 Nov 29, 2024
@akien-mga akien-mga modified the milestones: 4.4, 4.x Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants