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

Added round function to gles2 #30895

Merged
merged 1 commit into from
Jul 29, 2019

Conversation

clayjohn
Copy link
Member

Fixes: #24850

Implements the round() function in GLES2. Something similar can be done with a few of the other GLES3-only features found in http://docs.godotengine.org/en/latest/tutorials/misc/gles2_gles3_differences.html#shading-language

Putting this up first to see if this is the implementation we want to follow. Then I will make a PR with a few others.

Implementation Notes:

Right now, I have copied the implementation from the mix(a, b, bool_type) override. The shader compiler replaces calls to round() with calls to roundx(), which are defined in stdlib.glsl. This implementation works and is safe to use. However it may be unnecessary.

An alternative would be to just provide an overloaded round() function in stdlib.glsl without changing anything in the compiler. This seems to work, but some GLES2 devices that deviate from spec allow users to use round(), so I'm worried that it would crash on those devices. That being said, it is a much more elegant solution.

@clayjohn clayjohn requested a review from karroffel July 28, 2019 18:14
@clayjohn clayjohn requested a review from reduz as a code owner July 28, 2019 18:14
@clayjohn clayjohn force-pushed the gles2-shader-funcs branch from f706633 to 0c4ddab Compare July 29, 2019 00:59
@akien-mga akien-mga added this to the 3.2 milestone Jul 29, 2019
@reduz
Copy link
Member

reduz commented Jul 29, 2019

My only fear with adding more code to the base shaders is that versions will take longer to compile because more needs to be parsed. Maybe having some ifdef around these being used may work?

@clayjohn clayjohn force-pushed the gles2-shader-funcs branch from 0c4ddab to 103eab0 Compare July 29, 2019 17:57
@clayjohn clayjohn force-pushed the gles2-shader-funcs branch from 103eab0 to 3f25dde Compare July 29, 2019 18:17
@akien-mga akien-mga merged commit b697121 into godotengine:master Jul 29, 2019
@akien-mga
Copy link
Member

Thanks!

@@ -917,6 +928,7 @@ ShaderCompilerGLES2::ShaderCompilerGLES2() {
actions[VS::SHADER_CANVAS_ITEM].usage_defines["NORMAL"] = "#define NORMAL_USED\n";
actions[VS::SHADER_CANVAS_ITEM].usage_defines["NORMALMAP"] = "#define NORMALMAP_USED\n";
actions[VS::SHADER_CANVAS_ITEM].usage_defines["LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";
actions[VS::SHADER_CANVAS_ITEM].usage_defines["round"] = "#define ROUND_USED\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

One question about it, why don't add usage_defines for spatial shader here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah. Good catch. I added just for CanvasItem while testing and then forgot to add to Spatial before comitting.

Copy link
Member

Choose a reason for hiding this comment

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

And for Particles too ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only for GLES2. So no particle shaders. ;p

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.

3.1 OpenGL ES2.0 using round() causes shaders to not compile
5 participants