Previously retrieving a collection from the context like "selected_ids"
would give a linked list of allocated items. Now it returns a vector of
RNA pointers. Though the number of items is typically fairly small,
using contiguous memory and avoiding many small allocations are
typical performance improvements that could still be beneficial
when there are many items. Iteration also becomes much simpler.
Pull Request: https://projects.blender.org/blender/blender/pulls/119939
`asset_lib` is null when calling undo in active file browser space.
This causes crash in `asset undo poll`. So exit out of the poll function
when active file space is filebrowser.
Pull Request: https://projects.blender.org/blender/blender/pulls/119870
Since 660867fa00, having tree-view items uncollapsed by default using
the `set_collapsed()` functions wouldn't work anymore. An attempt to do
this would assert even, so eb71d9f7bc disabled the assert.
I think a function designed to handle exactly this is the best solution,
it makes the intent & behavior more clear than before, and highlights
that this is a special case.
Mitigates #117957, in that it solves the regression, but tree-views still
don't remember their state on screen-layout changes. This is a known
limitation and not supported.
Pull Request: https://projects.blender.org/blender/blender/pulls/119166
- Remove the unnecessary `ContainerValue` from the class hierarchy
- Construct `StringValue` with a `std::string` by value to avoid copies
- Remove some indirection by using type names directly instead of aliases
- Use utility methods to lookup/append specific data types for arrays/dicts
- Simplify conversion from unique_ptr to shared_ptr
- Avoid use of `new` and `delete`
- Avoid creating maps of all elements in vector for a single lookup
The `AssetEntryReader` and `AssetEntryWriter` classes were only used
once and just added indirection hiding the actual logic of reading and writing
asset metadata from the index. It also hid double map lookups and extra
string copies/allocations.
It's typically only worth specifying specific captures when the lambda
is stored as a function object for use later. Since this is just passed
as a FunctionRef as an iteration callback, we can use a less complex
capture here. That also helps clarify that this isn't a callback.
This class was just used to hold some functions which can just as well
be declared static outside of any class. It was never even instantiated.
Removing the class removes some indirection in this file that made
the code less readability, and avoids misleading readers.
Pull Request: https://projects.blender.org/blender/blender/pulls/119137
This class was used to make `AssetLibraryReference` work as a key
in `Map`. But defining a class for that is unnecessary. Instead it's
simpler to just declare the equals and hash operators separately.
Reducing the number of wrappers and indirections should make
this code slightly easier to understand.
Over the last couple years (!) UI buttons have moved to derived classes,
meaning we don't need to use the same "a1" and "a2" variables to store
different information. At this point, that information is set specifically
by internal UI code, or functions like `UI_but_*_set`.
These values are only set to their default 0 values now (or -1 in some
non-meaningful cases). This commit removes the values from buttons
and removes the remaining a1 and a2 arguments from the UI API.
Making the member private (or at least protected) makes threat safety
more tangible, and we don't need to expose locking in the API. Generally
we need to make data more encapsulated, so we can make edits more
controlled and threat safe.
Asset libraries also always have a catalog-service, so it can accessed
by reference, rather than pointer that would have to be null-checked.
Affects the catalog tree found in the catalog selector popup in the
asset shelf.
Looks like this added the padding for indented items without children
twice, which was misleading (because it would look like items were
nested deeper than siblings with children). I think it was added twice
since 2e02c0c515, which changed how tree view indentation is done.
Rebuilding the tree immediately after changes could cause the tree to be
rebuilt multiple times. More importantly, it made it harder to reason
about thread safety, since we would touch the tree within a whole bunch
of API functions. Now tree building is simplified and managed in a
single place, so making the tree building thread safe can be made
trivially in a follow-up.
Note, this means the initial catalog tree building doesn't happen in a
background thread together with loading the asset library and catalogs
anymore. But we would already do all further rebuilds on the main thread
anyway, this shouldn't have any notable impact.
Basic motivation is that `AssetCatalogService::get_catalog_tree()`
should return a const tree, since this tree is internal state and
shouldn't be modified from outside. This exposed a whole bunch of const
incorrectnesses and just generally allows to make much more of the API
const (as it should be).
Also use references instead of pointers in testing functions, where null
is not an expected value.
The asset catalog selector tree-view would store a copy of each of the
items in the catalog tree, including all of its sub-hierarchy. This can
be avoided, it can just use a reference.
This is generally more flexible and less error prone. The struct
implements a proper descructorfor this anyway. That also makes the
separate free function unnecessary-- it's redundant with the destructor.
Span is preferrable since it's agnostic of the source container,
makes it clearer that there is no ownership, is 8 bytes smaller,
and can be passed by value.
I added a new BLO_userdef_default.h header to contain declarations of
two global variables that are still defined in C files. Use of designated
initializers for large structs make those files harder to change.
Arguably this is a better header for them anyway.
Pull Request: https://projects.blender.org/blender/blender/pulls/118015
Use `BLI_strdupn` instead of repeating the same logic a bunch of times.
Use StringRef where StringRefNull isn't necessary, and use StringRefNull
instead of a std::string reference in one case.
This avoids the need for manual memory management/raw pointers,
improves const correctness, improves type safety, simplifies iteration,
and simplifies the process of registering a new asset shelf type.
Pull Request: https://projects.blender.org/blender/blender/pulls/117770
- Remove unnecessary use of "shelf" namespace inside that namespace
- Remove unused function declaration
- Don't bother passing a pointer to a pointer to the region data,
that didn't accomplish anything and added unnecessary complexity
Hans Goudey
Campbell Barton
Pratik Borhade
Julian Eisel
Brecht Van Lommel
Bastien Montagne