Fix preview scene objects marked dirty by migration by arttu-peltonen · Pull Request #6361 · Unity-Technologies/Graphics · GitHub
Skip to content

Fix preview scene objects marked dirty by migration #6361

Merged
JulienIgnace-Unity merged 4 commits into
masterfrom
rpw/bugfix/fix-preview-scene-dirtying
Nov 25, 2021
Merged

Fix preview scene objects marked dirty by migration #6361
JulienIgnace-Unity merged 4 commits into
masterfrom
rpw/bugfix/fix-preview-scene-dirtying

Conversation

@arttu-peltonen

Copy link
Copy Markdown
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

github-actions Bot commented Nov 22, 2021

Copy link
Copy Markdown

@TomasKiniulis TomasKiniulis left a comment

Copy link
Copy Markdown
Contributor

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.

@RSlysz RSlysz left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor Author

@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 a review from a team as a code owner November 25, 2021 11:20
@arttu-peltonen arttu-peltonen requested a review from a team 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