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

[SUGGGESTION] ShaderBuiltins should be split #78

Open
thargy opened this issue May 30, 2018 · 1 comment
Open

[SUGGGESTION] ShaderBuiltins should be split #78

thargy opened this issue May 30, 2018 · 1 comment

Comments

@thargy
Copy link
Contributor

thargy commented May 30, 2018

Further to the conversations in #65, #67 and GH-77, we should consider reserving ShaderBuiltins for members that will throw a ShaderBuiltinException as they cannot reasonably be implemented on the CPU - therefore code that uses them cannot be run on the CPU.

Methods that can have CPU equivalents should be in a seperate class (Builtins) to indicate that they can run on the GPU & CPU. This makes it easy to see if a file is using GPU only functionality (as it will reference ShaderBuiltins. By only including Builtins you can be sure the members used will be available when running the same code directly on the CPU.

Note, before GH-77 all members of ShaderBuiltins throw a ShaderBuiltinException anyway when executed on the CPU, even when they could be implemented to run on the CPU, so this wasn't as big an issue. However, going forward, the ability to share code between GPU & CPU is a USP of the library, and so consideration should be given to creating CPU implementations of members whenever feasible, so the distinction becomes more important.

@thargy
Copy link
Contributor Author

thargy commented May 30, 2018

Note, this will also make it easier to test, as an automatic test can easily be added to ensure all ShaderBuiltins members throw a ShaderBuiltinException on invocation. Further, an automated test can be added to test consistency between Builtins members on CPU & GPU.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant