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.

Sign up for GitHub

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

Jump to bottom

Fixed issues with old GL versions ( <= 2.1 ) #1518

Merged
merged 1 commit into from
Nov 8, 2022
Merged

Fixed issues with old GL versions ( <= 2.1 ) #1518

merged 1 commit into from
Nov 8, 2022

Conversation

bungoboingo
Copy link
Contributor

@bungoboingo bungoboingo commented Nov 8, 2022 •
edited
Loading

Fixes an issue with older GL versions <= 2.1 with new shader changes.

@bungoboingo bungoboingo marked this pull request as ready for review November 8, 2022 19:48
derezzedex
derezzedex suggested changes Nov 8, 2022
View reviewed changes
Copy link
Member

@derezzedex derezzedex left a comment

Choose a reason for hiding this comment

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

I would change the spaces, they have cause me trouble on the past.

Also for reference, the gl_FragColor was deprecated on GLSL 1.30 (which "starts on" GL 3.0 and GLES 3.0), which means that on shaders that are higher than this (thus the definition of HIGHER_THAN_300), when using the core profile, we should use the shader output variables out.

glow/src/shader/common/triangle.frag Outdated Show resolved Hide resolved
glow/src/shader/common/gradient.frag
@@ -14,7 +14,7 @@
in vec2 raw_position;

uniform vec4 gradient_direction;
uniform uint color_stops_size;
uniform int color_stops_size;
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uint being supported in GLSL 1.50 but the glUniform1u not existing in GL 2.0 brings me great pain

@ids1024
Copy link
Contributor

ids1024 commented Nov 8, 2022

What is the best way to test things with an old OpenGL version? I know when I've tried in the past it's been problematic since the driver just gave a context for a newer (compatible) version.

If possible it would be good to have some automated CI testing of this. It's something that breaks easily if changes aren't being tested with the older version.

@bungoboingo
Copy link
Contributor Author

bungoboingo commented Nov 8, 2022 •
edited
Loading

My current (new) strategy for testing OpenGL versions is to use Mesa & export MESA_GL_VERSION_OVERRIDE="x.x" to test downgrades. Same with MESA_GLES_VERSION_OVERRIDE="x.x". It's not ideal for sure. If Mesa has a way to disable GL version to test with only ES we could potentially automate it with CI to make sure everything runs at least, if not would be difficult as the only way to force ES is to set settings try_opengles_first: true in application settings. If we moved try_opengles_first setting to a feature flag or something it would be possible.

hecrj
hecrj approved these changes Nov 8, 2022
View reviewed changes
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the quick fix 🥳

@hecrj hecrj merged commit 8102f96 into iced-rs:master Nov 8, 2022
@bungoboingo bungoboingo deleted the fix/old_gl branch November 8, 2022 22:25
@hecrj hecrj mentioned this pull request Nov 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derezzedex derezzedex derezzedex requested changes

@hecrj hecrj hecrj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bungoboingo @ids1024 @hecrj @derezzedex