Skip to content

2021.2/hybrid switch underscore fix #5679

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

Merged

Conversation

JussiKnuuttila
Copy link
Contributor

@JussiKnuuttila JussiKnuuttila commented Sep 16, 2021

Prior to this PR, Hybrid Renderer shader constants used a naming format which frequently results in double underscores being present in the constant names, which unfortunately triggers warnings on the Nintendo Switch platform and causes massive log spam and slowdowns when shaders are compiled.

This PR alters the DOTS instancing property definitions in the Graphics package to remove underscores from the names, which prevents a double underscore from happening whenever there's a variable name that starts with an underscore (very common in Unity).

Using this PR requires a version of Unity that supports the underscoreless format. A PR will be landed in 2021.2 to support this. An unsupported Unity version cannot run the Hybrid Renderer, but GameObject rendering should be completely unaffected regardless of the versions, as the change should only affect Hybrid Renderer shader variants.

Checklist for PR maker

  • Have you added a backport label (if needed)? For example, the need-backport-* label. After you backport the PR, the label changes to backported-*.
  • Have you updated the changelog? Each package has a CHANGELOG.md file.
  • Have you updated or added the documentation for your PR? When you add a new feature, change a property name, or change the behavior of a feature, it's best practice to include related documentation changes in the same PR. If you do add documentation, make sure to add the relevant Graphics Docs team member as a reviewer of the PR. If you are not sure which person to add, see the Docs team contacts sheet.
  • Have you added a graphic test for your PR (if needed)? When you add a new feature, or discover a bug that tests don't cover, please add a graphic test.

Purpose of this PR

Why is this PR needed, what hard problem is it solving/fixing?


Testing status

Describe what manual/automated tests were performed for this PR


Comments to reviewers

Notes for the reviewers you have assigned.

…scores on Switch

# Conflicts:
#	com.unity.render-pipelines.core/ShaderLibrary/UnityInstancing.hlsl
#	com.unity.render-pipelines.universal/ShaderLibrary/UniversalDOTSInstancing.hlsl
#	com.unity.shadergraph/Editor/Generation/Processors/PropertyCollector.cs
@JussiKnuuttila JussiKnuuttila changed the base branch from master to 2021.2/staging September 16, 2021 07:37
@github-actions
Copy link

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_trunk
With changes to HDRP packages, you should also run
/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_trunk

URP
/.yamato%252Fall-urp.yml%2523PR_URP_trunk
With changes to URP packages, you should also run
/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_trunk

VFX
/.yamato%252Fall-vfx.yml%2523PR_VFX_trunk

SRP Core
You could run ABV on your branch before merging your PR, but it will start A LOT of jobs. Please be responsible about it and run it only when you feel the PR is ready:
/.yamato%252F_abv.yml%2523all_project_ci_trunk
Be aware that any modifications to the Core package impacts everyone in the Graphics repo so please discuss the PR with your lead.

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@JussiKnuuttila JussiKnuuttila marked this pull request as ready for review September 16, 2021 15:33
@JussiKnuuttila JussiKnuuttila requested review from a team as code owners September 16, 2021 15:33
@sebastienlagarde sebastienlagarde merged commit 7389a8c into 2021.2/staging Sep 16, 2021
@sebastienlagarde sebastienlagarde deleted the 2021.2/hybrid-switch-underscore-fix branch September 16, 2021 15:55
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.

2 participants