Skip to content

emitting UI geometry for offscreen cameras too (case 1344969 fix) #5894

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
merged 2 commits into from
Oct 12, 2021
Merged

emitting UI geometry for offscreen cameras too (case 1344969 fix) #5894

merged 2 commits into from
Oct 12, 2021

Conversation

manuele-bonanno
Copy link
Contributor

Purpose of this PR

Fix for https://fogbugz.unity3d.com/f/cases/1344969/

Testing status

Comments to reviewers

@github-actions
Copy link

github-actions bot commented Oct 4, 2021 •
edited
Loading

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)

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

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

Shader Graph
/jobDefinition/.yamato%252Fall-shadergraph.yml%2523PR_ShaderGraph_trunk
Depending on your PR, you may also want
/jobDefinition/.yamato%252Fall-shadergraph_builtin_foundation.yml%2523PR_ShaderGraph_BuiltIn_Foundation_trunk
/jobDefinition/.yamato%252Fall-shadergraph_builtin_lighting.yml%2523PR_ShaderGraph_BuiltIn_Lighting_trunk

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

ellioman
ellioman approved these changes Oct 4, 2021
View reviewed changes
Copy link
Contributor

@ellioman ellioman left a comment

Choose a reason for hiding this comment

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

Remember to update the changelog with the case :)

phi-lira
phi-lira approved these changes Oct 11, 2021
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.

Missing changelog.

@phi-lira phi-lira changed the base branch from master to universal/staging October 12, 2021 12:06
@phi-lira phi-lira removed the request for review from cinight October 12, 2021 12:07
@phi-lira phi-lira marked this pull request as ready for review October 12, 2021 12:07
@phi-lira phi-lira requested review from a team October 12, 2021 12:07
@phi-lira phi-lira changed the base branch from universal/staging to master October 12, 2021 12:09
@phi-lira phi-lira requested a review from a team as a code owner October 12, 2021 12:09
@theopnv theopnv removed the request for review from a team October 12, 2021 12:53
@phi-lira phi-lira changed the base branch from master to universal/staging October 12, 2021 13:56
@phi-lira phi-lira deleted the branch universal/staging October 12, 2021 14:07
@phi-lira phi-lira closed this Oct 12, 2021
@phi-lira phi-lira reopened this Oct 12, 2021
@phi-lira phi-lira changed the base branch from universal/staging to master October 12, 2021 14:12
@phi-lira phi-lira changed the base branch from master to universal/staging October 12, 2021 14:12
@phi-lira phi-lira removed request for a team October 12, 2021 14:13
@phi-lira
Copy link
Contributor

Made some mistakes when rebasing to universal/staging. Fixed now.

@phi-lira phi-lira merged commit b4f79f0 into universal/staging Oct 12, 2021
@phi-lira phi-lira deleted the urp/offscreen_camera_ui_fix branch October 12, 2021 14:14
@phi-lira phi-lira mentioned this pull request Oct 12, 2021
Closed
@phi-lira phi-lira restored the urp/offscreen_camera_ui_fix branch October 18, 2021 15:13
@phi-lira phi-lira deleted the urp/offscreen_camera_ui_fix branch October 18, 2021 15:19
phi-lira added a commit that referenced this pull request Oct 18, 2021
@phi-lira
emitting UI geometry for offscreen cameras too (case 1344969 fix) #5894
d6de034 
commit a99044f
Merge: bd0a2db 44b86ce
Author: Manuele Bonanno <manuele.bonanno@unity3d.com>
Date:   Mon Oct 11 14:17:46 2021 +0200

    Merge branch 'master' into urp/offscreen_camera_ui_fix

commit bd0a2db
Author: Manuele Bonanno <manuele.bonanno@unity3d.com>
Date:   Mon Oct 4 15:43:58 2021 +0200

    emitting UI geometry for offscreen cameras too (case 1344969 fix)
@phi-lira phi-lira mentioned this pull request Oct 18, 2021
Merged
odbb added a commit that referenced this pull request Oct 19, 2021
@odbb
Merge branch 'master' into adaptive-performance/add-decals-scaler
6e903e4 
* master:
  Enable iris normal for Eye shader (#5880)
  Update to HDRP Asset analytics (#6060)
  [HDPR] Update reference screenshots for Linux Vulkan
  Update 206_Motion_Vectors.png (#6046)
  Updated code owner for URP (#6052)
  [URP] Re-enable ios pbr tests #5983
  Fix error when using gizmos with camera stack in editor #5913
  emitting UI geometry for offscreen cameras too (case 1344969 fix) #5894
  Remove deprecated UNITY_USE_NATIVE_HDR keyword from shader code. #5569
  [XR][URP] Fix issue with vignette location in XR #5471 #5471
unity-emilk pushed a commit that referenced this pull request May 17, 2023
@manuele-bonanno
[URP] Fix for the UI being drawn twice in some scenarios
98c99c4 
This PR fixes a regression introduced by an UI related fix introduced in this PR:
#5894

While making sure that the bug fixed by that PR is still fixed, the new changes fix the additional draw calls issue. The handling of UI geometry for non Game cameras is now the same as HDRP, which makes the pipelines behaviour more consistent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ellioman ellioman ellioman approved these changes

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

Assignees
No one assigned
Labels
HDRP sdet Shader Graph SRP testing universal vfx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@manuele-bonanno @phi-lira @ellioman