This follows the guidance in T90747 for setting up the drag/drop area
and updating the scene.
Tested by dragging/dropping several different Environment assets onto
the viewport while Cycles rendered mode was active.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- fixT90747-asset_world (branched from master)
- Build Status
Buildable 17177 Build 17177: arc lint + arc unit
Event Timeline
| source/blender/editors/space_view3d/view3d_edit.c | ||
|---|---|---|
| 4886 | I'm unsure if this naked assignment is correct. | |
Comment Actions
Thanks for the patch!
Implementation looks straight forward.
I added @Julian Eisel (Severin) as reviewer as he is more familiar with UI related considerations.
I have been noted that you should not use Fix... in the title when adding new functionality, only when solving bugs. (I also did this in the past). You should use the area where the change is in. For here I would use Asset Browser: ...
| source/blender/editors/space_view3d/view3d_edit.c | ||
|---|---|---|
| 4886 | Seems ok, RNA doesn't have additional logic for world. | |
| 4903 | Would be more specific on the name and consistent with the other drop operations. Same for name + description. Note that you're not changing the world of the 3d view, but of the scene. | |
Comment Actions
Ah, indeed, undo support. Undo seems to work well and I was able to remove 1 of the notifications. I needed to keep 2 of them though:
| source/blender/editors/space_view3d/view3d_edit.c | ||
|---|---|---|
| 4892 | The NC_SPACE | ND_SPACE_VIEW3D notification is still needed if you drop a World while in Eevee rendered or material preview mode. Without it you have to move the viewport in order to see the change. | |
| source/blender/editors/space_view3d/view3d_edit.c | ||
|---|---|---|
| 4892 | In that case it's better to make sure the 3D View updates correctly for NC_SCENE | ND_WORLD. This if block probably needs correction: https://developer.blender.org/diffusion/B/browse/master/source/blender/editors/space_view3d/space_view3d.c$1559-1561 The design idea behind notifiers is that a notifier is sent indicating what changed, and editors can respond with a redraw if they care about this change. By sending a specific space notifier it's basically the wrong way around: the data change explicitly says which editor should redraw. So space notifiers should only be sent if the actual space data changed. | |
Comment Actions
Alright, this version now uses just 1 notification and has things respond appropriately.
The only drawback is that this is now the 3rd instance of the use_scene_world logic I could find in the codebase.
A separate cleanup patch could possible look like the following (had difficulty knowing where to place the common methods): P2429
Comment Actions
Great!
P2429 looks like a good thing to do, could you create a separate patch for this, tagging the EEVEE & Viewport module?