Skip to content

Conversation

Adrian1066
Copy link
Contributor

Please read the Contributing guide before making a PR.

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

This PR adds support for the XboxSeries platform to the SRP packages and the PPv2 package.


Testing status

Manual testing with HDRP packages
Testing of HDRP, URP, ShaderGraph and VFXGraph run locally.
Tested on Xbox to check for regressions


Comments to reviewers

PPV2 changes

The Xbox build target is wrapped to allow this package to work even in older versions of Unity as PS5 will not show up as a valid enum member there.

@github-actions
Copy link

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

// We need to disable the scalarization here on xbox due to bad code generated by FXC for the eye shader.
// This shouldn't have an enormous impact since with Area lights we are already exploded in VGPR by this point.
#if !defined(SHADER_API_XBOXONE) && (defined(PLATFORM_SUPPORTS_WAVE_INTRINSICS) && !defined(LIGHTLOOP_DISABLE_TILE_AND_CLUSTER))
#if !defined(SHADER_API_XBOXONE) && !defined(SHADER_API_GAMECORE) && (defined(PLATFORM_SUPPORTS_WAVE_INTRINSICS) && !defined(LIGHTLOOP_DISABLE_TILE_AND_CLUSTER))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this working on Xbox? was guessing only gamecore is a problem right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an old problem with just the shadow data scalarization, FXC fails hard with the eye shader here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is a fix for xbox, then we should do a separate PR, it shouldn't be part of the GameCore PR. But yes, as we backport up to 2020.2 it is ok, eye shader wasn't present before

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add a comment for the shader data scalarezitation for eye sahder, so we don't forget it is because of this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was there before and there is a comment up above :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    // We need to disable the scalarization here on xbox due to bad code generated by FXC for the eye shader.
    // This shouldn't have an enormous impact since with Area lights we are already exploded in VGPR by this point.

Copy link
Contributor

@esmelusina esmelusina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine for SG.

@sebastienlagarde sebastienlagarde merged commit a46b92f into master Feb 21, 2021
@sebastienlagarde sebastienlagarde deleted the platform/gamecore branch February 21, 2021 21:22
PaulDemeulenaere added a commit that referenced this pull request Feb 24, 2021
# Conflicts:
#	com.unity.visualeffectgraph/Shaders/ParticleLinesSW/PassForward.template

Todo restore minimal change in #3292 on PassForward.template
See https://github.com/Unity-Technologies/Graphics/pull/3292/files#diff-d6bea52c2291e8be1d7e94b7c2016caf6701b7f5993722f9dd082efcbe56d5ccR16
sebastienlagarde pushed a commit that referenced this pull request Feb 24, 2021
sebastienlagarde added a commit that referenced this pull request Feb 25, 2021
Co-authored-by: Adrian1066 <44636759+Adrian1066@users.noreply.github.com>
sebastienlagarde pushed a commit that referenced this pull request Mar 3, 2021
PaulDemeulenaere added a commit that referenced this pull request May 5, 2021
* Squashed commit of the following:

commit 1ee68db66657158ac331c590d41a16d06d125283
Merge: f96bff4b1432 d31d7ee
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 20:06:58 2021 +0100

    Merge branch 'vfx/staging' into vfx/feature/2d-renderer-integration

commit f96bff4b1432ae03b7a20b1151a10b3b43bd3db9
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 17:53:34 2021 +0100

    Fix ParticleLinesSW (TODO: Check & Isolate this fix)

commit eb749b7
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 17:30:47 2021 +0100

    *Add graphicTest for 2D Renderer

commit b6998e8
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 17:25:32 2021 +0100

    Update & Integrate graphicTest

commit 95714da
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 16:54:12 2021 +0100

    Move graphicTest asset

commit 3b50012
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 16:52:00 2021 +0100

    Add graphicTest settings

commit bbb4ac6
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 16:48:32 2021 +0100

    Some rename

commit 0497680
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 12 16:46:52 2021 +0100

    *Add debug data

commit e3c8992
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Thu Feb 11 11:11:31 2021 +0100

    *Some wip : identify an issue with sw line in URP

commit 0dfca77
Merge: dd43fb5 6136320
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Wed Feb 10 16:54:53 2021 +0100

    Merge branch 'vfx/staging' into vfx/feature/2d-renderer-integration

commit dd43fb5
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Mon Feb 8 16:28:27 2021 +0100

    *Test data

commit 23a87bb
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Mon Feb 8 16:23:26 2021 +0100

    *Add sorting layer UI (TODO : replace by a custom UI)

commit 928407c
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 5 17:49:20 2021 +0100

    *Extend minmal test

commit d23dd15
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 5 16:24:44 2021 +0100

    *TestData

commit 0e3e2b4
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 5 16:14:24 2021 +0100

    Fix particle SW (doesn't work in 2D btw)

commit 13ca23f
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 5 15:29:33 2021 +0100

    *test asset for output (wip)

commit d4e4b5b
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 5 15:19:50 2021 +0100

    Add PassForward2D for every urp output

commit fe288ae
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Fri Feb 5 11:47:46 2021 +0100

    Temp basic scene (& setup sorting layer)

commit ed5ebcb
Author: Paul Demeulenaere <pauld@unity3d.com>
Date:   Thu Feb 4 16:47:11 2021 +0100

    Add 2DRenderer in URP list

* GraphicTest : Use default zWrite mode

* Switch train to zTest always

* First step : Remove RenderSortingLayerFields, should do reflection for SortingLayerField

* Properly use reflection to avoid breakage due to api change in internal

* Temp revert fix to isolate change related to proj computation

* Fix line renderer issue due to inverted proj

* Update reference image for newly added test

* *Update documentation

* Reviewed documentation for the visual effect component

* Update VisualEffectComponent.md

* Reapply change from #3292

See https://github.com/Unity-Technologies/Graphics/pull/3292/files#diff-d6bea52c2291e8be1d7e94b7c2016caf6701b7f5993722f9dd082efcbe56d5ccR16

* Fix compilation

* Avoid leaking VFXVertexAdditionalProcess after VFXPassShadow generation : VFXApplyShadowBias was applied to Universal2D

@julienf It was an interesting bug the "VFXVertexAdditionalProcess" was remaining after shadow pass generation and was leaking to the following pass "Universal2D"
It could have been workaround changing the pass order here : https://github.com/Unity-Technologies/Graphics/pull/3489/files#diff-6c94d0a95b0b8a7f4c1b81748cdbc112bd5e2c6196fce18beef39dd79b22cb9aR9 but it's better to clean the define after usage.

* *Update changelog.md

Co-authored-by: Lewis Jordan <lewis.jordan@hotmail.co.uk>
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.