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

Autodetect GLES version #9655

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Autodetect GLES version #9655

wants to merge 2 commits into from

Conversation

System64fumo
Copy link
Contributor

Describe your PR, what does it fix/add?

This should automatically use the highest available version of GLES.
Should be fine for GLES 3.x es since nothing major changed between the versions, IE it should be compatible.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Haven't tested screen shaders or cm since i don't have one available on hand but should be all good according to logs.
I feel like this could be done better, LMK if there's any changes needed.

Is it ready for merging, or does it need work?

Ready to be merged.

Final words:
Vaxry if you're going to do Embedded System stuff at least support it properly.
As a man who plays with this stuff daily i feel like i have a say on this.

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a horrible idea, don't do this.

@System64fumo
Copy link
Contributor Author

I know that this is a terrible idea.. But at least this would increase compatibility.
There aren't any major changes between gl 3.x es versions afaik so this should be safe to do, Plus should anything go wrong it would just get disabled and complain in the logs.

Any particular reason you're against this? (Other than unpredictability)
gl 2.0 es is unaffected by this change btw.

@vaxerski
Copy link
Member

this is just wrong, and there is no reason to put version to 320 if the shader does well at 300. Instead, just make it 300 if we don't use any 310 / 320 features.

@System64fumo
Copy link
Contributor Author

You could do that, But PR guidelines state "no hacks" imo that's not a solution but a hack
It would also negate the minor performance improvements that newer version of gles offer, Not that it makes a huge difference but if it's possible to use whatever your card is compatible with then why not?

@System64fumo
Copy link
Contributor Author

That being said i've just tested these changes with some shaders on a few of my hardware and it works on all of them so i'm gonna say this works afaik.

Tested on:
Samsung Galaxy E5
OpenGL ES: 3.0

Orange Pi 5 Plus
OpenGL ES: 3.1

OnePlus 6T
OpenGL ES: 3.2

So as far as i'm aware shaders and color management should work fine everywhere.

@UjinT34
Copy link
Contributor

UjinT34 commented Mar 18, 2025

Does it solve any issue caused by "incorrect" version? "Works fine" or "we hope it'll work fine" doesn't mean it'll work better.
I am not saying that we shouldn't change this stuff. We should have something measurable or comparable to make a decision.

@System64fumo
Copy link
Contributor Author

Does it solve any issue caused by "incorrect" version?

Yes it allows the use of shaders on non 3.2 capable machines

"Works fine" or "we hope it'll work fine" doesn't mean it'll work better.

I mean so far this got rid of that annoying warning, Prevents compilation errors, And allows for shader usage which i guess is better than not having that.

We should have something measurable or comparable to make a decision.

As stated above, I've tested this on a few devices that have different versions of supported GLES.
Everything worked fine there.
And should continue working in the future as there really aren't that many differences between GLES versions,
It mostly comes down to hardware support, Optimizations, And extensions.

If you were talking about the performance hit then yeah sure idrc about forcing 3.0 if 3.2 is supported.
But this is the better approach to this problem, Up to vaxry to decide.

@leiserfg
Copy link
Contributor

Isn't this the same as setting #version 300 to all 3** shaders? I don't think the version of the glsl will change performance, the driver is the same.

@System64fumo
Copy link
Contributor Author

Fine, Screw it.
Lazy hack it is!

System64fumo and others added 2 commits March 19, 2025 18:58
Use GLES 3.0 for compatibility reasons.
@System64fumo
Copy link
Contributor Author

I went to buy milk but now i'm back with the commit.
This now uses GLES 3.0 instead of auto detecting.

Dear @vaxerski have a look will ya?

@System64fumo System64fumo reopened this Mar 19, 2025
@UjinT34 UjinT34 mentioned this pull request Mar 19, 2025
5 tasks
@vaxerski
Copy link
Member

this is still wrong. You can't change the gles version we run, as we offer 320 compat for screen shaders.

@UjinT34
Copy link
Contributor

UjinT34 commented Mar 19, 2025

Seems to work fine with 300 es #9600. 320 was picked because it was already used for custom screen shaders.

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.

4 participants