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.

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

Deprecate ComputeScreenPos #2529

Merged
merged 20 commits into from
Dec 14, 2020
Merged

Deprecate ComputeScreenPos #2529

merged 20 commits into from
Dec 14, 2020

Conversation

eh-unity
Copy link
Contributor

@eh-unity eh-unity commented Nov 4, 2020 •
edited

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

Deprecate ComputeScreenPos as it's a duplicate and badly named/documented. It confuses people.
Remove it's usage from URP.

Testing status

Describe what manual/automated tests were performed for this PR
2020.2 Pass, 072 fail is expected. https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/universal%252Fscreenpos-deprecate/.yamato%252Fall-universal.yml%2523All_Universal_CUSTOM-REVISION/4219839/job/pipeline
But the failed job passes in current master with 20.2.
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/universal%252Fscreenpos-deprecate/.yamato%252Funiversal-win-dx11.yml%2523Universal_Win_DX11_playmode_XR_mono_Linear_CUSTOM-REVISION/4221238/job

Comments to reviewers

Notes for the reviewers you have assigned.

@eh-unity eh-unity requested a review from Verasl November 5, 2020 11:53
@eh-unity eh-unity marked this pull request as ready for review November 6, 2020 12:13
@eh-unity eh-unity requested review from a team as code owners November 6, 2020 12:13
@phi-lira
Copy link
Contributor

Adding @rainsing for review on 2D changes.

@eh-unity eh-unity added the ready-to-merge Add this tag whenever your PR is ready to be merged. i.e, non draft, all reviewers approved, ABV label Nov 16, 2020
@eh-unity
Copy link
Contributor Author

This is universal only change and it's passing the tests https://yamato.cds.internal.unity3d.com/job/4296819 . CUSTOM-REVISON is 20.2, which fails because of the codestyle rename.
Should be good to go after @oleks-k docs review.

phi-lira
phi-lira approved these changes Nov 26, 2020
View reviewed changes
Copy link
Contributor

@phi-lira phi-lira left a comment

Choose a reason for hiding this comment

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

LGTM

oleks-k
oleks-k approved these changes Dec 4, 2020
View reviewed changes
Copy link
Contributor

@oleks-k oleks-k left a comment

Choose a reason for hiding this comment

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

Approving assuming that you will move writing-shaders-urp-reconstruct-world-position.md and the PNG to a separate PR as we agreed (to make the review process more optimal).
Thanks for the great input!

@eh-unity eh-unity mentioned this pull request Dec 4, 2020
Merged
@eh-unity
Copy link
Contributor Author

eh-unity commented Dec 4, 2020

Separated the docs into another branch: #2849

Verasl
Verasl approved these changes Dec 14, 2020
View reviewed changes
Copy link
Contributor

@Verasl Verasl left a comment

Choose a reason for hiding this comment

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

LGTM

@phi-lira phi-lira merged commit 67ff5fc into master Dec 14, 2020
@phi-lira phi-lira deleted the universal/screenpos-deprecate branch December 14, 2020 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rainsing rainsing rainsing approved these changes

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

@Verasl Verasl Verasl approved these changes

@oleks-k oleks-k oleks-k approved these changes

Assignees
No one assigned
Labels
ready-to-merge Add this tag whenever your PR is ready to be merged. i.e, non draft, all reviewers approved, ABV testing universal
Projects
None yet
Milestone
No milestone
5 participants
@eh-unity @phi-lira @rainsing @Verasl @oleks-k