LibOverride: Cleanup unused&missing data after resync. & Blendfile Reading: delay reporting missing linked data after liboverrides resync. #118577

Bastien Montagne · 2024-02-21T17:28:56+01:00

Bastien Montagne commented

2024-02-21 17:28:56 +01:00

LibOverride: Cleanup unused&missing data after resync.

Often when the reference linked data is significantly modified, a lot of
ghost' linked data remain referenced by liboverrides, even after
resync. This is due to the fact that missing data is ignored (skipped)
during resync process, to avoid potential destruction of data in case
the linked data is actually missing.

However, after all resync has been done, we can consider that missing
linked references and their liboverrides can be safely deleted, if the
later are not user-edited or hierarchy roots.

Blendfile Reading: delay reporting missing linked data after liboverrides resync.

When an asset is heavily modified, all production files having
liboverrides of it will still try to link all their known linked
reference IDs, leading to potentially thousands of not-really-useful
warnings about missing IDs in the console.

Now that liboverrides resync cleans up better these left-over data, it's
better to report missing linked data after the liboverride resync
process.

Note that the original place can still report all effectively missing
linked data if needed, but this is now a logging info, so it won't be
displayed anywhere unless explicitely requested.

### LibOverride: Cleanup unused&missing data after resync. Often when the reference linked data is significantly modified, a lot of ghost' linked data remain referenced by liboverrides, even after resync. This is due to the fact that missing data is ignored (skipped) during resync process, to avoid potential destruction of data in case the linked data is actually missing. However, after all resync has been done, we can consider that missing linked references and their liboverrides can be safely deleted, if the later are not user-edited or hierarchy roots. ### Blendfile Reading: delay reporting missing linked data after liboverrides resync. When an asset is heavily modified, all production files having liboverrides of it will still try to link all their known linked reference IDs, leading to potentially thousands of not-really-useful warnings about missing IDs in the console. Now that liboverrides resync cleans up better these left-over data, it's better to report missing linked data _after_ the liboverride resync process. Note that the original place can still report all effectively missing linked data if needed, but this is now a logging info, so it won't be displayed anywhere unless explicitely requested.

Bastien Montagne added 2 commits 2024-02-21 17:29:07 +01:00

bcacd15c9a LibOverride: Cleanup unused&missing data after resync.

Often when the reference linked data is significantly modified, a lot of
'ghost' linked data remain referenced by liboverrides, even after
resync. This is due to the fact that missing data is ignored (skipped)
during resync process, to avoid potential destruction of data in case
the linked data is actually missing.

However, after all resync has been done, we can consider that missing
linked references and their liboverrides can be safely deleted, if the
later are not user-edited or hierarchy roots.

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

42a36c2efd Blendfile Reading: delay reporting missing linked data after liboverrides resync.

When an asset is heavily modified, all production files having
liboverrides of it will still try to link all their known linked
reference IDs, leading to potentially thousands of not-really-useful
warnings about missing IDs in the console.

Now that liboverrides resync cleans up better these left-over data, it's
better to report missing linked data _after_ the liboverride resync
process.

Note that the original place can still report all effectively missing
linked data if needed, but this is now a logging info, so it won't be
displayed anywhere unless explicitely requested.

Bastien Montagne commented

2024-02-21 17:29:25 +01:00

@blender-bot build

Bastien Montagne commented

2024-02-21 17:31:42 +01:00

@SimonThommes This reduces the reports of missing data to something like 11 IDs , when opening 100_0020-lighting.blend from SVN rev 1846.

But it's somewhat hard to be fully sure that it's not clearing too much data in cases where it should not, so some more testing would be very appreciated :)

@SimonThommes This reduces the reports of missing data to something like 11 IDs , when opening `100_0020-lighting.blend` from SVN rev `1846`. But it's somewhat hard to be fully sure that it's not clearing too much data in cases where it should not, so some more testing would be very appreciated :)

👍 1

Simon Thommes commented

2024-02-21 18:23:59 +01:00

Haven't done more testing yet, but just opened a shot and the fact that there are still missing files reported and these are relevant ones is a good sign!

Bastien Montagne commented

2024-02-22 10:46:37 +01:00

@SimonThommes the main risk I see in this change is the liboverride resync code being over-enthusiastic in cleaning up missing data, leading to actual loss of data when the library gets actually missing (as opposed to when the linked library data change is on purpose).

I tried to mitigate this with some checks regarding the root of the liboverride hierarchy (i.e. typically the single top-level collection), but... hard to be 100% sure. :)

There is always a tradeoff between safety (not losing valid data) and cleanup (removing useless data).

@SimonThommes the main risk I see in this change is the liboverride resync code being over-enthusiastic in cleaning up missing data, leading to actual loss of data when the library gets actually missing (as opposed to when the linked library data change is on purpose). I tried to mitigate this with some checks regarding the root of the liboverride hierarchy (i.e. typically the single top-level collection), but... hard to be 100% sure. :) There is always a tradeoff between safety (not losing valid data) and cleanup (removing useless data).

Demeter Dzadik commented

2024-02-22 12:18:51 +01:00

I'm not finding the build under https://builder.blender.org/download/patch/ and it's been a while since I made a build for myself. Happy to help test if needed, if buildbot would help me out. Lemme try, although idk if it listens to me or only devs...

Demeter Dzadik commented

2024-02-22 12:19:37 +01:00

@blender-bot package

Blender Bot commented

2024-02-22 12:20:33 +01:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR118577) when ready.

Demeter Dzadik commented

2024-02-26 12:53:43 +01:00

TL;DR: Patch makes it so loud warnings only happen when stuff genuinely needs attention. Huge improvement for troubleshooting, and it's looking pretty good.

Tested on Mika character:

Scenario 1: No local modifications or reference to the linked data + Deletion

Link and override Mika.blend (library file) -> anim.blend, then link that into lighting.blend, to simulate our production.
Delete stuff from Mika.blend
When opening lighting.blend, we expect no warnings, since there were no local modifications or references to the deleted stuff, no user action is required for everything to return to a good clean state after auto-resync on file open.
On Master, it complains about all the deleted stuff, and with the patch it doesn't. Perfect! 👍

Scenario 2: Local reference to the linked data + Deletion

Create a local object in lighting.blend and reference a linked material from Mika.blend
Delete the material from Mika.blend
When opening lighting.blend, we expect 1 (pretty loud) warning, since this reference can't be automatically fixed and requires user to resolve.
On Master, our 1 useful warning is lost in the crowd of non-useful ones (about the previous deletions) and with the patch we get exactly 1 warning, perfect. 👍

Scenario 3: Add modifier to overridden obj + Deletion

Override the asset in lighting.blend (it is now doubly overridden - we tend to avoid this but might as well test since recursive resync is expected to work the same as single-level resync)
Add a modifier
Delete the object from the library
We expect either a giga loud warning that user-inputted data got lost, or some magical way of preserving the user-inputted data.
Modifier is lost, and we get: Warning: During resync of data-block output, 1 obsolete overrides were deleted, that had local changes defined by user

Request:
If at all possible, scenario 3 should print the names of the overridden IDs that were deleted.
And in the future, it would be great if this was a persistent message in the UI that the user has to manually and explicitly dismiss. Right now, even Blender's loudest warnings are very very shy, and run away into hiding as soon as you move your mouse.

Questions:

IIRC @mont29 you mentioned IRL that there are some cases similar to scenario 3, where the user-inputted data doesn't get lost? I guess this would mean if some data gets "un-deleted", then local modifications have a chance to come back to life. But that seems complex, is there really a case where that happens? It's not that important, just wondering if I understood that correctly.
This may be slightly off topic, but renaming IDs seems to act the same as deleting in all my scenarios. We are definitely trying to avoid renaming IDs during production, but I thought something like scenario 2 would allow renaming, since we're using a pointer to the linked ID, and I thought that magically survives across files. Is that not a thing?

TL;DR: Patch makes it so loud warnings only happen when stuff genuinely needs attention. Huge improvement for troubleshooting, and it's looking pretty good. Tested on Mika character: **Scenario 1: No local modifications or reference to the linked data + Deletion** - Link and override Mika.blend (library file) -> anim.blend, then link that into lighting.blend, to simulate our production. - Delete stuff from Mika.blend - When opening lighting.blend, we expect no warnings, since there were no local modifications or references to the deleted stuff, no user action is required for everything to return to a good clean state after auto-resync on file open. - On Master, it complains about all the deleted stuff, and with the patch it doesn't. Perfect! 👍 **Scenario 2: Local reference to the linked data + Deletion** - Create a local object in lighting.blend and reference a linked material from Mika.blend - Delete the material from Mika.blend - When opening lighting.blend, we expect 1 (pretty loud) warning, since this reference can't be automatically fixed and requires user to resolve. - On Master, our 1 useful warning is lost in the crowd of non-useful ones (about the previous deletions) and with the patch we get exactly 1 warning, perfect. 👍 **Scenario 3: Add modifier to overridden obj + Deletion** - Override the asset in lighting.blend (it is now doubly overridden - we tend to avoid this but might as well test since recursive resync is expected to work the same as single-level resync) - Add a modifier - Delete the object from the library - We expect either a giga loud warning that user-inputted data got lost, or some magical way of preserving the user-inputted data. - Modifier is lost, and we get: `Warning: During resync of data-block output, 1 obsolete overrides were deleted, that had local changes defined by user` -------------------------- **Request**: If at all possible, scenario 3 should print the names of the overridden IDs that were deleted. And in the future, it would be great if this was a persistent message in the UI that the user has to manually and explicitly dismiss. Right now, even Blender's loudest warnings are very very shy, and run away into hiding as soon as you move your mouse. --------------------------- **Questions**: - IIRC @mont29 you mentioned IRL that there are some cases similar to scenario 3, where the user-inputted data doesn't get lost? I guess this would mean if some data gets "un-deleted", then local modifications have a chance to come back to life. But that seems complex, is there really a case where that happens? It's not that important, just wondering if I understood that correctly. - This may be slightly off topic, but renaming IDs seems to act the same as deleting in all my scenarios. We are definitely trying to avoid renaming IDs during production, but I thought something like scenario 2 would allow renaming, since we're using a pointer to the linked ID, and I thought that magically survives across files. Is that not a thing?

Bastien Montagne commented

2024-02-26 14:21:03 +01:00

Thanks for the extensive testing!

Regarding testing three, this is done to avoid too many/long messages in the UI. All such 'modified yet deleted' overrides are listed in warning CLOG messages, which should appear in the console?

Technically, we could keep these 'modified without reference' overrides around. In fact, it was the behavior initially, but it then leaves waaaayyyyy too much trash data in the files, since there is no real way to clean them up automatically afterwards. What should be preserved is in cases where e.g. the whole override hierarchy (including its root, i.e. the main collection) gets missing.

As for renaming, there is no way to handle that currently. It would require some form of UUID for IDs, which is a fairly controversial topic still afaik (and not trivial to implement). Currently, from Blender PoV, ID names are their UUID (together with the type and library), so renaming an ID is just like changing its UUID: it becomes a new ID. While this can be worked around within a blendfile editing session, when it comes to linking, it's just impossible to fix, unless using external pipeline tooling to do some name remapping.

Thanks for the extensive testing! Regarding testing three, this is done to avoid too many/long messages in the UI. All such 'modified yet deleted' overrides are listed in `warning` CLOG messages, which should appear in the console? Technically, we could keep these 'modified without reference' overrides around. In fact, it was the behavior initially, but it then leaves waaaayyyyy too much trash data in the files, since there is no real way to clean them up automatically afterwards. What should be preserved is in cases where e.g. the whole override hierarchy (including its root, i.e. the main collection) gets missing. As for renaming, there is no way to handle that currently. It would require some form of UUID for IDs, which is a fairly controversial topic still afaik (and not trivial to implement). Currently, from Blender PoV, ID names _are_ their UUID (together with the type and library), so renaming an ID is just like changing its UUID: it becomes a new ID. While this can be worked around within a blendfile editing session, when it comes to linking, it's just impossible to fix, unless using external pipeline tooling to do some name remapping.

Demeter Dzadik commented

2024-02-26 15:42:55 +01:00

Regarding testing three, this is done to avoid too many/long messages in the UI. All such 'modified yet deleted' overrides are listed in warning CLOG messages, which should appear in the console?

I don't know what CLOG messages are, but they aren't showing up in my terminal window that's running Blender afaict. Only the message about how many user-modified overrides were deleted, but I don't see it listing the individual names of those IDs, which I think is important. So if we can get that, this LGTM.

we could keep these 'modified without reference' overrides around

Nah, I think it's okay to delete them, as long as there's a loud warning about it. A case that I can see is that a character gets overridden, and Lattice modifiers are added to deform it on a per-shot basis. Then the character could have some clothing pieces removed from its design, so it makes sense to discard the corresponding lattice modifiers. And the warning is an attempt to let the user know that their lattice modifier is gone because the object is "gone" - which is especially useful to reduce confusion when the object is not REALLY gone, it might've just gotten renamed, but we have no way to know that, making the loud warning necessary.

whole override hierarchy goes missing

Gotcha, I see what you meant now, tested that now by mangling a library path with Python and then fixing it, and indeed this works well too. I guess this is not related to the patch though.

renaming would require some form of UUID for IDs

Ah yeah, that's what I thought, at some point in the past few weeks I think I misunderstood something, hence my confusion here, but that clears things up.

> Regarding testing three, this is done to avoid too many/long messages in the UI. All such 'modified yet deleted' overrides are listed in warning CLOG messages, which should appear in the console? I don't know what CLOG messages are, but they aren't showing up in my terminal window that's running Blender afaict. Only the message about how many user-modified overrides were deleted, but I don't see it listing the individual names of those IDs, which I think is important. So if we can get that, this LGTM. -------------------------- > we could keep these 'modified without reference' overrides around Nah, I think it's okay to delete them, as long as there's a loud warning about it. A case that I can see is that a character gets overridden, and Lattice modifiers are added to deform it on a per-shot basis. Then the character could have some clothing pieces removed from its design, so it makes sense to discard the corresponding lattice modifiers. And the warning is an attempt to let the user know that their lattice modifier is gone because the object is "gone" - which is especially useful to reduce confusion when the object is not REALLY gone, it might've just gotten renamed, but we have no way to know that, making the loud warning necessary. > whole override hierarchy goes missing Gotcha, I see what you meant now, tested that now by mangling a library path with Python and then fixing it, and indeed this works well too. I guess this is not related to the patch though. > renaming would require some form of UUID for IDs Ah yeah, that's what I thought, at some point in the past few weeks I think I misunderstood something, hence my confusion here, but that clears things up.

Bastien Montagne commented

2024-02-26 17:21:42 +01:00

Regarding testing three, this is done to avoid too many/long messages in the UI. All such 'modified yet deleted' overrides are listed in warning CLOG messages, which should appear in the console?

I don't know what CLOG messages are, but they aren't showing up in my terminal window that's running Blender afaict. Only the message about how many user-modified overrides were deleted, but I don't see it listing the individual names of those IDs, which I think is important. So if we can get that, this LGTM.

Just checked, in my debug and release builds, and official release builds, I'll get these kind of messages in the system console:

WARN (bke.liboverride_resync): source/blender/blenkernel/intern/lib_override.cc:2597 lib_override_library_resync: Old override OBCube is being deleted even though it was user-edited

> > Regarding testing three, this is done to avoid too many/long messages in the UI. All such 'modified yet deleted' overrides are listed in warning CLOG messages, which should appear in the console? > > I don't know what CLOG messages are, but they aren't showing up in my terminal window that's running Blender afaict. Only the message about how many user-modified overrides were deleted, but I don't see it listing the individual names of those IDs, which I think is important. So if we can get that, this LGTM. Just checked, in my debug and release builds, and official release builds, I'll get these kind of messages in the system console: `WARN (bke.liboverride_resync): source/blender/blenkernel/intern/lib_override.cc:2597 lib_override_library_resync: Old override OBCube is being deleted even though it was user-edited`

Demeter Dzadik commented

2024-02-27 14:56:05 +01:00

Sorry, I think I missed it because it blended in with these other mysterious and un-useful warnings from some anim code in my test file.

Push it! :D

Sorry, I think I missed it because it blended in with these other mysterious and un-useful warnings from some anim code in my test file. ![image](/attachments/70a318ec-1c7b-479f-88dc-a75f974d36da) Push it! :D

image.png

29 KiB

Bastien Montagne merged commit 118caa7b45 into main

2024-02-27 16:43:50 +01:00

Bastien Montagne referenced this issue from a commit

2024-02-27 16:43:50 +01:00

Blendfile Reading: delay reporting missing linked data after liboverrides resync.

Bastien Montagne deleted branch tmp-missing-linked-data

2024-02-27 16:43:52 +01:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

LibOverride: Cleanup unused&missing data after resync. & Blendfile Reading: delay reporting missing linked data after liboverrides resync. #118577

LibOverride: Cleanup unused&missing data after resync.

Blendfile Reading: delay reporting missing linked data after liboverrides resync.