blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 781 Projects 19 Wiki Activity

UI: Asset Shelf (Experimental Feature) #104831

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

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 30 Commits 399 Files Changed - +16 -15
4 changed files with 16 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit f768cc2c31 - Show all commits

20
source/blender/editors/asset/intern/asset_shelf.cc
View File

@ -124,17 +124,17 @@ void ED_asset_shelf_region_draw(const bContext *C,
const uiStyle *style = UI_style_get();
const float padding = style->panelouter;
UI_block_layout(block,
UI_LAYOUT_VERTICAL,
UI_LAYOUT_PANEL,
padding,
-padding,
region->winx - 2 * padding,
1,
0,
style);
uiLayout *layout = UI_block_layout(block,
UI_LAYOUT_VERTICAL,
UI_LAYOUT_PANEL,
padding,
-padding,
region->winx - 2 * padding,
1,
0,
style);
shelf::build_asset_view(block, all_library_ref, shelf_settings, *C, *region);
shelf::build_asset_view(*layout, all_library_ref, shelf_settings, *C, *region);
int layout_width, layout_height;
UI_block_layout_resolve(block, &layout_width, &layout_height);

3
source/blender/editors/asset/intern/asset_shelf.hh
View File

@ -13,6 +13,7 @@ struct AssetLibraryReference;
struct AssetShelfSettings;
struct bContext;
struct uiBlock;
struct uiLayout;
namespace blender::asset_system {
class AssetCatalogPath;
@ -20,7 +21,7 @@ class AssetCatalogPath;
namespace blender::ed::asset::shelf {
void build_asset_view(uiBlock *block,
void build_asset_view(uiLayout &layout,
const AssetLibraryReference &library_ref,
const AssetShelfSettings *shelf_settings,
const bContext &C,

5
source/blender/editors/asset/intern/asset_shelf_asset_view.cc
View File

@ -170,7 +170,7 @@ std::unique_ptr<ui::AbstractViewItemDragController> AssetViewItem::create_drag_c
/* ---------------------------------------------------------------------- */
void build_asset_view(uiBlock *block,
void build_asset_view(uiLayout &layout,
const AssetLibraryReference &library_ref,
const AssetShelfSettings *shelf_settings,
const bContext &C,
@ -188,11 +188,12 @@ void build_asset_view(uiBlock *block,
asset_view->set_catalog_filter(catalog_filter_from_shelf_settings(shelf_settings, *library));
asset_view->set_tile_size(UI_preview_tile_size_x() * 0.8, UI_preview_tile_size_y() * 0.8);
uiBlock *block = uiLayoutGetBlock(&layout);
ui::AbstractGridView *grid_view = UI_block_add_view(
*block, "asset shelf asset view", std::move(asset_view));
ui::GridViewBuilder builder(*block);
builder.build_grid_view(*grid_view, region.v2d);
builder.build_grid_view(*grid_view, region.v2d, layout);
JulianEisel marked this conversation as resolved Outdated
Hans Goudey commented 2023-06-06 19:28:01 +02:00
Outdated
Review
Copy Link

If the dynamic_cast isn't meant to fail in some cases, it's probably better to use static_cast to avoid bloating the code with dynamic casting.

Also, class methods should generally be accessed with this->

If the dynamic_cast isn't meant to fail in some cases, it's probably better to use `static_cast` to avoid bloating the code with dynamic casting. Also, class methods should generally be accessed with `this->`
Julian Eisel commented 2023-06-12 16:27:15 +02:00
Outdated
Review
Copy Link

Not a fan of using static_cast for down casting. It removes type safety for virtually (pun intended) no benefit. Sure it's unlikely to cause issues here, but if it does it's good to get an exception thrown. There's a language feature designed for this, so I rather use it.

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-dynamic_cast
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c147-use-dynamic_cast-to-a-reference-type-when-failure-to-find-the-required-class-is-considered-an-error

Not a fan of using `static_cast` for down casting. It removes type safety for virtually (pun intended) no benefit. Sure it's unlikely to cause issues here, but if it does it's good to get an exception thrown. There's a language feature designed for this, so I rather use it. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-dynamic_cast https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c147-use-dynamic_cast-to-a-reference-type-when-failure-to-find-the-required-class-is-considered-an-error
Hans Goudey commented 2023-06-12 16:38:02 +02:00
Outdated
Review
Copy Link

Hard to argue with the code guidelines I guess.. Still though, I'd find it clearer to use static_cast, since dynamic_cast gives the impression that the author thinks the cast might fail. But using a reference for the variable negates that impression, making the whole thing confusing. Not a big deal though

Hard to argue with the code guidelines I guess.. Still though, I'd find it clearer to use `static_cast`, since `dynamic_cast` gives the impression that the author thinks the cast might fail. But using a reference for the variable negates that impression, making the whole thing confusing. Not a big deal though
}
/* ---------------------------------------------------------------------- */

3
source/blender/editors/asset/intern/asset_shelf_catalog_selector.cc
View File

@ -167,8 +167,7 @@ uiBlock *catalog_selector_block_draw(bContext *C, ARegion *region, void * /*arg1
"asset catalog tree view",
std::make_unique<AssetCatalogSelectorTree>(*library, *shelf_settings));
ui::TreeViewBuilder builder(*block);
builder.build_tree_view(*tree_view);
ui::TreeViewBuilder::build_tree_view(*tree_view, *layout);
}
UI_block_bounds_set_normal(block, 0.3f * U.widget_unit);