Move `Library.runtime` to be a pointer, move the related
`LibraryRuntime` struct to `BKE_library.hh`. Similar to e.g.
Mesh.runtime, that pointer is expected to always be valid, and is
allocated at readtime or when creating a new Library ID.
Related smaller changes:
* Write code now uses standard ID writing codepath for Library IDs too.
* Runtime pointer is reset to nullptr before writing.
* Looking up a library by its absolute path is now handled through a
dedicated utils, `search_filepath_abs`, instead of using
`BLI_findstring`.
Pull Request: https://projects.blender.org/blender/blender/pulls/134188
This adds a debugging utility for developers that makes it easier to what's
stored in a .blend file. Some possible use cases:
* Figure out if some specific data has been written to the .blend file.
* Check the order of data in the .blend file.
* Work towards having less runtime dependent changes in .blend files (it's
easier to diff the textual output).
It is **not** a goal to provide a general human and machine readable form of
.blend files (like xml/json files). That would be way more involved.
The way to use this is to set `GENERATE_DEBUG_BLEND_FILE` to `1` at the top of
`writefile.cc`. Then, whenever Blender saves a .blend file, it will generate a
`.debug.txt` file next to it.
There is already `blend2json.py` which serves a similar purpose but is a
separate program that can be executed on the .blend file afterwards. With the
`--full-data` flag it outputs comparable data, but the output is a bit more
verbose and it needs an extra step that can be avoided by generating the
`.debug.txt` file immediately. For certain use cases,
`GENERATE_DEBUG_BLEND_FILE` can be more convenient. It's also much simpler to
add log additional data in that file that is produced during the blend-write
process.
Pull Request: https://projects.blender.org/blender/blender/pulls/133063
Avoids some binary noise in saved files, and will some day allow us to use runtime data on read without having to explicitly clean it up (unless we refactor this into an allocated pointer instead!).
Pull Request: https://projects.blender.org/blender/blender/pulls/132190
Previously, calling `clear()` on `Map`, `Set` or `VectorSet` would remove all
elements but did not free the already allocated capacity. This is fine in most
cases, but has very bad and non-obvious worst-case behavior as can be seen in
#131793. The issue is that having a huge hash table with only very few elements
is inefficient when having to iterate over it (e.g. when clearing).
There used to be a `clear_and_shrink()` method to avoid this worst-case
behavior. However, it's not obvious that this should be used to improve
performance.
This patch changes the behavior of `clear` to what `clear_and_shrink` did before
to avoid accidentally running in worst-case behavior. The old behavior is still
available with the name `clear_and_keep_capacity`. This is more efficient if
it's known that the hash-table is filled with approximately the same number of
elements or more again.
The main annoying aspect from an API perspective is that for `Vector`, the
default behavior of `clear` is and should stay to not free the memory. `Vector`
does not have the same worst-case behavior when there is a lot of unused
capacity (besides taking up memory), because the extra memory is never looked
at. `std::vector::clear` also does not free the memory, so that's the expected
behavior. While this patch introduces an inconsistency between `Vector` and
`Map/Set/VectorSet` with regards to freeing memory, it makes them more
consistent in that `clear` is the better default when reusing the data-structure
repeatedly.
I went over existing uses of `clear` to see if any of them should be changed to
`clear_and_keep_capacity`. None of them seemed to really benefit from that or
showed that it was impossible to get into the worst-case scenario. Therefore,
this patch slightly changes the behavior of these calls (only performance wise,
semantics are exactly the same).
Pull Request: https://projects.blender.org/blender/blender/pulls/131852
Give the `IDWALK_CB_…` enum an explicit name:
`LibraryForeachIDCallbackFlag`. This way the flags are type-safe, and
it's known where values come from. This is much preferred (at least by
me) to just having `int flags`.
Uses of `0` have been replaced with `IDWALK_CB_NOP` as that has the same
value and is of the right type.
One invalid use of `IDWALK_NOP` was detected by this change, and is
replaced by `IDWALK_CB_NOP`.
This change might be incomplete; I gave the enum a name, fixed the
compiler errors, and then also updated assignments like `int cb_flag =
cb_data->cb_flag`. I might have missed some assignments to `int` though.
No functional changes.
Pull Request: https://projects.blender.org/blender/blender/pulls/131865
Two commits that basically do the same thing for two `enum`s: give
them a name.
- the `IDWALK_…` enum → `LibraryForeachIDFlag`.
- the `IDWALK_CB_…` enum → `LibraryForeachIDCallbackFlag`.
This way the flags are type-safe, and it's known where values come
from. This is much preferred (at least by me) to just having `int
flags`.
Uses of `0` have been replaced with `IDWALK_NOP` and `IDWALK_CB_NOP`,
as those have the same value and are of the right type.
One invalid use of `IDWALK_NOP` was detected by this change, and is
replaced by `IDWALK_CB_NOP`. And another one in the opposite
direction.
This change might be incomplete; I gave the enum a name, fixed the
compiler errors, and then also updated assignments like `int cb_flag =
cb_data->cb_flag`. I might have missed some assignments to `int`
though.
No functional changes.
----------
I intend to land this PR as its two separate commits. I just put them in the same PR so the buildbot can handle them in one go, and we don't have a stack of highly relatled PRs.
In the future this could also apply to the `IDWALK_RET_…` enum. This one I left out, though, because a proper cleanup there would also have to include their ambiguity on whether they are bitflags (like the enums in this PR) or not. Their values and the code in `BKE_lib_query_foreachid_process()` implies they are bitflags, but in practice they are never or'ed together and just used as discrete values.
Pull Request: https://projects.blender.org/blender/blender/pulls/131803
This improves the `write_libraries` function in a couple of ways:
* No need to split the `bmain` which I find somewhat hard to reason about. I
think it's good if `bmain` is as read-only as possible in this write-context.
Instead a more explicit C++ data structure (`MultiValueMap<Library *, ID *>`)
is used. I think this was the only place in write-code that used
`blo_split_main`.
* Deduplication of the check that determines whether a placeholder should be
written for a specific ID.
* General cleanup to avoid unnecessarily deep nesting when iterating over IDs.
No functional changes are expected.
Pull Request: https://projects.blender.org/blender/blender/pulls/131385
Splitting the write-loop into stages makes it easier to understand, debug and time.
All the complex filtering in `gather_local_ids_to_write` is the same as before.
With this and some previous refactors, it's also much easier to have a clean `write_id`
function that should also work for embedded IDs later on.
Pull Request: https://projects.blender.org/blender/blender/pulls/130983
This is a leftover from an earlier version of overrides, but is effectively dead
code currently. It was used when there were proportional diffing capabilities.
However, Bastien mentioned that this would be implemented differently nowadays
anyway if it becomes necessary again.
This simplifies the write ID loop quite significantly.
Pull Request: https://projects.blender.org/blender/blender/pulls/130928
This replaces the existing C API of `BLO_Write_IDBuffer` with a C++ API. This
helps because:
* No need for explicit freeing.
* Can use more generic `DynamicStackBuffer` utility instead of having a separate
implementation of that.
Additionally, the API is changed so that a new `BLO_Write_IDBuffer` is created
for each `ID` instead of reusing the same for multiple IDs. This simplifies the
code quite a bit and allows for better use of the RAII pattern.
I expect the performance to be the same as before. In theory, there could be a
small speedup, because the `BLO_Write_IDBuffer` is not allocated separately
anymore, but that should be negligible.
No functional changes are expected.
Pull Request: https://projects.blender.org/blender/blender/pulls/130452
This adds a `write_bhead` utility function and also reduces the scope of
some `BHead` variables. The separate `write_bhead` function was quite
useful for #129751 and will likely be useful in a potential separate
implementation too.
Pull Request: https://projects.blender.org/blender/blender/pulls/130457
`writedata` used to align the written buffer size to a multiple of 4. This
causes multiple issues:
* Writes uninitialized data.
* Crash with ASAN due to a heap buffer overflow if the buffer is not any longer
than what is passed in.
* Modifies the length of the buffer which can't be undone when reading the
buffer again.
I don't know of any reason for this alignment here. I'd think that it doesn't
matter when writing to a file. If it would matter, then we should probably align
to at least 8 nowadays because that's the alignment of pointers. The original
reason for this alignment seems to be lost to history. It was already part of
the initial commit.
Pull Request: https://projects.blender.org/blender/blender/pulls/129821
Before efb511a76d this was not necessary, because the G_FILE_COMPRESS option
was not disabled when writing memfile undo steps. However, compression is
generally disabled when writing quit.blend or autosave files.
Previously, values for `ID.flag` and `ID.tag` used the prefixes `LIB_` and
`LIB_TAG` respectively. This was somewhat confusing because it's not really
related to libraries in general. This patch changes the prefix to `ID_FLAG_` and
`ID_TAG_`. This makes it more obvious what they correspond to, simplifying code.
Pull Request: https://projects.blender.org/blender/blender/pulls/125811
Previous namings in makesdna code was very confusing and inconsistent.
This commit unifies names accross the codebase as such:
- `struct` for the structs definitions data.
- `type` for the types definitions data.
- `member` for the struct members definitions data.
Structs and types definitions are not in synced for two reasons:
- types contains also definitions for basic types (int, float, etc.).
- types can be discovered before their struct definition (as members of
other structs).
This commit also groups all members of `SDNA` struct more logically (all
'structs' ones together, then all 'types' ones, then all 'struct
members' ones).
This commit should have no behavioral change at all.
Pull Request: https://projects.blender.org/blender/blender/pulls/125605
If a data-block referenced the same shared data more than once, it used to be
written multiple times. This worked out so far, because the written data is
always the same. So when reading the file, it doesn't matter which written
buffer Blender decides to read on. However, this makes the .blend file look
corrupt and leads to memory leaks on load (#125001).
Pull Request: https://projects.blender.org/blender/blender/pulls/125489
Blendfile uses the 'old (memory) address' of its data as 'uid' in the
blendfile. There should only be one block written for a given address
and a same ID (each ID define its own 'virtual address space').
This commit checks that this condition is met at wrtite time (except for
undo steps, for performances reasons).
Tooling part of the investigations on #125001.
File from #124777 (from 2.79.1) cause a temp bScreen to not be properly
tagged as such, which prevents its deletion when related window is
closed, and leaves a zero-user bScreen in Main.
While this should not happen, this does not seem trivial to track down,
and not an important enough issue to spend more time on it.
Also reported as #124857.
This is a workaround to allow user to keep working without loss of data
when an issue like #124049 happens.
This commit also expose again the `use_all_linked_data_direct` debug
option, no idea why that one was removed.
Regression from 435b6743fd, no usage of unlinkable ID (mainly
shapekeys...) should make them directly linked.
Note that this had no serious consequences, it was mainly printing annoying
error messages in release builds, and asserting in debug ones.
This was a consequence of the work done in #106321, where this specific
'active in UI' case was not identified and properly handled.
Now, consider most ID usages from UI (editors) as 'weak links', i.e.
keep a reference to these IDs even if they are only indirectly used.
Note that missing weak links will not create placeholders if the source
data is not found in the library anymore on load. they are just silently
dropped.
Pull Request: https://projects.blender.org/blender/blender/pulls/122207
A few ID types are considered as 'never unused' in Blender (UI related
ones, the Libraries and the Scenes). Local IDs of this type are always
considered as used, even if no other ID links to them.
This was previously fairly weekly defined and implemented (mainly in the
writefile code and the 'tag unused' libquery functions).
This commit formalize this characteristic of ID types by adding a new
`IDTYPE_FLAGS_NEVER_UNUSED` flag, and using this in the few places in
the code that handle unused IDs.
This introduce a new "secret" per-repository property of type password
as described by #121856.
A token or secret may be used by some non blender.org repositories.
This only shows for remote repositories and is shown in the
"Add Remote Repository" popup.
This commit doesn't implement sending to token to the server which will
be implemented separately.
Ref !121886
Co-authored-by: Dalai Felinto <dalai@blender.org>
Adds a new asset shelf option (`STORE_ENABLED_CATALOGS_IN_PREFERENCES`
option in RNA) to use the Preferences for storing the enabled catalogs.
This way asset shelf types can decide if for their use-case, they want
to synchronize the enabled catalogs over Blender sessions and files, or
keep the stored locally in the file.
This is important because for example on one hand, it would be annoying
if for brush assets you'd have to enable the visible catalog tabs for
every 3D View and every file, while on the other hand you need that
level of control for the pose library where the catalogs the rigger/
animator cares about varies from project to project, character to
character and shot to shot.
Conceptually this also makes some sense: The new brush assets workflow
synchronizes brush assets and their catalogs across Blender sessions
and files, basically making them globally accessible independent of
the current file/project, so treating the enabled catalogs the same
is consistent.
Previously reviewed in #120264
Pull Request: https://projects.blender.org/blender/blender/pulls/121363
This makes the read and write API functions match more closely, and adds
asserts to check that the data size is as expected.
There are still a few places remaining that use BLO_read_data_address
and similar generic functions, these should eventually be replaced as well.
Pull Request: https://projects.blender.org/blender/blender/pulls/120994
This adds implicit sharing support for the `MemFile` undo-step. This decreases memory
usage and increases performance.
Implicit sharing allows the undo system to take (shared) ownership of some data.
Previously, the data would always be serialized and compared to the previous undo-step.
So this turns an O(n) operation into O(1) (in terms of memory usage and time).
Read/write code that wants to make use of this has to use the new `BLO_read_shared`
and `BLO_write_shared` functions respectively. Those either make use of implicit-sharing
internally or do the "full" read/write based on a passed-in function. It seems possible to
use the same API in the future to store shared data to .blend files.
Improvements:
* Much faster undo step creation in many cases by avoiding the majority data copies
and equality checks. This fixes#98574. I found undo step creation and undo step
decoding to be 2-5 times faster in some demo files from the blender website and in
some production files from the Heist project.
* Reduced memory usage when there is large data in `bmain`. For example, when
loading the same highly subdivided mesh that I used in #106228 the memory usage
is 1.03 GB now (compared to 1.62 GB in `main` currently). The main remaining copy
of the data now is done by rendering code.
* Some significant performance improvements were also measured for the new grease
pencil type (#105540).
There is one main downside of using implicit-sharing as implemented here: `MemFile`
undo steps can't be written as .blend files anymore. This has a few consequences:
* Auto-save becomes slower (up to 3x), because it can't just save the previous undo step
anymore and does a normal save instead. This has been discussed in more detail here:
https://devtalk.blender.org/t/remove-support-for-saving-memfile-undo-steps-as-blend-files-proposal/33544
It would be nice to work towards asynchronous auto-save to alleviate this problem.
Some previous work has been done to reduce the impact of this change in 41b10424c7
and f0f304e240. This has been committed separately in efb511a76d.
* Writing `quit.blend` has to do a normal file save now. So it's a bit slower too, but it's
less of a problem in practice.
* The `USE_WRITE_CRASH_BLEND` functionality does not work anymore. It doesn't seem
to be used by anyone (removed in e90f5d03c4)
There are also benefits to not writing `MemFile` from undo steps to disk. It allows us to
more safely do undo-specific optimizations without risking corrupted .blend files. This
is especially useful when we want to preserve forward compatibility in some cases.
This requires converting data before writing the .blend files, but this conversion is not
necessary for undo steps. Trying to implement this kind of optimization in the past has
often lead to bugs (e.g. 43b37fbc93).
Another new problem is that it is harder to know the size of each undo step. Currently, a
heuristic is used to approximate the memory usage, but better solutions could be found
if necessary.
Pull Request: https://projects.blender.org/blender/blender/pulls/106903
