Skip to content

Fix preview scene objects marked dirty by migration #6361

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
merged 4 commits into from
Nov 25, 2021

Conversation

arttu-peltonen
Copy link
Contributor


Purpose of this PR

Fixes case 1367204.

HDRP migration system marks GameObjects dirty whenever their components (e.g. HDAdditionalLightData) have received new versions and have gone through migration. This lets user know that they should save the scene (presumably to avoid executing migration every single time the scene is opened).

This workflow seems fine, but it's problematic when a prefab requiring migration is used as "Editing Environment" prefab (see documentation), meaning "a prefab that is used as the background context scene in Prefab Mode". In this situation, the editing environment scene is obviously not saved, and thus it also doesn't make sense to mark it dirty.


Testing status

Retraced repro steps of 1367204. Before the fix, asterisk appeared (briefly). After the fix, the asterisk no longer appears (and verified in debugger that the gameobjects with migrated components are not marked as dirty).

@arttu-peltonen arttu-peltonen self-assigned this Nov 22, 2021
@github-actions
Copy link

github-actions bot commented Nov 22, 2021

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%23PR_HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%2Fall-lightmapping.yml%23PR_Lightmapping_trunk

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.

Copy link
Contributor

@TomasKiniulis TomasKiniulis left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Also took a brief look if prefab is still dirtied after an actual modification is made and marked clean when saved. Looks good.

Copy link
Contributor

@RSlysz RSlysz left a comment

Choose a reason for hiding this comment

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

Have you tested that the prefab itself is dirty (meaning when you do save all assets, the serialization of the prefab is updated ?

@arttu-peltonen
Copy link
Contributor Author

Have you tested that the prefab itself is dirty (meaning when you do save all assets, the serialization of the prefab is updated ?

Yes, the bug prefab was made on an earlier version so it's out of date and requires re-save, after which I saw asset version was bumped. Opening and saving the prefab bypassed the issue even without the fix.

@RSlysz RSlysz self-requested a review November 25, 2021 10:16
@arttu-peltonen
Copy link
Contributor Author

Will retarget this to master. To me seems 10.x backport is needed, but 21.2 should be easy.

@arttu-peltonen arttu-peltonen force-pushed the rpw/bugfix/fix-preview-scene-dirtying branch from 557eff5 to 5ce23aa Compare November 25, 2021 11:20
@arttu-peltonen arttu-peltonen requested review from a team as code owners November 25, 2021 11:20
@arttu-peltonen arttu-peltonen changed the base branch from hd/bugfix to master November 25, 2021 11:20
@arttu-peltonen arttu-peltonen removed request for a team November 25, 2021 11:21
sebastienlagarde added a commit that referenced this pull request Dec 9, 2021
* - Fixed edges and ghosting appearing on shadow matte due to the shadow being black outside the range of the light (case 1371441). #6279

* Fixed interpolation issue with wind orientation (case 1379841). #6284

* Fixed range fields for depth of field #6285

* [Core] Fix XR support in CoreUtils.Drawfullscreen #6287

* ** Fixing DLSS failing when MV are disabled ** (#6292)

* Using texture types instead of RenderTexture types for DLSSPass

* Changelog

* Formatting

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* [Fix] Lens Flare visible when being behind a camera with Panini Projection on (case 1370214) (#6293)

* Fix panini for LensFlare

* Add changelog

* Update CHANGELOG.md

* Fixed the ray tracing acceleration structure build marker not being included in the ray tracing stats (case 1379383). #6277

* [HDRP] Remove alpha from local volumetric fog color field #6310

* [HDRP] Changed default numbder of physically based sky bounce from 8 to 3 #6304

* [HDRP] Improve decal performances when they use different material and the same draw order. #6303

* [HDRP] Update reference screenshots #6404

* Fixed Nans happening due to volumetric clouds when the pixel color is perfectly black (case 1379185). #6311

* Fixed missing information in the tooltip of affects smooth surfaces of the ray traced reflections denoiser (case 1376918). #6321

* Physically Based Sky documentation now mentions the warmup cost explicitly (#6323)

* Physically Based Sky documentation now mentions the warmup cost explicitly.

* Update CHANGELOG.md

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Reviewed PR #6031 (#6340)

* Reviewed PR #6031

Also generally improved this doc, fixed typos and added screenshots.

* Apply formatting changes

Co-authored-by: noreply@unity3d.com <noreply@unity3d.com>

* Fix preview scene objects marked dirty by migration #6361

* [HDRP][Path Tracing] Fixed PS5 build compilation warnings #6362

* Fix compil issue

* Apply formatting changes

* Update VisualEnvironment.cs

* Fix HDRP warning (#6396)

* [HDRP][Metal]

* Update reference screenshots sky

Co-authored-by: anisunity <42026998+anisunity@users.noreply.github.com>
Co-authored-by: Adrien de Tocqueville <adrien.tocqueville@unity3d.com>
Co-authored-by: Antoine Lelievre <antoinel@unity3d.com>
Co-authored-by: Kleber Garcia <kleber.garcia@unity3d.com>
Co-authored-by: skhiat <55133890+skhiat@users.noreply.github.com>
Co-authored-by: JulienIgnace-Unity <julien@unity3d.com>
Co-authored-by: Vic Cooper <63712500+Vic-Cooper@users.noreply.github.com>
Co-authored-by: noreply@unity3d.com <noreply@unity3d.com>
Co-authored-by: Arttu Peltonen <77337829+arttu-peltonen@users.noreply.github.com>
Co-authored-by: Emmanuel Turquin <emmanuel@turquin.org>
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.

5 participants