ON HOLD - Assets - Support Editing External BlendFile #117239

New Issue

Bastien Montagne · 2024-01-17T16:44:49+01:00

Bastien Montagne commented

2024-01-17 16:44:49 +01:00

WARNING This design is on hold, for the initial steps of #116337, much simpler handling will be implemented, which should not require any new management code in BKE/assets area.

This task is about the technical design to make it possible to edit external blendfiles, in the Asset Library context. The initial target is to make saving Brush assets possible, as part of the Brush Assets project (see #116337).

NOTE: this is somewhat related to the 'draft' idea from original Brush Asset design (#101942). However, given the shift in current project design, it is better to create a new clean design task.

Design Overview

Blender needs to be able to:

Open a .blend file in a secondary Main, different than the current active one.
Manipulate data-blocks in that secondary Main (on a basic level: Move IDs between Mains, add/delete IDs in secondary main, ...). Including dependencies
Save a secondary Main into a .blend file.

The current design is based on a 'manager' class, which manages the loading, editing and saving of the external asset library blendfile. Most operations require the manager to acquire the library, which effectively read the blendfile in its own temporary bmain. A manager should remain in its acquired state as little as possible, to reduce memory usage and risks of concurrent modifications.

Identified challenges so far:

Saving to the other asset library blendfile should not overwrite silently some potential concurrent editing done by another process.
Handling of data-blocks collisions and updates of existing assets in the library.
Performances and optimization (when editing heavy libraries).

Concurrent Accesses

The proposed solution is to check for external modifications of the target library file, and loop until the whole writing process can be done without conflicts. The data in the library blendfile should always have precedence over the modifications currently being saved.

graph TD;
    1[1: Store modification time of the target asset library file]-->2;
    2[2: Perform writing into a temp file]-->3;
    3[3: Has target file modification time changed?]-->4;
    3-->5;
    4[NO: 4: replace it by the temp written file];
    5[YES: 5: Re-load the target file]-->6;
    6[6: Merge the current to-be-written Main into it]-->1;

Step 6 ensures the data re-read from the target file has priority over the data stored into the Main to be written.

NOTE: This process has some potential TOC/TOU issue (since the target file could be modified after its modification time has been stat'ed, but before it is effectively replaced by its newly written version). For now, it is assumed that this situation can be considered as extremely unlikely, and can be ignored in practice.

Collisions and Updates of Existing Library Data

Assets (and even less basic IDs) have no UUID currently, so we have to rely on their names to identify them. Further more, different assets can use the same sub-data (e.g. a texture).

For Assets themselves, we can assume that if they have the same name, they are the same data.

For sub-IDs, it is way more challenging to find a 'correct' way to decide when to replace them, and when to store a new copy instead.

Proposed behavior to handle ID when there are name collisions. it tries to avoid systematic duplication of data in the asset libraries, while ensuring a

Operation	Explicitly Saved Assets	Implicitly Saved Assets	Implicitly Saved Non-Asset IDs
Save Replace or update existing library data.	Existing library assets are replaced by given ones to save.	Re-use existing library assets, do not update them. Do not follow further dependencies from these sub-assets.	If library ID is only used be replaced Assets and their dependencies, replace the library ID. Else, create a new ID in library (with a different name).
Save As Save new library data.	Colliding saved assets are renamed.	Re-use existing library assets, do not update them. Do not follow further dependencies from these sub-assets.	Create a new ID in library (with a different name).

Performances & Optimizations

Having to load a second complete blendfile in memory can become problematic if the library has a lot of data-blocks and/or heavy ones (like complex geometries etc.).

This is likely not an issue in the Brush Assets context though, so it does not have a high priority currently.

Plan of Action

Tasks to get a minimal testable 'save to default asset library' for brush assets.

These tasks are ordered by priority, from highest to lowest.

Finish basics of the low-level 'handler' implementation (BKE area).
- Implement 'Save' and 'Save As' API.
- Add unittests for these.
Add user-level minimal operators and UI to perform these save operations.
Add user preferences for the 'default' assets library.

> _**WARNING**_ This design is on hold, for the initial steps of #116337, much simpler handling will be implemented, which should not require any new management code in BKE/assets area. ----------- This task is about the technical design to make it possible to edit external blendfiles, in the Asset Library context. The initial target is to make saving Brush assets possible, as part of the Brush Assets project (see #116337). > *__NOTE__*: this is somewhat related to the 'draft' idea from original Brush Asset design (#101942). However, given the shift in current project design, it is better to create a new clean design task. ## Design Overview Blender needs to be able to: * Open a .blend file in a secondary Main, different than the current active one. * Manipulate data-blocks in that secondary Main (on a basic level: Move IDs between Mains, add/delete IDs in secondary main, ...). Including dependencies * Save a secondary Main into a .blend file. The current design is based on a 'manager' class, which manages the loading, editing and saving of the external asset library blendfile. Most operations require the manager to acquire the library, which effectively read the blendfile in its own temporary bmain. A manager should remain in its `acquired` state as little as possible, to reduce memory usage and risks of concurrent modifications. Identified challenges so far: * Saving to the other asset library blendfile should not overwrite silently some potential concurrent editing done by another process. * Handling of data-blocks collisions and updates of existing assets in the library. * Performances and optimization (when editing heavy libraries). ### Concurrent Accesses The proposed solution is to check for external modifications of the target library file, and loop until the whole writing process can be done without conflicts. The data in the library blendfile should always have precedence over the modifications currently being saved. ```mermaid graph TD; 1[1: Store modification time of the target asset library file]-->2; 2[2: Perform writing into a temp file]-->3; 3[3: Has target file modification time changed?]-->4; 3-->5; 4[NO: 4: replace it by the temp written file]; 5[YES: 5: Re-load the target file]-->6; 6[6: Merge the current to-be-written Main into it]-->1; ``` _Step 6 ensures the data re-read from the target file has priority over the data stored into the Main to be written._ > *__NOTE__*: This process has some potential TOC/TOU issue (since the target file _could_ be modified after its modification time has been `stat`'ed, but before it is effectively replaced by its newly written version). For now, it is assumed that this situation can be considered as extremely unlikely, and can be ignored in practice. ### Collisions and Updates of Existing Library Data Assets (and even less basic IDs) have no UUID currently, so we have to rely on their names to identify them. Further more, different assets can use the same sub-data (e.g. a texture). For Assets themselves, we can assume that if they have the same name, they are the same data. For sub-IDs, it is way more challenging to find a 'correct' way to decide when to replace them, and when to store a new copy instead. Proposed behavior to handle ID when there are name collisions. it tries to avoid systematic duplication of data in the asset libraries, while ensuring a | Operation | Explicitly Saved Assets | Implicitly Saved Assets | Implicitly Saved Non-Asset IDs | | --------- | ----------------------- | ----------------------- | ------------------------------ | | **Save** _Replace or update existing library data._ | Existing library assets are replaced by given ones to save. | Re-use existing library assets, do not update them. Do not follow further dependencies from these sub-assets. | If library ID is only used be replaced Assets and their dependencies, replace the library ID. Else, create a new ID in library (with a different name). | | **Save As** _Save new library data._ | Colliding saved assets are renamed. | Re-use existing library assets, do not update them. Do not follow further dependencies from these sub-assets. | Create a new ID in library (with a different name). | ### Performances & Optimizations Having to load a second complete blendfile in memory can become problematic if the library has a lot of data-blocks and/or heavy ones (like complex geometries etc.). This is likely not an issue in the Brush Assets context though, so it does not have a high priority currently. ## Plan of Action Tasks to get a minimal testable 'save to default asset library' for brush assets. _These tasks are ordered by priority, from highest to lowest._ * [ ] Finish basics of the low-level 'handler' implementation (BKE area). * [ ] Implement 'Save' and 'Save As' API. * [ ] Add unittests for these. * [ ] Add user-level minimal operators and UI to perform these save operations. * [ ] Add user preferences for the 'default' assets library.

👀 1

Bastien Montagne added the

Type

Design

label 2024-01-17 16:44:49 +01:00

~~Bastien Montagne referenced this issue 2024-01-17 16:46:19 +01:00~~

Brush Assets Tasks #116337

Brecht Van Lommel commented

2024-01-17 17:44:50 +01:00

Concurrent Accesses

I think writing to asset files is always an explicit user action in our current design. It can be "Save Changes" or "Delete" on a brush datablock, or deleting or dropping an asset in the asset browser.

So if there is a conflict, I think we would ask the user if they want to overwrite it? Besides leaving that choice to the user, the steps look fine to me.

Collisions and Updates of Existing Library Data

For collisions, I'm hoping we can save assets to individual asset .blend files as much as possible. And in a first iteration of the design we may enforce that and not allow editing of more complex .blend files, so that we can safely overwrite the whole .blend file contents without conflict resolution.

The proposed logic seems ok to me. Just maybe we don't want to deal with that for a while.

> Concurrent Accesses I think writing to asset files is always an explicit user action in our current design. It can be "Save Changes" or "Delete" on a brush datablock, or deleting or dropping an asset in the asset browser. So if there is a conflict, I think we would ask the user if they want to overwrite it? Besides leaving that choice to the user, the steps look fine to me. > Collisions and Updates of Existing Library Data For collisions, I'm hoping we can save assets to individual asset .blend files as much as possible. And in a first iteration of the design we may enforce that and not allow editing of more complex .blend files, so that we can safely overwrite the whole .blend file contents without conflict resolution. The proposed logic seems ok to me. Just maybe we don't want to deal with that for a while.

Bastien Montagne commented

2024-01-17 18:10:20 +01:00

Concurrent Accesses

I think writing to asset files is always an explicit user action in our current design. It can be "Save Changes" or "Delete" on a brush datablock, or deleting or dropping an asset in the asset browser.

So if there is a conflict, I think we would ask the user if they want to overwrite it? Besides leaving that choice to the user, the steps look fine to me.

Agreed.

Collisions and Updates of Existing Library Data

For collisions, I'm hoping we can save assets to individual asset .blend files as much as possible. And in a first iteration of the design we may enforce that and not allow editing of more complex .blend files, so that we can safely overwrite the whole .blend file contents without conflict resolution.

The proposed logic seems ok to me. Just maybe we don't want to deal with that for a while.

The main issue with one blendfile per brush asset is that there is no sharing of resources possible. Each brush needs its own copy of textures and all the other dependencies.

If we decide to go that way, then I think the whole current 'draft manager' code is useless. We only need to do a 'partial write' of the asset and its dependencies into a new file (or to replace the existing asset source file). There is no need for a temp Main and complex merging process.

Afaik we already have all the BKE/BLO code needed for that, so we mainly have to write some editor level code (operators & UI)? With some extra things to take care of probably:

Keep track of the mtime of the file each loaded brush asset comes from (to allow detecting modified asset files).

> > Concurrent Accesses > > I think writing to asset files is always an explicit user action in our current design. It can be "Save Changes" or "Delete" on a brush datablock, or deleting or dropping an asset in the asset browser. > > So if there is a conflict, I think we would ask the user if they want to overwrite it? Besides leaving that choice to the user, the steps look fine to me. Agreed. > > Collisions and Updates of Existing Library Data > > For collisions, I'm hoping we can save assets to individual asset .blend files as much as possible. And in a first iteration of the design we may enforce that and not allow editing of more complex .blend files, so that we can safely overwrite the whole .blend file contents without conflict resolution. > > The proposed logic seems ok to me. Just maybe we don't want to deal with that for a while. The main issue with one blendfile per brush asset is that there is no sharing of resources possible. Each brush needs its own copy of textures and all the other dependencies. If we decide to go that way, then I think the whole current 'draft manager' code is useless. We only need to do a 'partial write' of the asset and its dependencies into a new file (or to replace the existing asset source file). There is no need for a temp Main and complex merging process. Afaik we already have all the BKE/BLO code needed for that, so we mainly have to write some `editor` level code (operators & UI)? With some extra things to take care of probably: * Keep track of the `mtime` of the file each loaded brush asset comes from (to allow detecting modified asset files).

Brecht Van Lommel commented

2024-01-17 18:25:04 +01:00

I don't think the disk space or memory usage of image textures used by brushes is really a concern for now. Using high res images in brushes is not common I think. Maybe we need a better distinction between textures used to define the shape of the brush and textures used for projection/stencils that are not really part of the brush.

If we can do such individual asset .blend files without adding much code that would be great. The more complex case is interesting to investigate at some point, but I feel like that could sidetrack things too much right now. There are enough other challenges.

There is some UI needed to communicate what happens when you go and open such an individual asset .blend file and start changing things. But I think it could be enough to warn the says and explain that when they do that, it will be changed into a regular .blend that you can't edit remotely anymore.

I don't think the disk space or memory usage of image textures used by brushes is really a concern for now. Using high res images in brushes is not common I think. Maybe we need a better distinction between textures used to define the shape of the brush and textures used for projection/stencils that are not really part of the brush. If we can do such individual asset .blend files without adding much code that would be great. The more complex case is interesting to investigate at some point, but I feel like that could sidetrack things too much right now. There are enough other challenges. There is some UI needed to communicate what happens when you go and open such an individual asset .blend file and start changing things. But I think it could be enough to warn the says and explain that when they do that, it will be changed into a regular .blend that you can't edit remotely anymore.

Bastien Montagne referenced this issue

2024-01-19 19:34:15 +01:00

Report/confirm overwrite of 'asset library' blendfiles. #117346

Bastien Montagne changed title from ~~Assets - Support Editing External BlendFile~~ to ON HOLD - Assets - Support Editing External BlendFile

2024-01-24 14:47:45 +01:00

Bastien Montagne referenced this issue from a commit

2024-01-25 17:10:40 +01:00

Report/confirm overwrite of 'asset library' blendfiles.

Sign in to join this conversation.