UI: Asset Shelf (Experimental Feature) #104831

Closed
Julian Eisel wants to merge 399 commits from asset-shelf into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
9 changed files with 20 additions and 13 deletions
Showing only changes of commit 8c584cb7ae - Show all commits

View File

@ -1,4 +1,7 @@
# SPDX-FileCopyrightText: 2023 Blender Foundation
JulianEisel marked this conversation as resolved Outdated

Use updated copyright convention SPDX-FileCopyrightText.

# SPDX-FileCopyrightText: 2023 Blender Foundation
#
# SPDX-License-Identifier: GPL-2.0-or-later
Use updated copyright convention `SPDX-FileCopyrightText`. ``` # SPDX-FileCopyrightText: 2023 Blender Foundation # # SPDX-License-Identifier: GPL-2.0-or-later ```
#
# SPDX-License-Identifier: GPL-2.0-or-later
import bpy
from bpy.types import (
Panel,

View File

@ -1276,7 +1276,7 @@ class VIEW3D_MT_view(Menu):
layout.prop(view, "show_region_ui")
layout.prop(view, "show_region_tool_header")
if prefs.experimental.use_asset_shelf:
layout.prop(view, "show_regions_asset_shelf")
layout.prop(view, "show_region_asset_shelf")

Suggestion: it would be nice if this was greyed out when the asset menu isn't shown, when first testing this patch I found it confusing that toggling the region did nothing until entering sculpt mode.

Suggestion: it would be nice if this was greyed out when the asset menu isn't shown, when first testing this patch I found it confusing that toggling the region did nothing until entering sculpt mode.

Submitted as separate PR now since I'd like to get some feedback without delaying the initial asset shelf merge: #110756.

Submitted as separate PR now since I'd like to get some feedback without delaying the initial asset shelf merge: #110756.
layout.prop(view, "show_region_hud")
layout.separator()

View File

@ -422,7 +422,7 @@ typedef struct Menu {
typedef enum AssetShelfTypeFlag {
/** Do not trigger asset dragging on drag events. Drag events can be overridden with custom
* keymap items then. */
ASSET_SHELF_TYPE_NO_ASSET_DRAG = (1 << 0),
ASSET_SHELF_TYPE_FLAG_NO_ASSET_DRAG = (1 << 0),
JulianEisel marked this conversation as resolved Outdated

Suggestion: calling ASSET_SHELF_TYPE_* prefix reads like a type which are typically unique values, not flags. ASSET_SHELF_TYPE_FLAG_NO_ASSET_DRAG - some extra clarity at the expense of verbosity.

Suggestion: calling `ASSET_SHELF_TYPE_*` prefix reads like a type which are typically unique values, not flags. `ASSET_SHELF_TYPE_FLAG_NO_ASSET_DRAG` - some extra clarity at the expense of verbosity.
ASSET_SHELF_TYPE_FLAG_MAX
} AssetShelfTypeFlag;

View File

@ -52,8 +52,8 @@
#include "BLO_read_write.h"
/* TODO(Julian): For asset shelf region reading/writing. Region read/write should be done via a
* #ARegionType callback. */
/* TODO(@JulianEisel): For asset shelf region reading/writing. Region read/write should be done via
JulianEisel marked this conversation as resolved Outdated

picky use TODO(@username) format.

*picky* use `TODO(@username)` format.
* a #ARegionType callback. */
#include "../editors/asset/ED_asset_shelf.h"
#ifdef WITH_PYTHON

View File

@ -1,4 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
/* SPDX-FileCopyrightText: 2023 Blender Foundation
JulianEisel marked this conversation as resolved Outdated

Use updated copyright format

/* SPDX-FileCopyrightText: 2023 Blender Foundation
 *
 * SPDX-License-Identifier: GPL-2.0-or-later */
Use updated copyright format ``` /* SPDX-FileCopyrightText: 2023 Blender Foundation * * SPDX-License-Identifier: GPL-2.0-or-later */ ```
*
* SPDX-License-Identifier: GPL-2.0-or-later */
/** \file
* \ingroup edasset
@ -99,7 +101,8 @@ static AssetShelf *create_shelf_from_type(AssetShelfType &type)
static void activate_shelf(RegionAssetShelf &shelf_regiondata, AssetShelf &shelf)
{
JulianEisel marked this conversation as resolved Outdated

Suggestion: I find it reads baddy when it's the existence of data in a list is undefined.
In this case the caller can ensure it's in the list in the case it's just been created.

Suggestion: I find it reads baddy when it's the existence of data in a list is undefined. In this case the caller can ensure it's in the list in the case it's just been created.
shelf_regiondata.active_shelf = &shelf;
BLI_remlink_safe(&shelf_regiondata.shelves, &shelf);
BLI_assert(BLI_findindex(&shelf_regiondata.shelves, &shelf) > -1);
BLI_remlink(&shelf_regiondata.shelves, &shelf);
BLI_addhead(&shelf_regiondata.shelves, &shelf);
}
@ -156,6 +159,7 @@ static AssetShelf *update_active_shelf(const bContext &C,
LISTBASE_FOREACH (AssetShelfType *, shelf_type, &space_type.asset_shelf_types) {
if (asset_shelf_type_poll(C, shelf_type)) {
AssetShelf *new_shelf = create_shelf_from_type(*shelf_type);
BLI_addhead(&shelf_regiondata.shelves, new_shelf);
/* Moves ownership to the regiondata. */
activate_shelf(shelf_regiondata, *new_shelf);
return new_shelf;

View File

@ -130,7 +130,7 @@ void AssetView::build_items()
if (!show_names) {
item.hide_label();
}
if (shelf_.type->flag & ASSET_SHELF_TYPE_NO_ASSET_DRAG) {
if (shelf_.type->flag & ASSET_SHELF_TYPE_FLAG_NO_ASSET_DRAG) {
item.disable_asset_drag();
}

View File

@ -430,7 +430,7 @@ void PreviewGridItem::build_grid_tile(uiLayout &layout) const
* they will be upscaled to the button size. Should probably be done by the widget code. */
const int is_preview_flag = (BKE_icon_is_preview(preview_icon_id) ||
BKE_icon_is_image(preview_icon_id)) ?
UI_BUT_ICON_PREVIEW :
int(UI_BUT_ICON_PREVIEW) :
JulianEisel marked this conversation as resolved Outdated

-Wextra warns: grid_view.cc:432:68: warning: enumerated and non-enumerated type in conditional expression. int(UI_BUT_ICON_PREVIEW).

`-Wextra` warns: `grid_view.cc:432:68: warning: enumerated and non-enumerated type in conditional expression`. `int(UI_BUT_ICON_PREVIEW)`.
0;
ui_def_but_icon(but,
preview_icon_id,

View File

@ -860,17 +860,17 @@ static void rna_Space_show_region_hud_update(bContext *C, PointerRNA *ptr)
}
/* Asset Shelf Regions */
JulianEisel marked this conversation as resolved Outdated

This should probably be called show_region_asset_shelf ? (regions -> region). Even though internally disabling the asset shelf also disables the asset-shelf header, this is more of a detail. The HEADER & TOOL_HEADER have the same logic without naming the property "regions".

This should probably be called `show_region_asset_shelf` ? (`regions` -> `region`). Even though internally disabling the asset shelf also disables the asset-shelf header, this is more of a detail. The HEADER & TOOL_HEADER have the same logic without naming the property "regions".
static bool rna_Space_show_regions_asset_shelf_get(PointerRNA *ptr)
static bool rna_Space_show_region_asset_shelf_get(PointerRNA *ptr)
{
return !rna_Space_bool_from_region_flag_get_by_type(ptr, RGN_TYPE_ASSET_SHELF, RGN_FLAG_HIDDEN);
}
static void rna_Space_show_regions_asset_shelf_set(PointerRNA *ptr, bool value)
static void rna_Space_show_region_asset_shelf_set(PointerRNA *ptr, bool value)
{
rna_Space_bool_from_region_flag_set_by_type(ptr, RGN_TYPE_ASSET_SHELF, RGN_FLAG_HIDDEN, !value);
rna_Space_bool_from_region_flag_set_by_type(
ptr, RGN_TYPE_ASSET_SHELF_HEADER, RGN_FLAG_HIDDEN, !value);
}
static void rna_Space_show_regions_asset_shelf_update(bContext *C, PointerRNA *ptr)
static void rna_Space_show_region_asset_shelf_update(bContext *C, PointerRNA *ptr)
{
rna_Space_bool_from_region_flag_update_by_type(C, ptr, RGN_TYPE_ASSET_SHELF, RGN_FLAG_HIDDEN);
}
@ -3495,7 +3495,7 @@ static void rna_def_space_generic_show_region_toggles(StructRNA *srna, int regio
}
if (region_type_mask & ((1 << RGN_TYPE_ASSET_SHELF) | (1 << RGN_TYPE_ASSET_SHELF_HEADER))) {
region_type_mask &= ~((1 << RGN_TYPE_ASSET_SHELF) | (1 << RGN_TYPE_ASSET_SHELF_HEADER));
DEF_SHOW_REGION_PROPERTY(show_regions_asset_shelf, "Asset Shelf", "");
DEF_SHOW_REGION_PROPERTY(show_region_asset_shelf, "Asset Shelf", "");
}
BLI_assert(region_type_mask == 0);
}

View File

@ -2078,7 +2078,7 @@ static void rna_def_asset_shelf(BlenderRNA *brna)
PropertyRNA *prop;
static const EnumPropertyItem asset_shelf_flag_items[] = {
{ASSET_SHELF_TYPE_NO_ASSET_DRAG,
{ASSET_SHELF_TYPE_FLAG_NO_ASSET_DRAG,
"NO_ASSET_DRAG",
0,
"No Asset Dragging",