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

Remove Runtime Uses of EnableKeyword/DisableKeyword for shader_feature Keywords #7582

Closed
Cameron-Micka opened this issue Mar 24, 2020 · 4 comments
Labels
Bug Shaders / Materials Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on

Comments

@Cameron-Micka
Copy link
Member

Describe the bug

A handful of issues have been reported around shader features working in editor but not working within a built player. This is due to a few MRTK scripts using the EnableKeyword and DisableKeyword methods at runtime for keywords which utilize the shader_feature pragma. For more information as to why this is an issue please see "Special requirements for Scripting with the Standard Shader."

To reproduce

For example, if you look at ConstructGltf.CreateMRTKShaderMaterial() you can see code which does the following:

material.EnableKeyword("_ALPHATEST_ON");
material.DisableKeyword("_ALPHABLEND_ON");
material.DisableKeyword("_ALPHAPREMULTIPLY_ON");

This is perfectly valid within editor scripts, but not at runtime.

Expected behavior

Runtime calls to EnableKeyword/DisableKeyword should be removed and replaced with explicit material references. Please see this thread for more information.

Target platform

  • Any built Unity player.
@mtaulty
Copy link

mtaulty commented Aug 7, 2020

Like @FreakTheMighty, I've come across this issue when trying to load glTF models dynamically as per #7543.

I'm on a slightly old toolkit and so in order to temporarily get around the problem with directional light, I've switched the _DIRECTIONAL_LIGHT in the MRTK standard shader to be marked with the pragma multi_compile rather than shader_feature - I'm assuming that I'll take a hit for this and it doesn't solve the potential issues with the other uses of Enable/DisableKeyword in the glTF code but for my needs it seemed to make things a lot better (i.e. I got some light on my models on device).

@Cameron-Micka
Copy link
Member Author

@mtaulty that should be a totally fine workaround for your scenario. You will notice slightly longer build times as more shader variants will get included in the build. But, I wouldn't expect you experience any significant runtime costs.

@Cameron-Micka Cameron-Micka self-assigned this Apr 20, 2021
@stale
Copy link

stale bot commented Apr 18, 2022

This issue has been marked as stale by an automated process because it has not had any recent activity. It will be automatically closed in 30 days if no further activity occurs. If this is still an issue please add a new comment with more recent details and repro steps.

@stale stale bot added the Stale label Apr 18, 2022
@AMollis AMollis added this to the MRTK 2.x future milestone Apr 27, 2023
@stale stale bot removed the Stale label Apr 27, 2023
@IssueSyncBot
Copy link
Collaborator

We appreciate your feedback and thank you for reporting this issue.

Microsoft Mixed Reality Toolkit version 2 (MRTK2) is currently in limited support. This means that Microsoft is only fixing high priority security issues. Unfortunately, this issue does not meet the necessary priority and will be closed. If you strongly feel that this issue deserves more attention, please open a new issue and explain why it is important.

Microsoft recommends that all new HoloLens 2 Unity applications use MRTK3 instead of MRTK2.

Please note that MRTK3 was released in August 2023. It features an all new architecture for developing rich mixed reality experiences and has a minimum requirement of Unity 2021.3 LTS. For more information about MRTK3, please visithttps://www.mixedrealitytoolkit.org.

Thank you for your continued support of the Mixed Reality Toolkit!

@IssueSyncBot IssueSyncBot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 2024
@IssueSyncBot IssueSyncBot added the Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on label Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Shaders / Materials Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on
Projects
None yet
Development

No branches or pull requests

4 participants