Dragging Material from Asset Browser (Link mode) to Viewport empty space removes this material from all objects #95636

New Issue

Johannes Kollmer · 2022-02-09T10:36:34+01:00

Johannes Kollmer commented

2022-02-09 10:36:34 +01:00

System Information
Operating system: Windows-10-10.0.19043-SP0 64 Bits
Graphics card: NVIDIA GeForce RTX 2080 SUPER/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 497.29

Blender Version
Broken: version: 3.0.1, branch: master, commit date: 2022-01-25 17:19, hash: dc2d180181
Also broken in: version: 3.0

Short description of error
Dragging a Material from the Asset browser to empty Space in Viewport removes the Material from all objects in the scene

Exact steps for others to reproduce the error

Create a couple of cubes
Drag a material from the asset browser to the objects
Drag the same material to empty space in the 3d Viewport
rotate the viewport
The cubes now have no material anymore.

**System Information** Operating system: Windows-10-10.0.19043-SP0 64 Bits Graphics card: NVIDIA GeForce RTX 2080 SUPER/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 497.29 **Blender Version** Broken: version: 3.0.1, branch: master, commit date: 2022-01-25 17:19, hash: `dc2d180181` Also broken in: version: 3.0 **Short description of error** Dragging a Material from the Asset browser to empty Space in Viewport removes the Material from all objects in the scene **Exact steps for others to reproduce the error** 1. Create a couple of cubes 2. Drag a material from the asset browser to the objects 3. Drag the same material to empty space in the 3d Viewport 4. rotate the viewport 5. The cubes now have no material anymore.

Johannes Kollmer commented

2022-02-09 10:36:34 +01:00

Added subscriber: @Lumpengnom-3

Philipp Oeser commented

2022-02-09 12:33:31 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2022-02-09 12:33:31 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser commented

2022-02-09 12:33:31 +01:00

Can confirm (but only in Link mode).

Also getting this (which might not be related, this seems correct):

WARN (blo.readfile): source/blender/blenloader/intern/readfile.c:4390 link_named_part: Append: ID 'MAred' is already linked

Can confirm (but only in `Link` mode). Also getting this (which might not be related, this seems correct): ``` WARN (blo.readfile): source/blender/blenloader/intern/readfile.c:4390 link_named_part: Append: ID 'MAred' is already linked ```

Philipp Oeser commented

2022-02-09 12:35:52 +01:00

Will set this to High prio (even though this is not a regression, seems like it never worked)

Philipp Oeser changed title from ~~Dragging Material from Asset Browser to Viewport removes this material from all objects~~ to Dragging Material from Asset Browser (Link mode) to Viewport empty space removes this material from all objects

2022-02-09 12:37:10 +01:00

Johannes Kollmer commented

2022-02-09 20:07:00 +01:00

Here it happens in "link mode" and also in "Append (Reuse Data)" Mode. Not in "Append" mode, though.

Vladimir commented

2022-02-10 21:53:53 +01:00

Added subscriber: @Wovchick

Sybren A. Stüvel self-assigned this 2022-02-21 14:37:14 +01:00

Sybren A. Stüvel removed their assignment 2022-02-21 15:05:31 +01:00

Sybren A. Stüvel commented

2022-02-21 15:05:31 +01:00

Added subscribers: @mont29, @dr.sybren

Sybren A. Stüvel commented

2022-02-21 15:05:31 +01:00

This seems to have something to do with the way the material is imported:

BKE_id_free_ex(Main * bmain, void * idv, int flag, const _Bool use_flag_from_idtag) (blender/source/blender/blenkernel/intern/lib_id_delete.c:95)
id_delete(Main * bmain, const _Bool do_tagged_deletion) (blender/source/blender/blenkernel/intern/lib_id_delete.c:352)
BKE_id_multi_tagged_delete(Main * bmain) (blender/source/blender/blenkernel/intern/lib_id_delete.c:375)
BKE_blendfile_append(BlendfileLinkAppendContext * lapp_context, ReportList * reports) (blender/source/blender/blenkernel/intern/blendfile_link_append.c:1188)
wm_file_link_append_datablock_ex(Main * bmain, Scene * scene, ViewLayer * view_layer, View3D * v3d, const char * filepath, const short id_code, const char * id_name, const int flag) (blender/source/blender/windowmanager/intern/wm_files_link.c:543)
WM_file_append_datablock(Main * bmain, Scene * scene, ViewLayer * view_layer, View3D * v3d, const char * filepath, const short id_code, const char * id_name, int flag) (blender/source/blender/windowmanager/intern/wm_files_link.c:580)
WM_drag_asset_id_import(wmDragAsset * asset_drag, const int flag_extra) (blender/source/blender/windowmanager/intern/wm_dragdrop.c:559)
WM_drag_get_local_ID_or_import_from_asset(const wmDrag * drag, int idcode) (blender/source/blender/windowmanager/intern/wm_dragdrop.c:601)
view3d_id_drop_copy(wmDrag * drag, wmDropBox * drop) (blender/source/blender/editors/space_view3d/space_view3d.c:773)
wm_drop_prepare(bContext * C, wmDrag * drag, wmDropBox * drop) (blender/source/blender/windowmanager/intern/wm_dragdrop.c:394)
wm_handlers_do_intern(bContext * C, wmWindow * win, wmEvent * event, ListBase * handlers) (blender/source/blender/windowmanager/intern/wm_event_system.c:3053)
wm_handlers_do(bContext * C, wmEvent * event, ListBase * handlers) (blender/source/blender/windowmanager/intern/wm_event_system.c:3159)
wm_event_do_region_handlers(bContext * C, wmEvent * event, ARegion * region) (blender/source/blender/windowmanager/intern/wm_event_system.c:3530)
wm_event_do_handlers_area_regions(bContext * C, wmEvent * event, ScrArea * area) (blender/source/blender/windowmanager/intern/wm_event_system.c:3560)
wm_event_do_handlers(bContext * C) (blender/source/blender/windowmanager/intern/wm_event_system.c:3745)
WM_main(bContext * C) (blender/source/blender/windowmanager/intern/wm.c:639)
main(int argc, const char ** argv) (blender/source/creator/creator.c:560)

This requires some more knowledge than I have of how the "append/link on drag" and "use on drop" collaborate with each other. @mont29 do you know what could be the root cause here? I'm guessing there is an implicit assumption somewhere in the code that a dragged-and-dropped asset is actually going to be linked to something, and if that isn't the case, it'll be deleted. In this case, it means that a pre-existing material is deleted, which is not supposed to happen.

This seems to have something to do with the way the material is imported: ``` BKE_id_free_ex(Main * bmain, void * idv, int flag, const _Bool use_flag_from_idtag) (blender/source/blender/blenkernel/intern/lib_id_delete.c:95) id_delete(Main * bmain, const _Bool do_tagged_deletion) (blender/source/blender/blenkernel/intern/lib_id_delete.c:352) BKE_id_multi_tagged_delete(Main * bmain) (blender/source/blender/blenkernel/intern/lib_id_delete.c:375) BKE_blendfile_append(BlendfileLinkAppendContext * lapp_context, ReportList * reports) (blender/source/blender/blenkernel/intern/blendfile_link_append.c:1188) wm_file_link_append_datablock_ex(Main * bmain, Scene * scene, ViewLayer * view_layer, View3D * v3d, const char * filepath, const short id_code, const char * id_name, const int flag) (blender/source/blender/windowmanager/intern/wm_files_link.c:543) WM_file_append_datablock(Main * bmain, Scene * scene, ViewLayer * view_layer, View3D * v3d, const char * filepath, const short id_code, const char * id_name, int flag) (blender/source/blender/windowmanager/intern/wm_files_link.c:580) WM_drag_asset_id_import(wmDragAsset * asset_drag, const int flag_extra) (blender/source/blender/windowmanager/intern/wm_dragdrop.c:559) WM_drag_get_local_ID_or_import_from_asset(const wmDrag * drag, int idcode) (blender/source/blender/windowmanager/intern/wm_dragdrop.c:601) view3d_id_drop_copy(wmDrag * drag, wmDropBox * drop) (blender/source/blender/editors/space_view3d/space_view3d.c:773) wm_drop_prepare(bContext * C, wmDrag * drag, wmDropBox * drop) (blender/source/blender/windowmanager/intern/wm_dragdrop.c:394) wm_handlers_do_intern(bContext * C, wmWindow * win, wmEvent * event, ListBase * handlers) (blender/source/blender/windowmanager/intern/wm_event_system.c:3053) wm_handlers_do(bContext * C, wmEvent * event, ListBase * handlers) (blender/source/blender/windowmanager/intern/wm_event_system.c:3159) wm_event_do_region_handlers(bContext * C, wmEvent * event, ARegion * region) (blender/source/blender/windowmanager/intern/wm_event_system.c:3530) wm_event_do_handlers_area_regions(bContext * C, wmEvent * event, ScrArea * area) (blender/source/blender/windowmanager/intern/wm_event_system.c:3560) wm_event_do_handlers(bContext * C) (blender/source/blender/windowmanager/intern/wm_event_system.c:3745) WM_main(bContext * C) (blender/source/blender/windowmanager/intern/wm.c:639) main(int argc, const char ** argv) (blender/source/creator/creator.c:560) ``` This requires some more knowledge than I have of how the "append/link on drag" and "use on drop" collaborate with each other. @mont29 do you know what could be the root cause here? I'm guessing there is an implicit assumption somewhere in the code that a dragged-and-dropped asset is actually going to be linked to something, and if that isn't the case, it'll be deleted. In this case, it means that a pre-existing material is deleted, which is not supposed to happen.

Bastien Montagne commented

2022-02-21 17:59:47 +01:00

That backtrace is actually not relevant, since that specific deletion is valid (it's deleting the linked version of the material, not the local one, which is expected behavior when re-using an appended ID instead).

Deletion of the local one happens from WM_drag_free_imported_drag_ID, which unconditionally deletes the dragged ID when dropped on no valid target. Fixing this is relatively trivial:

diff --git a/source/blender/windowmanager/intern/wm_dragdrop.c b/source/blender/windowmanager/intern/wm_dragdrop.c
index f69a612df19..685d74f6d20 100644
--- a/source/blender/windowmanager/intern/wm_dragdrop.c
+++ b/source/blender/windowmanager/intern/wm_dragdrop.c
@@ -651,8 +651,12 @@ void WM_drag_free_imported_drag_ID(struct Main *bmain, wmDrag *drag, wmDropBox *
   }
 
   ID *id = BKE_libblock_find_name(bmain, asset_drag->id_type, name);
-  if (id) {
-    BKE_id_delete(bmain, id);
+  if (id != NULL) {
+    /* Do not delete the dragged ID if it has any user, otherwise if it is a 're-used' ID it will
+     * cause T95636. Note that we need first to add the user that we want to remove in
+     * #BKE_id_free_us. */
+    id_us_plus(id);
+    BKE_id_free_us(bmain, id);
   }
 }

Unfortunately, while digging a bit more, I found that there are a lot more issues when you start mixing Link and Append modes from the asset browser, e.g. once a material has been appended, it becomes impossible to get it linked using the drag'n'drop feature... This needs more investigation.

Another question raised here is what happens to indirectly linked/appended data (like nodetrees for a material, geometry and materials for an objects, etc.). Afaict currently, those will remain around as orphaned data, which is also a fairly bad behavior.

That backtrace is actually not relevant, since that specific deletion is valid (it's deleting the linked version of the material, not the local one, which is expected behavior when re-using an appended ID instead). Deletion of the local one happens from `WM_drag_free_imported_drag_ID`, which unconditionally deletes the dragged ID when dropped on no valid target. Fixing this is relatively trivial: ``` diff --git a/source/blender/windowmanager/intern/wm_dragdrop.c b/source/blender/windowmanager/intern/wm_dragdrop.c index f69a612df19..685d74f6d20 100644 --- a/source/blender/windowmanager/intern/wm_dragdrop.c +++ b/source/blender/windowmanager/intern/wm_dragdrop.c @@ -651,8 +651,12 @@ void WM_drag_free_imported_drag_ID(struct Main *bmain, wmDrag *drag, wmDropBox * } ID *id = BKE_libblock_find_name(bmain, asset_drag->id_type, name); - if (id) { - BKE_id_delete(bmain, id); + if (id != NULL) { + /* Do not delete the dragged ID if it has any user, otherwise if it is a 're-used' ID it will + * cause T95636. Note that we need first to add the user that we want to remove in + * #BKE_id_free_us. */ + id_us_plus(id); + BKE_id_free_us(bmain, id); } } ``` Unfortunately, while digging a bit more, I found that there are a lot more issues when you start mixing `Link` and `Append` modes from the asset browser, e.g. once a material has been appended, it becomes impossible to get it linked using the drag'n'drop feature... This needs more investigation. ------------- Another question raised here is what happens to indirectly linked/appended data (like nodetrees for a material, geometry and materials for an objects, etc.). Afaict currently, those will remain around as orphaned data, which is also a fairly bad behavior.

Bastien Montagne commented

2022-02-22 10:16:10 +01:00

Added subscriber: @JulianEisel

Bastien Montagne commented

2022-02-22 10:16:11 +01:00

@JulianEisel if you are fine with proposed fix above, would commit this for 3.1 since this issue is very bad.

And I think we need to create a task to properly handle deletion of 'aborted' dragged data, because right now this is handled in a very poor way.

@JulianEisel if you are fine with proposed fix above, would commit this for 3.1 since this issue is very bad. And I think we need to create a task to properly handle deletion of 'aborted' dragged data, because right now this is handled in a very poor way.

Julian Eisel commented

2022-02-23 00:00:34 +01:00

If you say this fix is what we should do then I'm fine with it of course.

Quite some things changed since this ID freeing for aborting was added, and it wasn't tested a lot since then I think. Now the temporary ID loading code may be a better way to go about it.

If you say this fix is what we should do then I'm fine with it of course. Quite some things changed since this ID freeing for aborting was added, and it wasn't tested a lot since then I think. Now the temporary ID loading code may be a better way to go about it.

blender-admin commented

2022-02-25 11:37:55 +01:00

This issue was referenced by 66328db703

This issue was referenced by 66328db703bba8196f159d15d9632c34d845892d

Bastien Montagne commented

2022-02-25 11:40:30 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Bastien Montagne closed this issue

2022-02-25 11:40:30 +01:00

Bastien Montagne self-assigned this 2022-02-25 11:40:30 +01:00

Thomas Dinges added this to the 3.1 milestone 2023-02-08 15:52:46 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Dragging Material from Asset Browser (Link mode) to Viewport empty space removes this material from all objects #95636