As an optimization, dependency graph evaluation skips through
no-op nodes at the scheduling stage. However, that leaves update
flags enabled on the node, and the most problematic one is the
USER_MODIFIED flag, which get flushed to children every time
the no-op node is tagged.
This in turn can cause simulation caches downstream to be
permanently stuck in an outdated state until the depsgraph
is rebuilt and the stuck flag cleared out.
Differential Revision: https://developer.blender.org/D16868
The issue was introduced by the optimization of hidden objects and modifiers
in the f12f7800c2.
The solution here detects that either an object is hidden or the modifier is
disabled and does special tricks to ensure the dependencies are evaluated.
This is done by constructing a separate minimal dependency graph needed for
the object on which the modifier is being applied on. This minimal dependency
graph will not perform visibility optimization, making it so modifier
dependencies are ensured to be evaluated.
The downside of such approach is that some dependencies which are not needed
for the modifier are still evaluated. There is no currently an easy way to
avoid this. At least not without introducing possible race conditions with
other dependency graphs.
If the performance of applying modifiers in such cases becomes a problem the
possible solution would be to create a temporary object with a single modifier
so that only minimal set of dependencies is pulled in the minimal dependency
graph.
Differential Revision: https://developer.blender.org/D16421
Motivation is to disambiguate on the naming level what the matrix
actually means. It is very easy to understand the meaning backwards,
especially since in Python the name goes the opposite way (it is
called `world_matrix` in the Python API).
It is important to disambiguate the naming without making developers
to look into the comment in the header file (which is also not super
clear either). Additionally, more clear naming facilitates the unit
verification (or, in this case, space validation) when reading an
expression.
This patch calls the matrix `object_to_world` which makes it clear
from the local code what is it exactly going on. This is only done
on DNA level, and a lot of local variables still follow the old
naming.
A DNA rename is setup in a way that there is no change on the file
level, so there should be no regressions at all.
The possibility is to add `_matrix` or `_mat` suffix to the name
to make it explicit that it is a matrix. Although, not sure if it
really helps the readability, or is it something redundant.
Differential Revision: https://developer.blender.org/D16328
This is the conventional way of dealing with unused arguments in C++,
since it works on all compilers.
Regex find and replace: `UNUSED\((\w+)\)` -> `/*$1*/`
When a change happens which invalidates view layers the syncing will be postponed until the first usage.
This will improve importing or adding many objects in a single operation/script.
`BKE_view_layer_need_resync_tag` is used to tag the view layer to be out of sync. Before accessing
`BKE_view_layer_active_base_get`, `BKE_view_layer_active_object_get`, `BKE_view_layer_active_collection`
or `BKE_view_layer_object_bases` the caller should call `BKE_view_layer_synced_ensure`.
Having two functions ensures that partial syncing could be added as smaller patches in the future. Tagging a
view layer out of sync could be replaced with a partial sync. Eventually the number of full resyncs could be
reduced. After all tagging has been replaced with partial syncs the ensure_sync could be phased out.
This patch has been added to discuss the details and consequences of the current approach. For clarity
the call to BKE_view_layer_ensure_sync is placed close to the getters.
In the future this could be placed in more strategical places to reduce the number of calls or improve
performance. Finding those strategical places isn't that clear. When multiple operations are grouped
in a single script you might want to always check for resync.
Some areas found that can be improved. This list isn't complete.
These areas aren't addressed by this patch as these changes would be hard to detect to the reviewer.
The idea is to add changes to these areas as a separate patch. It might be that the initial commit would reduce
performance compared to master, but will be fixed by the additional patches.
**Object duplication**
During object duplication the syncing is temporarily disabled. With this patch this isn't useful as when disabled
the view_layer is accessed to locate bases. This can be improved by first locating the source bases, then duplicate
and sync and locate the new bases. Will be solved in a separate patch for clarity reasons ({D15886}).
**Object add**
`BKE_object_add` not only adds a new object, but also selects and activates the new base. This requires the
view_layer to be resynced. Some callers reverse the selection and activation (See `get_new_constraint_target`).
We should make the selection and activation optional. This would make it possible to add multiple objects
without having to resync per object.
**Postpone Activate Base**
Setting the basact is done in many locations. They follow a rule as after an action find the base and set
the basact. Finding the base could require a resync. The idea is to store in the view_layer the object which
base will be set in the basact during the next sync, reducing the times resyncing needs to happen.
Reviewed By: mont29
Maniphest Tasks: T73411
Differential Revision: https://developer.blender.org/D15885
The internal state tracking is not fully suited for such kind
of optimization yet.
It is probably not that much work to make them work, but the
issue caused by the changes is serious enough for the studio
so it feels better to revert changes for now and have a closer
look into remaining issues without pressure.
A regression since ac20970bc2
The issue was caused by depsgraph clearing all id->recalc flags
wrongly assuming that all IDs are fully evaluated.
This change makes it so the depsgraph becomes aware of possibly
incompletely evaluated IDs.
Differential Revision: https://developer.blender.org/D15946
This change makes it so that objects which are temporary hidden from
the viewport (the icon toggle in outliner) do not affect on the
performance of the viewport.
The attached file demonstrates the issue. Before this change hiding
the object does not change FPS, after this change FPS goes high when
the object is hidden.
F13435936
Changing the object temporary visibility is already expected to tag
scene for bases updates, which flushes down the stream to the object
visibility update. So the only remaining topic was to ensure the
graph does a special round of visibility update on such changes.
Differential Revision: https://developer.blender.org/D15813
While it is hard to measure the performance impact accurately, there
is no need to perform per-modifier string lookup on every frame update.
Implemented as an exceptional case in the code which flushes updates to
the entire component. Sounds a bit suboptimal, but there are already
other exception cases handled in the function.
Differential Revision: https://developer.blender.org/D15812
While missing the break before a default that only breaks isn't
an error, it means adding new cases needs to remember to add the
break for an existing case, changing the default case will also
result in an unintended fall-through.
Also avoid `default:;` and add an explicit break.
Need to update relations when modifiers are added or removed
since those create nodes in the dependency graph.
Added an assert statement to point at possible culprit so
that issues can be fixed more quickly.
Solves long-standing issue when dependencies of disabled modifiers are
evaluated.
Simple test case: no drivers or animation. Manually enabling modifier
is expected to bring FPS up, enabling modifier will bring FPS (sine
evaluation can not be avoided)
F13336690
More complex test case: modifier visibility is driven by an animated
property. In am ideal world FPS during property being zero is fast
and when property is 1 the FPS is low.
F13336691.
Differential Revision: https://developer.blender.org/D15625
A mistake in the 0dcee6a386 which made specific driven visibility
to work, but did not properly handle actual time-based visibility.
The basic idea of the change is to preserve recalculation flags of
nodes which were tagged for update but were not evaluated due to
visibility constraints. In the file from the report this makes it
so tagging which is done first time ID is in the dependency graph
are handled when the ID actually becomes visible. This is what
solved the root of the problem from the report: there was missing
geometry update since it was "swallowed" by the evaluation during
the object being invisible. In other configurations this change
allows to handle pending geometry updates due to animated modifiers
be handled when object becomes visible without time change.
This change also solves visibility issue of the synchronization
component which also started to be handled badly since the
previous fix attempt. Basically, the needed exception in its
visibility handling did not happen and a regular logic was used
for it.
Tested with files from the T99733, T99976, and from the Heist
project.
Differential Revision: https://developer.blender.org/D15544
The issue was caused by the fact that objects with driven or animated
visibility were considered visible by the dependency graph evaluation.
This change makes it so the dependency graph evaluation is aware of
visibility which might be changing. This is achieved by evaluating the
path of the graph which affects objects visibility and adjusts to it
before evaluating the rest of the graph.
There is some time penalty to this, but there does not seem to be a
way to fully avoid this penalty.
With the production shot from the heist project the FPS drops by a
tenth of a frame (~9.4 vs ~9.3 fps) when adding a driver to an object
which keeps it visible. Note that this is a bit hard to measure since
the FPS fluctuates quite a bit throughout the playback. On the other
hand, having a driver on a visibility of a heavy object from character
and setting visibility to false gives big speedup.
Also worth noting that there is no penalty at all when there are no
animated visibilities in the scene.
Differential Revision: https://developer.blender.org/D15498
The goal is to make it possible to evaluate the graph in multiple
passes without evaluating the same node multiple times.
Currently should not be any functional changes.
Not sure why multiple pools were created: the pool should be able to
handle two sets of tasks.
Perhaps non-measurable improvement in terms of performance but this
change simplifies code a bit.
Solves compilation warning with Clang, and moves manipulation with
DNA structures to the designed way for C++.
The tests and few other places are update to the new code by Jacques.
Ref T96847
Maniphest Tasks: T96847
Differential Revision: https://developer.blender.org/D14625
- Missing star prefix.
- Unnecessary indentation.
- Blank line after dot-points
(otherwise doxygen merges with the previous dot-point).
- Use back-slash for doxygen commands.
- Correct spelling.
This adds missing cases to detect edit mode for Curves objects.
Unlike other object types, Curves do not have specific edit data,
rather we edit the original data directly, and rely on `Object.mode`.
For this, `BKE_object_data_is_in_editmode` had to be modified to
take a pointer to the object. This affects two places: the outliner
and the dependency graph. For the former place, the object pointer
is readily available, and we can use it. For the latter, the object
pointer is not available, however since it is used to update edit
mode pointers, and since Curves do not have such data, we can
safely pass null to the function here.
This also fixes the assertion failure that happens when closing a file
in edit mode.
Differential Revision: https://developer.blender.org/D14330
This quick fix will populate the runtime orig pointers to avoid
crashes when a grease pencil object uses layer transforms, parenting
or modifiers.
This will have to be revisited and fixed with a better solution.
This commit renames enums related the "Curve" object type and ID type
to add `_LEGACY` to the end. The idea is to make our aspirations clearer
in the code and to avoid ambiguities between `CURVE` and `CURVES`.
Ref T95355
To summarize for the record, the plans are:
- In the short/medium term, replace the `Curve` object data type with
`Curves`
- In the longer term (no immediate plans), use a proper data block for
3D text and surfaces.
Differential Revision: https://developer.blender.org/D14114
Use a shorter/simpler license convention, stops the header taking so
much space.
Follow the SPDX license specification: https://spdx.org/licenses
- C/C++/objc/objc++
- Python
- Shell Scripts
- CMake, GNUmakefile
While most of the source tree has been included
- `./extern/` was left out.
- `./intern/cycles` & `./intern/atomic` are also excluded because they
use different header conventions.
doc/license/SPDX-license-identifiers.txt has been added to list SPDX all
used identifiers.
See P2788 for the script that automated these edits.
Reviewed By: brecht, mont29, sergey
Ref D14069
This implements the update cache described in T95401.
The cache is currently only used for drawing strokes and
sculpting (using the push brush).
**Note: Making use of the cache throughout grease pencil will
have to be done incrementally in other patches. **
The update cache stores what elements have changed in the
original data-block since the last time the eval object
was updated. Additionally, the update cache can store multiple
updates to the data and minimizes the number of elements
that need to be copied.
Elements can be tagged using `BKE_gpencil_tag_full_update` and
`BKE_gpencil_tag_light_update`. A full update means that the element
itself will be copied but also all of the content inside. E.g. when a
layer is tagged for a full update, the layer, all the frames inside the
layer and all the strokes inside the frames will be copied.
A light update means that only the properties of the element are copied
without any of the content. E.g. if a layer is tagged with a light
update, it will copy the layer name, opacity, transform, etc.
When the update cache is in use (e.g. elements have been tagged) then
the depsgraph will not trigger a copy-on-write, but an update-on-write.
This means that the update cache will be used to determine what elements
have changed and then only those elements will be copied over to the
eval object.
If the update cache is empty or the data block was tagged with a full
update, we always fall back to a copy-on-write.
Currently, the update cache is only used by the active depsgraph. This
is because we need to free the update cache after an update-on-write so
it's reset and we need to make sure it is not freed or read by other
depsgraphs.
Co-authored-by: @yann-lty
This patch was contributed by The SPA Studios.
Reviewed By: sergey, antoniov, #dependency_graph, pepeland, mendio
Maniphest Tasks: T95401
Differential Revision: https://developer.blender.org/D13984
Based on discussions from T95355 and T94193, the plan is to use
the name "Curves" to describe the data-block container for multiple
curves. Eventually this will replace the existing "Curve" data-block.
However, it will be a while before the curve data-block can be replaced
so in order to distinguish the two curve types in the UI, "Hair Curves"
will be used, but eventually changed back to "Curves".
This patch renames "hair-related" files, functions, types, and variable
names to this convention. A deep rename is preferred to keep code
consistent and to avoid any "hair" terminology from leaking, since the
new data-block is meant for all curve types, not just hair use cases.
The downside of this naming is that the difference between "Curve"
and "Curves" has become important. That was considered during
design discussons and deemed acceptable, especially given the
non-permanent nature of the somewhat common conflict.
Some points of interest:
- All DNA compatibility is lost, just like rBf59767ff9729.
- I renamed `ID_HA` to `ID_CV` so there is no complete mismatch.
- `hair_curves` is used where necessary to distinguish from the
existing "curves" plural.
- I didn't rename any of the cycles/rendering code function names,
since that is also used by the old hair particle system.
Differential Revision: https://developer.blender.org/D14007
Part of T91671.
Not much else to say, this is mainly a massive deletion of code.
Note that a few cleanups possible after this proxy removal were kept out
of this commit to try to reduce a bit its size.
Reviewed By: sergey, brecht
Maniphest Tasks: T91671
Differential Revision: https://developer.blender.org/D13995
The issue was happening with a specific file where the ID management
code was not fully copying all modifiers because of the extra check
in the `BKE_object_support_modifier_type_check()`.
While it is arguable that copy-on-write should be a 1:1 copy there is
no real need to maintain the per-modifier pointer to its original.
Use its SessionUUID to perform lookup in the original datablock.
Downside of this approach is that it is a linear lookup instead of
direct pointer access, but the upside is that there is less pointers
to manage and that the file with unsupported modifiers does behave
correct without any asserts.
Differential Revision: https://developer.blender.org/D13993
The modifiers are mapped between original and evaluated objects based on
their session IDs. The pointer to original modifier is no longer needed
for the backup: it remained from the initial implementation which was
rewritten at some point.
This is a preparation for removal of the pointer to original modifier.
Blender would have crashed when renaming bone in Edit Mode, Saving, and
than selecting/deselecting.
Caused by a mistake in the 0f89bcdbeb: can not "short-circuit" the
CoW update if it was explicitly requested.
Safest for now solution seems to be to store whether the CoW component
has been explicitly tagged, so that the following configuration can be
supported:
DEG_id_tag_update(id, ID_RECALC_GEOMETRY);
DEG_id_tag_update(id, ID_RECALC_COPY_ON_WRITE);
Differential Revision: https://developer.blender.org/D13966
The evaluated mesh is a result of evaluated modifiers, and referencing
other evaluated IDs such as materials.
It can not be stored in the EditMesh structure which is intended to be
re-used by many areas. Such sharing was causing ownership errors causing
bugs like
T93855: Cycles crash with edit mode and simultaneous viewport and final render
The proposed solution is to store the evaluated edit mesh and its cage in
the object's runtime field. The motivation goes as following:
- It allows to avoid ownership problems like the ones in the linked report.
- Object level is chosen over mesh level is because the evaluated mesh
is affected by modifiers, which are on the object level.
This patch allows to have modifier stack of an object which shares mesh with
an object which is in edit mode to be properly taken into account (before
the change the modifier stack from the active object will be used for all
objects which share the mesh).
There is a change in the way how copy-on-write is handled in the edit mode to
allow proper state update when changing active scene (or having two windows
with different scenes). Previously, the copt-on-write would have been ignored
by skipping tagging CoW component. Now it is ignored from within the CoW
operation callback. This allows to update edit pointers for objects which are
not from the current depsgraph and where the edit_mesh was never assigned in
the case when the depsgraph was evaluated prior the active depsgraph.
There is no user level changes changes expected with the CoW handling changes:
should not affect on neither performance, nor memory consumption.
Tested scenarios:
- Various modifiers configurations of objects sharing mesh and be part of the
same scene.
- Steps from the reports: T93855, T82952, T77359
This also fixes T76609, T72733 and perhaps other reports.
Differential Revision: https://developer.blender.org/D13824
- Added space below non doc-string comments to make it clear
these aren't comments for the symbols directly below them.
- Use doxy sections for some headers.
Ref T92709
Not sure why this bug was only discovered by such an elaborate steps
and why it took so long to be discovered. The root of the issue is
that in the 956c539e59 the typical flow of tag+flush+evaluate was
violated and tagging for visibility change happened after flush.
Copy-on-write data blocks could be referenced from python but were not
properly managing python reference counting.
This would leak memory for any evaluated data-blocks accessed by Python.
Reviewed By: sergey
Ref D12850
Seems to be residue from an early 2.80 days: the placeholder code path
is no longer used.
Remove all the tricky code for this, and make it clear that the
`deg_expand_copy_on_write_datablock` is used on an non-expanded
datablock.
Should be no functional changes. And should help simplify D12850.
Differential Revision: https://developer.blender.org/D12852
