Skip to content

Update TextureGather Macros in GLCore.hlsl #7029

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.

Sign up for GitHub

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

Jump to bottom
Merged
phi-lira merged 2 commits into from
Feb 8, 2022
Merged

Update TextureGather Macros in GLCore.hlsl #7029

phi-lira merged 2 commits into from
Feb 8, 2022

Conversation

@gmitrano-unity
Copy link
Contributor

@gmitrano-unity gmitrano-unity commented Feb 7, 2022 •
edited
Loading

Purpose of this PR

This change updates the platform shader macros in GLCore.hlsl to enable
support for TextureGather operations on Unity's 4.5 shader target.

The changes in this PR are based off of GH #2346 which modified the
GLES3.hlsl file in order to fix cubemap array support. This PR basically
takes those same changes and applies them to GLCore.hlsl in addition.

This commit also adds some missing component specific macros for
texture gather (red, green, blue, alpha).

Testing status

Tested GLCore (4.5) locally on Viking Village URP with FSR on

Comments to reviewers

Notes for the reviewers you have assigned.

Checklist for PR maker

  • Backports?
  • Changleog updates

@gmitrano-unity
Update TextureGather Macros in GLCore.hlsl
823d600 
This change updates the platform shader macros in GLCore.hlsl to enable
support for TextureGather operations on Unity's 4.5 shader target.

The changes in this PR are based off of GH #2346 which modified the
GLES3.hlsl file in order to fix cubemap array support. This PR basically
takes those same changes and applies them to GLCore.hlsl in addition.

This commit also adds some missing component specific macros for
texture gather (red, green, blue, alpha).
@github-actions
Copy link

github-actions bot commented Feb 7, 2022

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://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

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:
/jobDefinition/.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.

@PaulDemeulenaere PaulDemeulenaere mentioned this pull request Feb 8, 2022
Closed
@gmitrano-unity
Update Changelog
f7d0975 
This commit updates the changelog to reflect the recent changes to the
GLCore.hlsl header.
@gmitrano-unity gmitrano-unity marked this pull request as ready for review February 8, 2022 13:16
@phi-lira phi-lira merged commit 6f95176 into master Feb 8, 2022
@phi-lira phi-lira deleted the core/glcore-gather-fixes branch February 8, 2022 16:09
gmitrano-unity added a commit that referenced this pull request Feb 9, 2022
@gmitrano-unity
Fix Regression in GLCore from GH #7029
b3a9312 
The changes in GH #7029 accidentally broke the platform shader macro
override system for GLCore's texture cube arrays. This change fixes
the issue by restoring the missing platform macros.
This was referenced Feb 9, 2022
Merged
Merged
PaulDemeulenaere pushed a commit that referenced this pull request Feb 11, 2022
@gmitrano-unity @PaulDemeulenaere
Fix Regression in GLCore from GH #7029
ca0371d 
The changes in GH #7029 accidentally broke the platform shader macro
override system for GLCore's texture cube arrays. This change fixes
the issue by restoring the missing platform macros.
phi-lira pushed a commit that referenced this pull request Feb 16, 2022
@gmitrano-unity
Fix Regression in GLCore from GH #7029 (#7062)
21f5b9c 
* Fix Regression in GLCore from GH #7029

The changes in GH #7029 accidentally broke the platform shader macro
override system for GLCore's texture cube arrays. This change fixes
the issue by restoring the missing platform macros.

* Fall Back to UNITY_NO_CUBEMAP_ARRAY Due to Bug

Due to some sort of bug, SHADER_AVAILABLE_CUBEARRAY is not defined in
compute shaders. This commit switches the code back to
UNITY_NO_CUBEMAP_ARRAY to avoid futher problems.

(FogBugz Case 1402457)

* Fix Comment Typo

* Revert UNITY_NO_CUBEMAP_ARRAY Changes

This change removes usage of UNITY_NO_CUBEMAP_ARRAY and replaces it with
SHADER_AVAILABLE_CUBEARRAY since that's the correct macro to be using.
The bug we encountered after this change in VFX was actually due to a
missing pragma: "#pragma require cubearray".
PaulDemeulenaere pushed a commit that referenced this pull request Feb 16, 2022
@gmitrano-unity @PaulDemeulenaere
Fix Regression in GLCore from GH #7029 (#7062)
521a98a 
* Fix Regression in GLCore from GH #7029

The changes in GH #7029 accidentally broke the platform shader macro
override system for GLCore's texture cube arrays. This change fixes
the issue by restoring the missing platform macros.

* Fall Back to UNITY_NO_CUBEMAP_ARRAY Due to Bug

Due to some sort of bug, SHADER_AVAILABLE_CUBEARRAY is not defined in
compute shaders. This commit switches the code back to
UNITY_NO_CUBEMAP_ARRAY to avoid futher problems.

(FogBugz Case 1402457)

* Fix Comment Typo

* Revert UNITY_NO_CUBEMAP_ARRAY Changes

This change removes usage of UNITY_NO_CUBEMAP_ARRAY and replaces it with
SHADER_AVAILABLE_CUBEARRAY since that's the correct macro to be using.
The bug we encountered after this change in VFX was actually due to a
missing pragma: "#pragma require cubearray".
PaulDemeulenaere added a commit that referenced this pull request Feb 21, 2022
@PaulDemeulenaere
Backport of #7025
b929d49 
Bonus : Including changes from https://github.cds.internal.unity3d.com/unity/vfx-graphics/pull/295 (cc @gabriel-delacruz)

commit c7ed2f6
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 21 15:38:13 2022 +0100

    Update changelog.md

commit 351ec2a
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 21 15:13:10 2022 +0100

    Fix OGL in Standalone

    - "opengl" corresponds to "kShaderCompPlatformGL_Obsolete" (GL2+)
    - "glcore" is "kShaderCompPlatformOpenGLCore" (GL 3+ supports compute)

    I don't explain why it sometimes works in editor or sometimes completly silent.  ¯¯\_(ヅ)_/¯¯

    See https://github.cds.internal.unity3d.com/unity/unity/blob/8d5d36f123d953029e8bf8f982576aa6a0696a2d/Tools/UnityShaderCompiler/ShaderCompilerClient.h#L35
    See also special alias declaration https://github.cds.internal.unity3d.com/unity/unity/blob/5a8e0e752a3977687cd273e5692567e834e3f27b/Tools/UnityShaderCompiler/Utilities/ShaderImportUtils.cpp#L51

    Fix case https://fogbugz.unity3d.com/f/cases/1403988/

commit 734850d
Merge: a700c33 778ddac
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 21 09:19:44 2022 +0100

    Merge branch 'master' into vfx/fix/1392834-remove-invalid-vfx-passes-from-sg

commit a700c33
Author: Paul Demeulenaere <[email protected]>
Date:   Thu Feb 17 17:01:26 2022 +0100

    *Apply formatting

commit bfd7d6c
Author: Paul Demeulenaere <[email protected]>
Date:   Thu Feb 17 16:52:05 2022 +0100

    kPragmaDescriptorNone isn't null

    Simpler to avoid multiple search for replacement
    Resolve issue #7025 (comment)

commit 350e4aa
Author: Paul Demeulenaere <[email protected]>
Date:   Thu Feb 17 12:01:07 2022 +0100

    *Add correct support of #pragma require cubearray

    Several cases where needed
    - Compute (and implicit compute like sorting)
    - Builtin Output
    - SG Output

commit 882bdb7
Author: Paul Demeulenaere <[email protected]>
Date:   Thu Feb 17 10:12:18 2022 +0100

    Correct implementation VFXCommon.hlsl

    TODO: Add needed pragma dynamically https://unity.slack.com/archives/C02TGPN8MRV/p1644932447721759?thread_ts=1644242387.827459&cid=C02TGPN8MRV

commit 4bb5cac
Merge: da0454c a4b081a
Author: Paul Demeulenaere <[email protected]>
Date:   Thu Feb 17 09:52:47 2022 +0100

    Merge branch 'master' into vfx/fix/1392834-remove-invalid-vfx-passes-from-sg

    # Conflicts:
    #	com.unity.visualeffectgraph/CHANGELOG.md

commit da0454c
Author: Paul Demeulenaere <[email protected]>
Date:   Thu Feb 17 09:52:10 2022 +0100

    Revert "Workaround"

    This reverts commit 6a70285.

commit 6a70285
Author: Paul Demeulenaere <[email protected]>
Date:   Fri Feb 11 09:28:49 2022 +0100

    Workaround

    Issue introduced at #2346
    See this conversation

commit ca0371d
Author: Gregory Mitrano <[email protected]>
Date:   Wed Feb 9 08:19:20 2022 -0500

    Fix Regression in GLCore from GH #7029

    The changes in GH #7029 accidentally broke the platform shader macro
    override system for GLCore's texture cube arrays. This change fixes
    the issue by restoring the missing platform macros.

commit 3683c87
Author: Paul Demeulenaere <[email protected]>
Date:   Tue Feb 8 18:28:45 2022 +0100

    Revert unexpected merge change

commit dbf2964
Merge: 3032c86 f70620a
Author: Paul Demeulenaere <[email protected]>
Date:   Tue Feb 8 18:03:15 2022 +0100

    Merge branch 'master' into vfx/fix/1392834-remove-invalid-vfx-passes-from-sg

commit 3032c86
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 7 16:44:26 2022 +0100

    *Update changelog.md

commit 033b409
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 7 16:40:32 2022 +0100

    *Update comment

    To anticipate incoming change with GetInstancingAdditionalDefines

commit 4f88add
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 7 16:31:32 2022 +0100

    Add OpenGL in target

    Early detecting VFX compilation error

commit f4513a7
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 7 16:22:06 2022 +0100

    Fix build of GLCore.hlsl

    Fix failure com.unity.render-pipelines.core/ShaderLibrary/Texture.hlsl(66)
    https://github.com/Unity-Technologies/Graphics/blob/65776aefaaa7f24455cc4b5fa8b3b90c8dd606b7/com.unity.render-pipelines.core/ShaderLibrary/Texture.hlsl#L66

commit 7fed326
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 7 16:03:44 2022 +0100

    Same instancing option removal in HDRP

commit 16fae6c
Author: Paul Demeulenaere <[email protected]>
Date:   Mon Feb 7 15:56:37 2022 +0100

    Removing pass was wrong, replace pragma

    Some needed pass uses the default target 2.0 on URP and it can be problematic to change the pass count
    Bonus : add a mechanism to remove useless instancing multicompile

commit 9789e99
Author: Paul Demeulenaere <[email protected]>
Date:   Fri Feb 4 17:19:30 2022 +0100

    Statify kInvalidPassWithPragmas

commit c26c54b
Merge: e348f54 5850cf7
Author: Paul Demeulenaere <[email protected]>
Date:   Fri Feb 4 16:56:57 2022 +0100

    Merge branch 'master' into vfx/fix/1392834-remove-invalid-vfx-passes-from-sg

commit e348f54
Author: Paul Demeulenaere <[email protected]>
Date:   Tue Feb 1 17:37:43 2022 +0100

    Draft removing target 2.0

    See https://fogbugz.unity3d.com/f/cases/1392834/
    See also this conversation https://unity.slack.com/archives/G1BTWN88Z/p1643362523117700

# Conflicts:
#	com.unity.visualeffectgraph/CHANGELOG.md
#	com.unity.visualeffectgraph/Shaders/SDFBaker/GenSdfRayMap.compute
#	com.unity.visualeffectgraph/Shaders/Sort.compute
#	com.unity.visualeffectgraph/Shaders/UpdateStrips.compute
#	com.unity.visualeffectgraph/Shaders/VFXCameraSort.template
#	com.unity.visualeffectgraph/Shaders/VFXCopyBuffer.compute
#	com.unity.visualeffectgraph/Shaders/VFXFillIndirectArgs.compute
#	com.unity.visualeffectgraph/Shaders/VFXInit.template
#	com.unity.visualeffectgraph/Shaders/VFXOutputUpdate.template
#	com.unity.visualeffectgraph/Shaders/VFXUpdate.template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@phi-lira phi-lira phi-lira approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

SRP universal

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gmitrano-unity @phi-lira