Shader system improvements and fixes. #2577

EliteMasterEric · 2022-07-29T23:33:31Z

Provides the following changes:

Store the original values for the fragment/vertex header such that they can be accessed by child classes.
Store the original values for the fragment/vertex body such that they can be accessed by child classes.
Store the original values for the fragment/vertex source such that they can be accessed by child classes.
Added a configurable glVersion annotation (and corresponding variables) which can be used to modify the GLSL version directive. The default value has been set to 120, which should resolve an issue on some platforms where the uniform type is unavailable.
Added checks to __processGLData to ensure a variable exists before it is assigned via Reflection.
- This resolves an issue caused by custom implementation of variables in child classes.
- Added caching to the field check to reduce calls to Reflect.fields, improving performance.

These changes were performed to support fixes for FlxRuntimeShader, as part of the pull request HaxeFlixel/flixel-addons#368. Making changes here prevents code duplication in FlxRuntimeShader, improving maintainability.

These changes have been tested on the Windows HXCPP platform.

Implemented using WebAudio API targeting HTML5 and OpenAL targeting Windows & Neko

…elop

JonnycatMeow

PLEASE MERGE THIS!!!

loudoweb · 2022-08-14T09:15:21Z

Interesting. I was looking for a feature to change a CONST value at runtime. I think storing fragment body and header would be easier for me than parsing, diassembling an reassembling the whole shader. Some shaders need to use CONST in a for loop because non-constant values are forbidden, but different values can provide a better accuracy depending on what has been set in the uniform (e.g: Glow, Outline...). So it's looking great.

That being said, if you have only tested on Windows. This need to be tested in HL, MAC, Android, iOS and HTML5 before being merged.

Also can you provide details on your issue on "some platforms", which platforms are your talking about?

JonnycatMeow · 2022-08-14T12:42:47Z

i tested it on mac and i can confirm it works on mac too

MAJigsaw77 · 2022-10-11T10:23:22Z

@mastereric This pull request makes android to crash, I think is because of glVersion you added, gl version is different in android btw

JonnycatMeow · 2022-10-11T17:30:57Z

@mastereric This pull request makes android to crash, I think is because of glVersion you added, gl version is different in android btw

what glversion should be put to use for android?

MAJigsaw77 · 2022-10-12T03:51:35Z

Idk the exact version

MAJigsaw77 · 2022-10-12T03:51:59Z

You have to find out, is your pull request anyway

JonnycatMeow · 2022-10-12T05:44:44Z

the glsl version for android is #version 300 es how i found out is because of the flxFixedShader https://github.com/YoshiCrafter29/YoshiCrafterEngine/blob/45ddea338a211ff2d05b7d85a3a2c38bb8ab287f/source/FlxFixedShader.hx#L55

MAJigsaw77 · 2022-10-12T12:40:52Z

That version is wrong

…ssion' into develop

EliteMasterEric · 2022-12-01T23:26:24Z

Took a bit of time today to fast forward the branch, as well as implement a couple fixes.

Mainly, glVersion is now a string, and the value defaults to 300 es on Android as suggested.

Hopefully this works for people, I've been having reports that AMD Radeon cards are having issues specifically.

MAJigsaw77 · 2022-12-02T05:23:01Z

Dude, the default version by openfl is 100

MAJigsaw77 · 2022-12-02T05:23:22Z

If you put 300 es it will broke everything

JonnycatMeow · 2022-12-03T06:11:20Z

i get an error trying to build an fnf game for mac and here is the error:

JonnycatMeow · 2022-12-03T06:16:22Z

actually nvm i fixed it

JonnycatMeow · 2022-12-03T06:19:03Z

src/openfl/utils/_internal/ShaderMacro.hx

+ {
+ // Specify the default glVersion.
+ // We can use compile defines to guess the value that prevents crashes in the majority of cases.
+ return #if (android) "100" #elseif (web) "100" #else "100" #end;


added another #elseif for mac and make it 120 because that the only version that is supported and then put else 100

make sure to do it for everyone of the code that says 100 even the extension

it will look like this

Wrapping the 100 makes no sense, if you force 100 on a platform that doesn't support it you should obviously get an error.

Thanks for letting me know the correct default version for Mac though.

EliteMasterEric · 2022-12-04T02:48:17Z

This PR is now ready and fully tested on a variety of platforms.

See more at my test workbench here: https://github.com/MasterEric/Flixel-GalaxyShaderWorkbench

EDIT: Make sure to use the build instructions to ensure the correct library versions are installed before testing.

EliteMasterEric · 2022-12-04T17:40:48Z

With help from @JonnycatMeow I was able to test the workbench on their Mac platform. Notably, their version of OpenGL ONLY supports 110 and 120. This may be significant for those looking to resolve compatibility issues.

JonnycatMeow · 2022-12-07T17:45:48Z

bro can someone merge this please!!!!

Fast forward

Raltyro · 2023-02-26T07:52:04Z

The Sound modification seems pretty handy for streaming
although is it on purpose that the samplerate is locked at 44100?

EliteMasterEric · 2023-02-27T03:49:13Z

The Sound modification seems pretty handy for streaming although is it on purpose that the samplerate is locked at 44100?

This pull request is not related to sound.

joshtynjala · 2023-02-27T16:49:03Z

This pull request is not related to sound.

Look on the Commits tab of this PR. You merged in a SampleDataEvent commit, and maybe some other stuff. I mentioned this to you on Discord, but you never responded.

EliteMasterEric · 2023-02-27T17:02:59Z

This pull request is not related to sound.

Look on the Commits tab of this PR. You merged in a SampleDataEvent commit, and maybe some other stuff. I mentioned this to you on Discord, but you never responded.

Thank you for informing me of this. I will re-review my branch, make appropriate fixes, and merge them ASAP so that review can continue.

EliteMasterEric · 2023-02-27T17:48:57Z

It appears that changes related to other pull requests (#2534 and #2515) accidentally got included in the working branch for this PR. Those changes have now been removed, and the PR has additionally been fast-forwarded to match the target branch.

Please inform me here of any additional review changes.

Geokureli · 2023-04-27T21:33:33Z

src/openfl/utils/_internal/ShaderMacro.hx

+ default:
+ return glExtensions;
+
+ case "100":
+ return glExtensions;
+ case "110":
+ return glExtensions;
+ case "120":
+ return glExtensions;
+ case "130":
+ return glExtensions;
+ case "140":
+ return glExtensions;
+ case "150":
+ return glExtensions;
+
+ case "300 es":
+ var hasSeparateShaderObjects = false;
+ for (extension in glExtensions)
+ {
+ if (extension.name == "GL_ARB_separate_shader_objects") hasSeparateShaderObjects = true;
+ if (extension.name == "GL_EXT_separate_shader_objects") hasSeparateShaderObjects = true;
+ }
+
+ #if desktop
+ if (!hasSeparateShaderObjects)
+ {
+ #if linux
+ return glExtensions.concat([{name: "GL_EXT_separate_shader_objects", behavior: "require"}]);
+ #else
+ return glExtensions.concat([{name: "GL_ARB_separate_shader_objects", behavior: "require"}]);
+ #end
+ }
+ #end
+ return glExtensions;
+
+ case "310 es":
+ var result = buildGLSLExtensions(glExtensions, "300 es", isFragment);
+ return result;
+
+ case "320 es":
+ var result = buildGLSLExtensions(glExtensions, "310 es", isFragment);
+ return result;
+
+ case "330":
+ var result = buildGLSLExtensions(glExtensions, "320 es", isFragment);
+ return result;
+
+ case "400":
+ return glExtensions;
+ case "410":
+ return glExtensions;
+ case "420":
+ return glExtensions;
+ case "430":
+ return glExtensions;
+ case "440":
+ return glExtensions;
+ case "450":
+ return glExtensions;
+ case "460":
+ return glExtensions;


I don't get why this switch seems so needlessly convoluted.

Why have a bunch of cases that behave exactly like the default case?

why not group similar results using case "400" | "410":?

why "310 es" pass "300 es" into buildGLSLExtensions and so on for "320 es" and "330"?

why store a result var just to return it

src/openfl/utils/_internal/ShaderMacro.hx

T1mL3arn · 2023-12-16T03:58:09Z

src/openfl/utils/_internal/ShaderMacro.hx

+ // We don't know this GLSL version, so don't do anything.
+ return source;


Maybe show an explicit warning like "OpenFl doesn't know glsl version ${glVersion}, no conversion will happen" ?

So then how do we use a certain glsl version in the project.xml or could it only be used in .hx code?

You don't specify GLSL version in the project.xml you specify it when you construct the shader

oh ok so then the default whould be 120 for desktops and for web it whould be 300?

oh ok so then the default whould be 120 for desktops and for web it whould be 300?

The default would be the result of getDefaultGLSLVersion

then how whould i use a certain glsl version?

then how whould i use a certain glsl version?

You can use @:glVersion("120") the same way you use @:glFragmentBody("...").

As a note there is an FlxRuntimeShader in flixel-addons. It was originally built by me and ran on this, but it was modified to not use this code so it could be merged. When this is merged it will get changed back and then it will support setting the glVersion among other things.

Raltyro · 2023-12-17T10:51:27Z

The Sound modification seems pretty handy for streaming
although is it on purpose that the samplerate is locked at 44100?

When did i said this what

EliteMasterEric · 2023-12-17T20:56:44Z

The Sound modification seems pretty handy for streaming
although is it on purpose that the samplerate is locked at 44100?

When did i said this what

I accidentally included another PR in this one by mistake.

JonnycatMeow · 2024-02-21T08:42:04Z

there should be core and compatibility support for macOS
https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.1.50.pdf
this shader version only uses core for macOS because compatibility craps its self

JonnycatMeow · 2024-02-21T08:46:27Z

JonnycatMeow · 2024-02-21T08:46:40Z

JonnycatMeow · 2024-02-21T09:19:25Z

JonnycatMeow · 2024-02-21T09:19:35Z

My source https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.1.50.pdf

scanline and others added 7 commits September 24, 2017 04:14

SampleDataEvent

2779f9e

Implemented using WebAudio API targeting HTML5 and OpenAL targeting Windows & Neko

Merge branch 'develop' of https://github.com/scanline/openfl into dev…

2af76c9

…elop

cleanup

63fb5f7

Add public functions for retrieving lists of cached assets.

ae34ce1

Added individual functions for clearing Bitmaps, Fonts, and Sounds.

c0c16d4

Add the methods to the interface as well.

c6718fa

Shader system improvements and fixes.

9f7b6e2

EliteMasterEric mentioned this pull request Jul 29, 2022

Added FlxRuntimeShader HaxeFlixel/flixel-addons#368

Closed

JonnycatMeow approved these changes Aug 5, 2022

View reviewed changes

EliteMasterEric added 3 commits August 5, 2022 20:25

Merge remote-tracking branch 'openfl/develop' into develop

acd50c1

Merge remote-tracking branch 'origin/bugfix/shader-fixes' into develop

37c7870

Merge remote-tracking branch 'xsam/develop' into develop

fa59359

JonnycatMeow mentioned this pull request Aug 10, 2022

Fixed the lag issue HaxeFlixel/flixel#2620

Closed

Raltyro approved these changes Aug 14, 2022

View reviewed changes

giuppe and others added 5 commits October 12, 2022 22:57

fixed UncaughtErrorEvents not working (openfl#2574)

9ff76cf

Merge remote-tracking branch 'giuppe/issue-2574-uncaught-errors-regre…

31eb4b6

…ssion' into develop

Merge remote-tracking branch 'openfl/develop' into develop

68d1f0c

Merge branch 'develop' into bugfix/shader-fixes

e80cc64

Made glVersion a String, changed version for Android.

29705c7

JonnycatMeow reviewed Dec 3, 2022

View reviewed changes

EliteMasterEric added 2 commits December 3, 2022 21:12

Further Linux fixes

21332bd

Added default version for MacOS

3fd5763

EliteMasterEric marked this pull request as ready for review December 4, 2022 02:47

EliteMasterEric added 2 commits February 1, 2023 16:29

Merge pull request #1 from openfl/develop

687688f

Fast forward

Merge branch 'develop' into bugfix/shader-fixes

1689bfe

EliteMasterEric added 3 commits February 27, 2023 12:22

Merge remote-tracking branch 'openfl/develop' into bugfix/shader-fixes

39ec254

Cleanup to remove unrelated changes.

1d48bcd

Cleanup to remove unrelated changes 2

98fbfc9

Geokureli reviewed Apr 27, 2023

View reviewed changes

src/openfl/utils/_internal/ShaderMacro.hx Outdated Show resolved Hide resolved

Additional fixes and reability refactor

536a985

T1mL3arn reviewed Dec 16, 2023

View reviewed changes

		// We don't know this GLSL version, so don't do anything.
		return source;

Shader system improvements and fixes. #2577

Are you sure you want to change the base?

Shader system improvements and fixes. #2577

Conversation

EliteMasterEric commented Jul 29, 2022

JonnycatMeow left a comment

Choose a reason for hiding this comment

loudoweb commented Aug 14, 2022 • edited

JonnycatMeow commented Aug 14, 2022

MAJigsaw77 commented Oct 11, 2022

JonnycatMeow commented Oct 11, 2022

MAJigsaw77 commented Oct 12, 2022

MAJigsaw77 commented Oct 12, 2022

JonnycatMeow commented Oct 12, 2022

MAJigsaw77 commented Oct 12, 2022

EliteMasterEric commented Dec 1, 2022

MAJigsaw77 commented Dec 2, 2022

MAJigsaw77 commented Dec 2, 2022

JonnycatMeow commented Dec 3, 2022 • edited

JonnycatMeow commented Dec 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JonnycatMeow Dec 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EliteMasterEric commented Dec 4, 2022 • edited

EliteMasterEric commented Dec 4, 2022

JonnycatMeow commented Dec 7, 2022

Raltyro commented Feb 26, 2023

EliteMasterEric commented Feb 27, 2023

joshtynjala commented Feb 27, 2023

EliteMasterEric commented Feb 27, 2023

EliteMasterEric commented Feb 27, 2023

Geokureli Apr 27, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Raltyro commented Dec 17, 2023

EliteMasterEric commented Dec 17, 2023

JonnycatMeow commented Feb 21, 2024

JonnycatMeow commented Feb 21, 2024

JonnycatMeow commented Feb 21, 2024

JonnycatMeow commented Feb 21, 2024

JonnycatMeow commented Feb 21, 2024

loudoweb commented Aug 14, 2022 •

edited

JonnycatMeow commented Dec 3, 2022 •

edited

JonnycatMeow Dec 3, 2022 •

edited

EliteMasterEric commented Dec 4, 2022 •

edited

Geokureli Apr 27, 2023 •

edited