From 4588112db38e5695d6db02e9ec6ef0d430d7772a Mon Sep 17 00:00:00 2001 From: james7132 Date: Wed, 9 Mar 2022 13:00:06 -0800 Subject: [PATCH 01/20] Start #53: Generalized Hierarchies --- rfcs/53-generalized-hierarchy.md | 273 +++++++++++++++++++++++++++++++ 1 file changed, 273 insertions(+) create mode 100644 rfcs/53-generalized-hierarchy.md diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md new file mode 100644 index 00000000..be5f5670 --- /dev/null +++ b/rfcs/53-generalized-hierarchy.md @@ -0,0 +1,273 @@ +# Feature Name: `generalized-hierarchies` + +## Summary +Create a general hierarchy or hierarchies for Bevy worlds that is ergonomic to +use and always globally consistent. + +## Motivation +Hierarchies are present in all parts of game development. The one that +immediately comes to mind is the Transform hierarchy, where children exist in the +local space of their parents. However, this concept is can be more general than +transform hierarchies: + + - UI elements have their own parent-sibling-child relationships that are not + strictly expressible in 3D affine transforms. + - Animations are usually bound to a bone hierarchy that may or may not involve + Transforms, and is typically queried based on paths within the hierarchy. + - Certain attributes intuitively should be inherited from parent to child. For + example, hiding a parent via the `Visibility` component typically means the + children of that parent (and all of their descendants) are also hidden. Many + of these properties are not directly tied to transforms or their propagation. + However, many of these use cases also share a similar pattern of detecting + changes and propagating them either down to their descendants or bubbling up a + message through their ancestors. + +All of these cases involve mutating, querying, and traversing these large trees +or forests of linked entities. Throughout each of these operations, it's +imperative that Bevy provides a globally consistent view of the +hierarchy/hiearchies. + +Many of these operations are not cheap to do at runtime. Queries and traversals +can be linear in the number of children and involve heavy random access patterns +which can be detrimental to game performance. + +## Background +In this RFC, we will be using the following terms repeatedly: + + - Parent: this refers to the upper level Entity relative to their children. Each + Entity can either have zero or one parents, but may have zero or more + children. + - Child: this refers to a lower level Entity relative to their parent. Any child + will have exactly one parent. Children can have children of their own. + - Sibling: a child that shares the same parent as another different entity. + - Roots: These are entities without parents. May or may not have children. + - Leaves: These are entities without children. May or may not have a parent. + +Currently, there are two components under `bevy_transform` that describe Bevy's +current hierarchy: `Parent` and `Children`. Adding or altering the `Parent` +component will not immediately manifest in an update to `Children`. + +A dedicated transform hierarchy maintenance system is run every game tick to +update `Children` to match the actual Entities that point to said Entity as a +Parent. separate `PreviousParent` component is used to keep track of which +entity was the entity's parent before it's current one. This is created, managed, +and removed by the maintenance system. This introduces state where both +components on different entities are out of sync, and relies on the maintenance +system to make it eventually consistent. + +In the meantime, querying for either results in a inconsistent state of the +world, and is the subject of much frustration from those not intimately familiar +with the inner workings of the system. + +## User-facing explanation +Instead of using three separate components to manage the hierachy, this proposal +aims to have only one: `Hierarchy`. This hierarchy component provides *read-only* +access into an entity's immediate hierarchial relationships: its parent, +previous sibling, next sibling, and it's first child. All of these may return +`Option`, signalling that such relationships do not exist. This creates +a single component that creates a top-down forest of entity trees, where each +entity forms a doubly-linked list of siblings. It may also cache infromation +about the component's position within the hierarchy like it's depth. + +Mutations to the hierarchy: de-parenting, re-parenting, re-ordering of children, +etc. can only be done via Commands. This enforces a globally consistent view at +any given time. Delaying any modifications to a global synchronization point, +much like component removal. The `Hierarchy` component is not publicly +constructible and must be made via the dedicated hierarchy management commnds. + +To make querying for specific parts of the hierarchy easier, ZST marker +components will be added and removed to make querying for specific cases easier, +and will be kept in sync with the hierarchy state: `Parent`, `Child`, +`FirstSibling`, `LastSibling`. These can be used in conjunction with ECS Query +filters to speed up finding targetted use cases. For example, `Without` +can be used to query for roots, or `With` can be used to find all of the +non-leaf entities. + +To signal changes in the hierachy, a `HierarchyEvent` event type is registered +and any changes made to the hierarchy is published as an event. + +## Implementation strategy +The core of this proposal is the `Hierarchy` component, which roughly looks like +this: + +```rust +#[derive(Component)] +pub struct Hierarchy { + depth: u16, + parent: Option, + prev: Option, + next: Option + first_child: Option, +} +``` + +This type has no public constructors and has no public APIs that allow for +mutation of any of the internal data, only getters. The consistency between +multiple instances of the component is maintained by a set of dedicated hierachy +management commands. The following should be added as extensions to +EntityCommands: + + - `set_parent(Option)` - sets the parent of a given entity. + - If `Hierarchy` is on neither parent or child, it will be added and both + will be atomically updated to reflect this change. + - If the parent already had children, the new child will be prepended at + the start of the linked list, and the sibling pointers of both will be + updated. + - If the child already had a parent, that previous parent will be updated + too. + - A HierarchyEvent is added to the queue to reflect this update. + - `move_to_first(Entity)` - Moves one of the children to the front. + - `move_to_last(Entity)` - Moves one of the children to the end. + - `remove_all_children()` - Disconnects all children from a parent. All + children become roots. + - A HierarchyEvent is added to the queue to reflect this update. + +All of these commands will also update the four marker components appropriately. +Optionally, we could also mark these components as changed (even though they're +ZSTs) to leverage change detection. + +One optional extension to this design is to create dedicated `HierarchyQuery`, which is a custom SystemParam that wraps multiple Query objects to simplify +querying for items in the hierarchy: + + - `iter_descendants(_mut)` - returns an iterator over `(parent, child)` results + from a query parameter. This does a depth-first traversal over the hierarchy + starting at some entity. + - `iter_ancestors(_mut)` - returns an iterator over `(parent, child)` results + from a query parameter. This bubbles up queries from the bottom to the top + starting at some entity. + +As these operations are handled at any command buffer flush. The hierarchy +maintenance system can be removed. + +## Benefits +The biggest benefit to this new design is guarenteed global consistency. The +hierarchy cannot be arbitrarily mutated between synchronization points. + +The second benefit is that any update to the hierarchy is "atomic" and is `O(1)` +for the majority of the operations handled. + +## Drawbacks +There are a notable number of drawbacks to this design: + + - Counting children is now `O(n)`. Finding and editing children order is still + `O(n)`, that has not changed. + - The fact that the hierarchy is not optimally stored in memory is still an + issue, and all hiearchy traversals require heavy random access into memory. + - Updates are still not immediately visible and deferred until the next command + buffer flushes. + - Hierarchy updates are now single threaded. + - The marker components may cause heavy archetype fragmentation. + - Some potential implementations of this hierarchy requires normal ECS commands + to be aware of the hierarchy. For example, despawning a child in the middle of + the linked list without updating it's immediate siblings will violate several + invariants of the system. + +## Rationale and alternatives +The primary goal of this proposal is to get rid of the hierarchy maintenance +system and the consistency issues that come with it's design, which it delivers +on. + +### Keep `Parent`/`Children` as is. Implement Commands/Events. +This simply keeps the components as is but restricts both of them to be +read-only. The commands and events seen in this proposal can also be implemented +on top of them. The internals of `Children` could be replaced with a `HashSet` +to keep operations close to `O(1)`, but a consistent ordering will be lost here. +Marker components will not be needed and normal change detection can be used. The +events can also be implemented as a more consistent replacement to +`PreviousParent`. + +#### Benefits +This rocks the boat the least and provides the same benefit. + +#### Drawbacks +TODO: Complete this section. + +### Type Markers in Components +It may be possible to include type markers as a part of the hierarchy components +to differentiate multiple hierachies from each other. This would allow an entity +to be part of multiple hierarchies at the same time. + +```rust +#[derive(Component)] +pub struct Hierarchy { + depth: u16, + parent: Option, + prev: Option, + next: Option + first_child: Option, + _marker: PhantomData, +} +``` + +#### Benefits +This allows us to directly tie hierarchies to the components they're targetting. +Instead of a general `Hierarchy`, a `Hierarchy` declares it's meant +for `bevy_transform`'s transform components. + +It also allows multiple overlapping hierarchies to be made that may differ from +each other depending on the use case. + +#### Drawbacks +This can be quite confusing to show in an editor and is reasonably complex to +understand from a user perspective. A compelling use case might be needed here. + +### Hierarchical ECS Storage +This involves directly adding a third ECS storage option. In this solution, a +data-oriented SoA structure of split BlobVec columns is used, virtually identical +to the way Tables are currently stored. However, entities are stored in +depth-first order, and an implicit `Option` parent reference is stored +alongisde every Entity. Hierarchy roots are stored at the beginning of the +slices, and leaves near the end. When a entity's components are mutated, it's +marked "dirty" by moving it to the end of the slice. A separate "first dirty" +index is kept to track a list of all dirty hierarchy entities as a form of change +detection. Finally, when iterating over the storage, if a child is found before +it's parent, it's swapped with it's parent to enforce the depth-first invariant. + +#### Benefits +This enables linear iteration time over hierarchical components akin to that of a +typical ECS table, allowing depth first iteration without random accesses. + +#### Drawbacks +This method is very write heavy. Components are moved whenever there is a +mutation, and shuffled around to ensure that the depth first iteration invariant +is maintained. This may be impractical for hierarchy dependent components that +are very big in size. + +The need to mutate and shuffle the storage during iteration also means parallel +iteration cannot be supported on it, so going wide might not be possible. Even +read-only queries against this storage requires exclusive access to the table. + +### ECS Relations +ECS relations can be used to represent the exact same links in the hierarchy, and +might be usable as a different backing for the hierarchy, provided the same +consistency guarentees. + +#### Benefits +Relations are generic, expressing more than just hierarchical relations, but any +link between multiple entities. The exact benefits and drawbacks relative to this +design is reliant on the exact implementation. + +## Prior art +The original design that this proposal is based on is this 2014 article from +[bitsquid +developers](https://bitsquid.blogspot.com/2014/10/building-data-oriented-entity-system.html). +The memory layout sugggested is awfully similar to bevy's ECS tables. However, +components are regularly swapped based on their dirty status. Placing dirty +entities near the end of each contiguous slice. The hierarchical ECS storage +alterantive aims to implement this. + +A similar [blog +post](https://blog.molecular-matters.com/2013/02/22/adventures-in-data-oriented-design-part-2-hierarchical-data/) +from 2013 suggests a similar approach. + +Unity offers similar APIs to the `HierarchyQuery`, with similar performance +penalties for heavy use. + + - [GameObject.GetComponentsInChildren](https://docs.unity3d.com/ScriptReference/GameObject.GetComponentsInChildren.html) + - [GameObject.GetComponentsInParent](https://docs.unity3d.com/ScriptReference/GameObject.GetComponentsInParent.html) + +## Unresolved questions + + - How do we resolve ordering problems for the same entity? This ties into the + question of Command determinism. From 89ba359b69f8587028e2abf6991cfec9f82ab953 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 16:56:51 -0500 Subject: [PATCH 02/20] Bold terminology in background. Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index be5f5670..74593e4c 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -34,14 +34,14 @@ which can be detrimental to game performance. ## Background In this RFC, we will be using the following terms repeatedly: - - Parent: this refers to the upper level Entity relative to their children. Each + - **Parent:** this refers to the upper level Entity relative to their children. Each Entity can either have zero or one parents, but may have zero or more children. - - Child: this refers to a lower level Entity relative to their parent. Any child + - **Child:** this refers to a lower level Entity relative to their parent. Any child will have exactly one parent. Children can have children of their own. - - Sibling: a child that shares the same parent as another different entity. - - Roots: These are entities without parents. May or may not have children. - - Leaves: These are entities without children. May or may not have a parent. + - **Sibling:** a child that shares the same parent as another different entity. + - **Roots:** These are entities without parents. May or may not have children. + - **Leaves:** These are entities without children. May or may not have a parent. Currently, there are two components under `bevy_transform` that describe Bevy's current hierarchy: `Parent` and `Children`. Adding or altering the `Parent` From afe04c46e4916f2c72f713dcd8378dcc1ecd77fa Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 16:57:05 -0500 Subject: [PATCH 03/20] Uncapitalize Commands. Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index 74593e4c..8281b003 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -70,7 +70,7 @@ entity forms a doubly-linked list of siblings. It may also cache infromation about the component's position within the hierarchy like it's depth. Mutations to the hierarchy: de-parenting, re-parenting, re-ordering of children, -etc. can only be done via Commands. This enforces a globally consistent view at +etc. can only be done via commands. This enforces a globally consistent view at any given time. Delaying any modifications to a global synchronization point, much like component removal. The `Hierarchy` component is not publicly constructible and must be made via the dedicated hierarchy management commnds. From 8365dd4606f736a71e1237c08447595f3fad6dc7 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 16:57:28 -0500 Subject: [PATCH 04/20] Code-ize EntityCommands. Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index 8281b003..8dea1bdb 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -105,7 +105,7 @@ This type has no public constructors and has no public APIs that allow for mutation of any of the internal data, only getters. The consistency between multiple instances of the component is maintained by a set of dedicated hierachy management commands. The following should be added as extensions to -EntityCommands: +`EntityCommands`: - `set_parent(Option)` - sets the parent of a given entity. - If `Hierarchy` is on neither parent or child, it will be added and both From c3fd4c3c4f92e1514187825082a205a7191ae887 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 16:57:55 -0500 Subject: [PATCH 05/20] remove_all -> disconnect_children Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index 8dea1bdb..d1a13d1b 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -118,7 +118,7 @@ management commands. The following should be added as extensions to - A HierarchyEvent is added to the queue to reflect this update. - `move_to_first(Entity)` - Moves one of the children to the front. - `move_to_last(Entity)` - Moves one of the children to the end. - - `remove_all_children()` - Disconnects all children from a parent. All + - `disconnect_children()` - Disconnects all children from a parent. All children become roots. - A HierarchyEvent is added to the queue to reflect this update. From 055e1873ce87145e6d4073626c3354e661bf1a75 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 16:58:30 -0500 Subject: [PATCH 06/20] More code formatting. Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index d1a13d1b..4063965a 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -127,7 +127,7 @@ Optionally, we could also mark these components as changed (even though they're ZSTs) to leverage change detection. One optional extension to this design is to create dedicated `HierarchyQuery`, which is a custom SystemParam that wraps multiple Query objects to simplify +F>`, which is a custom `SystemParam` that wraps multiple `Query` objects to simplify querying for items in the hierarchy: - `iter_descendants(_mut)` - returns an iterator over `(parent, child)` results From 6c3cfcf083456d3080daede14098b34070a896b0 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 16:59:11 -0500 Subject: [PATCH 07/20] More code formatting. Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index 4063965a..e8de2dbd 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -120,7 +120,7 @@ management commands. The following should be added as extensions to - `move_to_last(Entity)` - Moves one of the children to the end. - `disconnect_children()` - Disconnects all children from a parent. All children become roots. - - A HierarchyEvent is added to the queue to reflect this update. + - A `HierarchyEvent` is added to the queue to reflect this update. All of these commands will also update the four marker components appropriately. Optionally, we could also mark these components as changed (even though they're From c49bd8e56aa3cd6d4e70b2a7a5f6e4352a2e17da Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 16:59:44 -0500 Subject: [PATCH 08/20] Update motivation wording Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index e8de2dbd..067f02c2 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -27,7 +27,7 @@ or forests of linked entities. Throughout each of these operations, it's imperative that Bevy provides a globally consistent view of the hierarchy/hiearchies. -Many of these operations are not cheap to do at runtime. Queries and traversals +Many of these operations are both abundantly common and moderately expensive. Queries and traversals can be linear in the number of children and involve heavy random access patterns which can be detrimental to game performance. From ce9aee792e46bc959d4385f7c31ffd68a4599d07 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 9 Mar 2022 17:00:17 -0500 Subject: [PATCH 09/20] Fix grammar Co-authored-by: Alice Cecile --- rfcs/53-generalized-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index 067f02c2..88f27f77 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -137,7 +137,7 @@ querying for items in the hierarchy: from a query parameter. This bubbles up queries from the bottom to the top starting at some entity. -As these operations are handled at any command buffer flush. The hierarchy +As these operations are handled at any command buffer flush, the hierarchy maintenance system can be removed. ## Benefits From 51a538ec1408d7ac582c6ab9f788c7d53e84f76a Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Mar 2022 21:19:22 -0800 Subject: [PATCH 10/20] Address review comments and optimization insights --- rfcs/53-generalized-hierarchy.md | 253 +++++++++++++++++++++---------- 1 file changed, 176 insertions(+), 77 deletions(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index 88f27f77..d4a8d701 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -27,9 +27,9 @@ or forests of linked entities. Throughout each of these operations, it's imperative that Bevy provides a globally consistent view of the hierarchy/hiearchies. -Many of these operations are both abundantly common and moderately expensive. Queries and traversals -can be linear in the number of children and involve heavy random access patterns -which can be detrimental to game performance. +Many of these operations are both abundantly common and moderately expensive. +Queries and traversals can be linear in the number of children and involve heavy +random access patterns which can be detrimental to game performance. ## Background In this RFC, we will be using the following terms repeatedly: @@ -87,49 +87,32 @@ To signal changes in the hierachy, a `HierarchyEvent` event type is registered and any changes made to the hierarchy is published as an event. ## Implementation strategy -The core of this proposal is the `Hierarchy` component, which roughly looks like -this: - -```rust -#[derive(Component)] -pub struct Hierarchy { - depth: u16, - parent: Option, - prev: Option, - next: Option - first_child: Option, -} -``` - -This type has no public constructors and has no public APIs that allow for -mutation of any of the internal data, only getters. The consistency between -multiple instances of the component is maintained by a set of dedicated hierachy -management commands. The following should be added as extensions to -`EntityCommands`: - - - `set_parent(Option)` - sets the parent of a given entity. - - If `Hierarchy` is on neither parent or child, it will be added and both - will be atomically updated to reflect this change. - - If the parent already had children, the new child will be prepended at - the start of the linked list, and the sibling pointers of both will be - updated. - - If the child already had a parent, that previous parent will be updated - too. - - A HierarchyEvent is added to the queue to reflect this update. - - `move_to_first(Entity)` - Moves one of the children to the front. - - `move_to_last(Entity)` - Moves one of the children to the end. - - `disconnect_children()` - Disconnects all children from a parent. All - children become roots. - - A `HierarchyEvent` is added to the queue to reflect this update. - -All of these commands will also update the four marker components appropriately. -Optionally, we could also mark these components as changed (even though they're -ZSTs) to leverage change detection. - -One optional extension to this design is to create dedicated `HierarchyQuery`, which is a custom `SystemParam` that wraps multiple `Query` objects to simplify +This design attempts to minimize the changes made to the current design while +addressing the issues enumerated above. The core changes to be made are as +follows: + + - Make `Parent` and `Children` (mostly) publicly immutable. + - Make mutations to the hierarchy rely solely on commands. + - Update hierarchy commands to automatically update both `Parent` and + `Children` to remain consistent with each other. + - Remove the `parent_update_system`. + - Remove `PreviousParent`. + - Register several hierarchy events that fires whenever time a new child is + added or removed from a parent. + - Create a `HiearchyQuery` custom SystemParam for more easily iterating over + components in the hierarchy. + +### HierarchyQuery +An extension to this design is to create dedicated `HierarchyQuery`, which +is a custom `SystemParam` that wraps multiple `Query` objects to simplify querying for items in the hierarchy: + - `roots(_mut)` - returns an iterator over entities that match the provided + query parameters that do not have a parent. + - `parent(_mut)` - returns single result from the parent of an entity. + - `iter_children(_mut)` - returns an iterator over `(parent, child)` results + from a query parameter. This does a depth-first traversal over the hierarchy + starting at some entity. - `iter_descendants(_mut)` - returns an iterator over `(parent, child)` results from a query parameter. This does a depth-first traversal over the hierarchy starting at some entity. @@ -137,45 +120,139 @@ querying for items in the hierarchy: from a query parameter. This bubbles up queries from the bottom to the top starting at some entity. -As these operations are handled at any command buffer flush, the hierarchy -maintenance system can be removed. +As suggested by the generic parameters, the query results and filters are +identical to the ones used by a normal Query or QueryState. + +The main motivation for this is to make creating systems like +`transform_propagate_system` easier. These queries can be generically used for +any system that needs some form of hierachy traversal. These queries can be quite +expensive to run, so the documentation for them should reflect this and +discourage misuing it. + +### HierarchyEvents +Three new events should be added here, each corresponding to a specific change +in the hierarchy. + +```rust +pub enum ChildAdded { + pub parent: Entity, + pub child: Entity, +} + +pub enum ChildRemoved { + pub parent: Entity, + pub child: Entity, +} + +pub enum ChildMoved { + pub parent: Entity, + pub previous_parent: Entity, + pub new_parent: Entity, +} +``` + +Both `ChildAdded` and `ChildRemoved` will be fired when a child moved occurs. +These can be used to detect changes where `PreviousParent` is currently used. + +### `bevy_hierarchy`-`bevy_transform` split. +These hierarchy types should be moved to a new `bevy_hierarchy` crate to avoid +requiring compiling the hierachy alongside the default "f32-based affine +transform system in Euclidean space". This should allow users to create their own +transform systems (i.e. f64-based transforms, fixed-point transforms, +non-Euclidean spaces) while also being able to leverage the same +parent-sibling-child hierarchies currently found in `bevy_transform`. This should +also allow us to internally leverage the same decoupling too for specialized +transforms for 2D and UI. ## Benefits -The biggest benefit to this new design is guarenteed global consistency. The -hierarchy cannot be arbitrarily mutated between synchronization points. -The second benefit is that any update to the hierarchy is "atomic" and is `O(1)` -for the majority of the operations handled. + - This design has (nearly) guarenteed global consistency. The hierarchy cannot + be arbitrarily mutated in inconsistent between synchronization points. + - The cache locality when iterating over the immediate children of an entity is + retained. This saves an extra cache miss per entity when traversing the + hierarchy. + - Any update to the hierarchy is "atomic" and is `O(n)` in the number of + children an entity has. For any practical hierarchy, this will typically be + a very small n. + - Existing query filters still work as is. For example, `Without` can + still be used to find root entities, and `Changed` can be used to + find any entity with either new children, removed children, or reordered + children. + - The hierarchy is decoupled from the base transform system, allowing both Bevy + developer and third-party crates to make their own independent transform + system without compiling the default transform system. ## Drawbacks -There are a notable number of drawbacks to this design: - - Counting children is now `O(n)`. Finding and editing children order is still - `O(n)`, that has not changed. - The fact that the hierarchy is not optimally stored in memory is still an issue, and all hiearchy traversals require heavy random access into memory. - Updates are still not immediately visible and deferred until the next command buffer flushes. - Hierarchy updates are now single threaded. - - The marker components may cause heavy archetype fragmentation. - Some potential implementations of this hierarchy requires normal ECS commands - to be aware of the hierarchy. For example, despawning a child in the middle of - the linked list without updating it's immediate siblings will violate several - invariants of the system. + to be aware of the hierarchy. Using `EntityCommands::remove` to remove + `Children` or `Parent` will break the invariants of the system without some + hooks to enforce the invariants on removal. ## Rationale and alternatives The primary goal of this proposal is to get rid of the hierarchy maintenance system and the consistency issues that come with it's design, which it delivers on. -### Keep `Parent`/`Children` as is. Implement Commands/Events. -This simply keeps the components as is but restricts both of them to be -read-only. The commands and events seen in this proposal can also be implemented -on top of them. The internals of `Children` could be replaced with a `HashSet` -to keep operations close to `O(1)`, but a consistent ordering will be lost here. -Marker components will not be needed and normal change detection can be used. The -events can also be implemented as a more consistent replacement to -`PreviousParent`. +### Hierarchy as a Resource +This would store the resource of the entire world as a dedicated Resource +instead of as components in normal ECS storage. A prototypical design for this +resource would be: + +```rust +pub struct Hierarchy { + relations: Vec<(Option, Vec)>, + entities: Vec, + indices: SparseArray, +} +``` + +This would be strikingly similar to the `SparseSet` used in `bevy_ecs`. The main +difference here is that the parallel `Vec`s are kept in topological order. When +iterating over the hierarchy, children that are found before their parent will +swapped with their parents. + +An alternative version of this hierarchy resource only stores parents. This may +speed up iteration as individual relations are smaller during swaps, but finding +children in the hierarchy is O(n) over the entire world. If the depth of each +entity within the hierarchy, and the hierarchy is kept sorted in depth order this +can be kept to O(n) in the next depth. + +#### Benefits + + - Iteration in hierarchy order (parents first) is entirely linear. This may make + transform and other parent-reliant propagation signfigantly faster. + +#### Drawbacks + + - Merging and splitting hierarchies loaded from scenes can be quite difficult to + maintain. + - Iteration requires mutative access to the resource, which can limit + parallelism of systems traversing the hierarchy. The alternative requires a + dedicated O(n) sweep over the hierarchy whenever it's mutated. + - Hierarhcy + +### Linked List Hierarchy +The core of this alternative is the `Hierarchy` component, which roughly looks +like this: + +```rust +#[derive(Component)] +pub struct Hierarchy { + parent: Option, + prev: Option, + next: Option + first_child: Option, +} +``` + +Like the main proposal here, there are no public constructors or mutation APIs, +relying only on commands to mutate the internals. #### Benefits This rocks the boat the least and provides the same benefit. @@ -183,6 +260,21 @@ This rocks the boat the least and provides the same benefit. #### Drawbacks TODO: Complete this section. +### HashSet `Children` +The internals of `Children` could be replaced with a `HashSet`. +operations close to `O(1)`, versus `O(n)` operations against a `SmallVec`. + +#### Benefits +This makes all operations against `Children` O(1). Could speed up a number of +hierarchy related commands. + +#### Drawbacks + + - We lose any ability to order children. This is critically important for + certain use cases like UI. + - We lose cache locality for entities with few children. Adding a single child + forces an allocation. + ### Type Markers in Components It may be possible to include type markers as a part of the hierarchy components to differentiate multiple hierachies from each other. This would allow an entity @@ -190,19 +282,21 @@ to be part of multiple hierarchies at the same time. ```rust #[derive(Component)] -pub struct Hierarchy { - depth: u16, - parent: Option, - prev: Option, - next: Option - first_child: Option, +pub struct Parent { + parent: Entity, + _marker: PhantomData, +} + +#[derive(Component)] +pub struct Children { + children: SmallVec<[Entity, 8]>, _marker: PhantomData, } ``` #### Benefits This allows us to directly tie hierarchies to the components they're targetting. -Instead of a general `Hierarchy`, a `Hierarchy` declares it's meant +Instead of a general `Parent`, a `Parent` declares it's meant for `bevy_transform`'s transform components. It also allows multiple overlapping hierarchies to be made that may differ from @@ -224,15 +318,20 @@ index is kept to track a list of all dirty hierarchy entities as a form of chang detection. Finally, when iterating over the storage, if a child is found before it's parent, it's swapped with it's parent to enforce the depth-first invariant. +This is very similar to the "Hierarchy as a Resource" alternative, but + #### Benefits -This enables linear iteration time over hierarchical components akin to that of a -typical ECS table, allowing depth first iteration without random accesses. + + - Iteration in hierarchy order (parents first) is entirely linear. This may make + transform and other parent-reliant propagation signfigantly faster. + - Unlike the hierarchy as a resource, this guarentees linearity of all iterated + components, not just the hierarchy relations itself. #### Drawbacks This method is very write heavy. Components are moved whenever there is a -mutation, and shuffled around to ensure that the depth first iteration invariant -is maintained. This may be impractical for hierarchy dependent components that -are very big in size. +mutation, and shuffled around to ensure that the topologically ordered iteration +invariant is maintained. This may be impractical for hierarchy dependent +components that are very big in size. The need to mutate and shuffle the storage during iteration also means parallel iteration cannot be supported on it, so going wide might not be possible. Even From dc7be60f566f5f008d693aa28304d31c2fb8feec Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Mar 2022 21:38:06 -0800 Subject: [PATCH 11/20] Fix up TODOs --- rfcs/53-generalized-hierarchy.md | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index d4a8d701..d4ba3995 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -166,6 +166,8 @@ transforms for 2D and UI. ## Benefits + - This rocks the boat the least compared to the following alternatives and + largely provides the same benefits. - This design has (nearly) guarenteed global consistency. The hierarchy cannot be arbitrarily mutated in inconsistent between synchronization points. - The cache locality when iterating over the immediate children of an entity is @@ -186,13 +188,14 @@ transforms for 2D and UI. - The fact that the hierarchy is not optimally stored in memory is still an issue, and all hiearchy traversals require heavy random access into memory. - - Updates are still not immediately visible and deferred until the next command - buffer flushes. + - Updates are still not immediately visible within a stage and deferred until + the next command buffer flush. - Hierarchy updates are now single threaded. - Some potential implementations of this hierarchy requires normal ECS commands to be aware of the hierarchy. Using `EntityCommands::remove` to remove `Children` or `Parent` will break the invariants of the system without some - hooks to enforce the invariants on removal. + hooks to enforce the invariants on removal. This is partially mitigatable via + documentation until hooks or relations lands. ## Rationale and alternatives The primary goal of this proposal is to get rid of the hierarchy maintenance @@ -227,6 +230,7 @@ can be kept to O(n) in the next depth. - Iteration in hierarchy order (parents first) is entirely linear. This may make transform and other parent-reliant propagation signfigantly faster. + - Can be used with the command-driven interface proposed in this design. #### Drawbacks @@ -235,7 +239,6 @@ can be kept to O(n) in the next depth. - Iteration requires mutative access to the resource, which can limit parallelism of systems traversing the hierarchy. The alternative requires a dedicated O(n) sweep over the hierarchy whenever it's mutated. - - Hierarhcy ### Linked List Hierarchy The core of this alternative is the `Hierarchy` component, which roughly looks @@ -255,10 +258,24 @@ Like the main proposal here, there are no public constructors or mutation APIs, relying only on commands to mutate the internals. #### Benefits -This rocks the boat the least and provides the same benefit. + + - Hierarchy updates are all `O(1)`. + - The Hierarchy component is smaller in ECS storage than the current + Parent/Children combination. With Entity-niching, a `Hierarchy` component is + only 32 bytes in memory, compared to the 64 + 8 used by the current two + components. + - Requires less archetype fragmentation due to only requiring one component + instaed of two. + - Getting siblings does not require a parent Query::get call. #### Drawbacks -TODO: Complete this section. + + - Iterating over children requires `O(n)` Query::get calls. + - Despawning a child without updating it's sibilings causes all subsequent + children to be in a broken state. This exacerbates the lack of hooks, and + would require query access in hooks to fix up. + - Change detection and With/Without filters no longer work as is. Will need + dedicated ZST marker components to get the same filtering capacity. ### HashSet `Children` The internals of `Children` could be replaced with a `HashSet`. From ad6257c1eb4cb19bb3a070495640996fb844690b Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Mar 2022 22:23:22 -0800 Subject: [PATCH 12/20] Update user facing explaination --- rfcs/53-generalized-hierarchy.md | 53 ++++++++++++++++---------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index d4ba3995..9f4f985a 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -60,31 +60,18 @@ world, and is the subject of much frustration from those not intimately familiar with the inner workings of the system. ## User-facing explanation -Instead of using three separate components to manage the hierachy, this proposal -aims to have only one: `Hierarchy`. This hierarchy component provides *read-only* -access into an entity's immediate hierarchial relationships: its parent, -previous sibling, next sibling, and it's first child. All of these may return -`Option`, signalling that such relationships do not exist. This creates -a single component that creates a top-down forest of entity trees, where each -entity forms a doubly-linked list of siblings. It may also cache infromation -about the component's position within the hierarchy like it's depth. - -Mutations to the hierarchy: de-parenting, re-parenting, re-ordering of children, -etc. can only be done via commands. This enforces a globally consistent view at -any given time. Delaying any modifications to a global synchronization point, -much like component removal. The `Hierarchy` component is not publicly -constructible and must be made via the dedicated hierarchy management commnds. - -To make querying for specific parts of the hierarchy easier, ZST marker -components will be added and removed to make querying for specific cases easier, -and will be kept in sync with the hierarchy state: `Parent`, `Child`, -`FirstSibling`, `LastSibling`. These can be used in conjunction with ECS Query -filters to speed up finding targetted use cases. For example, `Without` -can be used to query for roots, or `With` can be used to find all of the -non-leaf entities. - -To signal changes in the hierachy, a `HierarchyEvent` event type is registered -and any changes made to the hierarchy is published as an event. +Both of the current components' public interface will be made *read-only*. +Structural mutations to the hierarchy: de-parenting, re-parenting, and moving +children from one parent to another, etc. can only be done via commands. This +defers all changes to the hierarchy until the next command buffer flush. +This enforces a globally consistent view at any given time. Delaying any +modifications to a global synchronization point, much like component removal. +Both components will not publicly constructible and must be made via the +dedicated hierarchy management commands. + +`PreviousParent` will also be removed and replaced with `ChildAdded`, +`ChildRemoved` and `ChildMoved` events instead. These will signal changes in the +hierachy instead of relying on change detection on `Parent` and `PreviousParent`. ## Implementation strategy This design attempts to minimize the changes made to the current design while @@ -170,9 +157,13 @@ transforms for 2D and UI. largely provides the same benefits. - This design has (nearly) guarenteed global consistency. The hierarchy cannot be arbitrarily mutated in inconsistent between synchronization points. + Changes are deferred to a until the end of the current stage instead of a + single global maintainence system. - The cache locality when iterating over the immediate children of an entity is retained. This saves an extra cache miss per entity when traversing the hierarchy. + - The removal of `PreviousParent` should reduce the archetype fragmentation of + the current solution. - Any update to the hierarchy is "atomic" and is `O(n)` in the number of children an entity has. For any practical hierarchy, this will typically be a very small n. @@ -191,6 +182,8 @@ transforms for 2D and UI. - Updates are still not immediately visible within a stage and deferred until the next command buffer flush. - Hierarchy updates are now single threaded. + - The commands applied for controlling the hierarchy are computationally more + expensive, requiring more branching access and more `World::get` calls. - Some potential implementations of this hierarchy requires normal ECS commands to be aware of the hierarchy. Using `EntityCommands::remove` to remove `Children` or `Parent` will break the invariants of the system without some @@ -241,8 +234,14 @@ can be kept to O(n) in the next depth. dedicated O(n) sweep over the hierarchy whenever it's mutated. ### Linked List Hierarchy -The core of this alternative is the `Hierarchy` component, which roughly looks -like this: +Instead of using two separate components to manage the hierarchy, this alternative +aims to have only one: `Hierarchy`. This hierarchy component provides *read-only* +access into an entity's immediate hierarchial relationships: its parent, +previous sibling, next sibling, and it's first child. All of these may return +`Option`, signalling that such relationships do not exist. This creates +a single component that creates a top-down forest of entity trees, where each +entity forms a doubly-linked list of siblings. It may also cache infromation +about the component's position within the hierarchy like it's depth. ```rust #[derive(Component)] From 8fc0e25a4d09bf3a513a9cd06910d3db2b68bab4 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Mar 2022 22:30:42 -0800 Subject: [PATCH 13/20] update summary --- rfcs/53-generalized-hierarchy.md | 33 +++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-generalized-hierarchy.md index 9f4f985a..b27f6465 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-generalized-hierarchy.md @@ -1,8 +1,10 @@ # Feature Name: `generalized-hierarchies` ## Summary -Create a general hierarchy or hierarchies for Bevy worlds that is ergonomic to -use and always globally consistent. +The current transform hierarchy operations currently allow for it to remain in a +inconsistent state. This RFC proposes a few changes to make it globally +consistent at any given time, as well as make it easier to support non-Tranform +uese cases. ## Motivation Hierarchies are present in all parts of game development. The one that @@ -195,7 +197,7 @@ The primary goal of this proposal is to get rid of the hierarchy maintenance system and the consistency issues that come with it's design, which it delivers on. -### Hierarchy as a Resource +### Alternative: Hierarchy as a Resource This would store the resource of the entire world as a dedicated Resource instead of as components in normal ECS storage. A prototypical design for this resource would be: @@ -233,14 +235,14 @@ can be kept to O(n) in the next depth. parallelism of systems traversing the hierarchy. The alternative requires a dedicated O(n) sweep over the hierarchy whenever it's mutated. -### Linked List Hierarchy -Instead of using two separate components to manage the hierarchy, this alternative -aims to have only one: `Hierarchy`. This hierarchy component provides *read-only* -access into an entity's immediate hierarchial relationships: its parent, -previous sibling, next sibling, and it's first child. All of these may return -`Option`, signalling that such relationships do not exist. This creates -a single component that creates a top-down forest of entity trees, where each -entity forms a doubly-linked list of siblings. It may also cache infromation +### Alternative: Linked List Hierarchy +Instead of using two separate components to manage the hierarchy, this +alternative aims to have only one: `Hierarchy`. This hierarchy component provides +*read-only* access into an entity's immediate hierarchial relationships: its +parent, previous sibling, next sibling, and it's first child. All of these may +return `Option`, signalling that such relationships do not exist. This +creates a single component that creates a top-down forest of entity trees, where +each entity forms a doubly-linked list of siblings. It may also cache infromation about the component's position within the hierarchy like it's depth. ```rust @@ -258,7 +260,8 @@ relying only on commands to mutate the internals. #### Benefits - - Hierarchy updates are all `O(1)`. + - Hierarchy updates are all `O(1)`, and can be maintained within a system. Does + not require commands to ensure consistency. - The Hierarchy component is smaller in ECS storage than the current Parent/Children combination. With Entity-niching, a `Hierarchy` component is only 32 bytes in memory, compared to the 64 + 8 used by the current two @@ -276,7 +279,7 @@ relying only on commands to mutate the internals. - Change detection and With/Without filters no longer work as is. Will need dedicated ZST marker components to get the same filtering capacity. -### HashSet `Children` +### Alternative: HashSet `Children` The internals of `Children` could be replaced with a `HashSet`. operations close to `O(1)`, versus `O(n)` operations against a `SmallVec`. @@ -322,7 +325,7 @@ each other depending on the use case. This can be quite confusing to show in an editor and is reasonably complex to understand from a user perspective. A compelling use case might be needed here. -### Hierarchical ECS Storage +### Alternative: Hierarchical ECS Storage This involves directly adding a third ECS storage option. In this solution, a data-oriented SoA structure of split BlobVec columns is used, virtually identical to the way Tables are currently stored. However, entities are stored in @@ -353,7 +356,7 @@ The need to mutate and shuffle the storage during iteration also means parallel iteration cannot be supported on it, so going wide might not be possible. Even read-only queries against this storage requires exclusive access to the table. -### ECS Relations +### Alternative: ECS Relations ECS relations can be used to represent the exact same links in the hierarchy, and might be usable as a different backing for the hierarchy, provided the same consistency guarentees. From 68d4bcc1737bfe95a3902e4deb87b969631f2315 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 13 Mar 2022 00:52:30 -0800 Subject: [PATCH 14/20] Address review comments --- ...ierarchy.md => 53-consistent-hierarchy.md} | 83 ++++++++++++------- 1 file changed, 51 insertions(+), 32 deletions(-) rename rfcs/{53-generalized-hierarchy.md => 53-consistent-hierarchy.md} (94%) diff --git a/rfcs/53-generalized-hierarchy.md b/rfcs/53-consistent-hierarchy.md similarity index 94% rename from rfcs/53-generalized-hierarchy.md rename to rfcs/53-consistent-hierarchy.md index b27f6465..ae51ef0c 100644 --- a/rfcs/53-generalized-hierarchy.md +++ b/rfcs/53-consistent-hierarchy.md @@ -1,4 +1,4 @@ -# Feature Name: `generalized-hierarchies` +# Feature Name: `consistent-hierarchy` ## Summary The current transform hierarchy operations currently allow for it to remain in a @@ -91,6 +91,20 @@ follows: - Create a `HiearchyQuery` custom SystemParam for more easily iterating over components in the hierarchy. +This change will repurpose the following existing `EntityCommand` extension +functions for this purpose: + + - `add_child` + - `push_children` + - `insert_children` + - `remove_child` (this will be added as a wrapper around `remove_children`) + - `remove_children` + +An additional `set_parent(Option)` command should also be added as a +child-side way of altering the hierarchy, defaulting to adding the entity as the +last child to the provided parent if provided, and detaching the child from it's +parent if `None`. + ### HierarchyQuery An extension to this design is to create dedicated `HierarchyQuery`, which is a custom `SystemParam` that wraps multiple `Query` objects to simplify @@ -294,37 +308,6 @@ hierarchy related commands. - We lose cache locality for entities with few children. Adding a single child forces an allocation. -### Type Markers in Components -It may be possible to include type markers as a part of the hierarchy components -to differentiate multiple hierachies from each other. This would allow an entity -to be part of multiple hierarchies at the same time. - -```rust -#[derive(Component)] -pub struct Parent { - parent: Entity, - _marker: PhantomData, -} - -#[derive(Component)] -pub struct Children { - children: SmallVec<[Entity, 8]>, - _marker: PhantomData, -} -``` - -#### Benefits -This allows us to directly tie hierarchies to the components they're targetting. -Instead of a general `Parent`, a `Parent` declares it's meant -for `bevy_transform`'s transform components. - -It also allows multiple overlapping hierarchies to be made that may differ from -each other depending on the use case. - -#### Drawbacks -This can be quite confusing to show in an editor and is reasonably complex to -understand from a user perspective. A compelling use case might be needed here. - ### Alternative: Hierarchical ECS Storage This involves directly adding a third ECS storage option. In this solution, a data-oriented SoA structure of split BlobVec columns is used, virtually identical @@ -385,6 +368,42 @@ penalties for heavy use. - [GameObject.GetComponentsInChildren](https://docs.unity3d.com/ScriptReference/GameObject.GetComponentsInChildren.html) - [GameObject.GetComponentsInParent](https://docs.unity3d.com/ScriptReference/GameObject.GetComponentsInParent.html) +## Future Work +One of the immediate pieces of future work here is to patch up the potential +footgun of using `EntityCommands::remove` on either `Parent` or `Children` via +add/remove/changed hooks. This is currently mitigatable via documentation, but a +complete solution prevents users from breaking the invariant in the first place. + +### Multiple Hierarchies via Type Markers +It may be possible to include type markers as a part of the hierarchy components +to differentiate multiple hierachies from each other. This would allow an entity +to be part of multiple hierarchies at the same time. + +```rust +#[derive(Component)] +pub struct Parent { + parent: Entity, + _marker: PhantomData, +} + +#[derive(Component)] +pub struct Children { + children: SmallVec<[Entity, 8]>, + _marker: PhantomData, +} +``` + +This allows us to directly tie hierarchies to the components they're targetting. +Instead of a general `Parent`, a `Parent` declares it's meant +for `bevy_transform`'s transform components. + +It also allows multiple overlapping hierarchies to be made that may differ from +each other depending on the use case. + +However, this may be quite confusing to show in an editor and is reasonably +complex to understand from a user perspective. A compelling use case might be +needed to justify this kind of change. + ## Unresolved questions - How do we resolve ordering problems for the same entity? This ties into the From 3f7af390b25835f97c01e6239cd33c41d404ac6f Mon Sep 17 00:00:00 2001 From: James Liu Date: Sun, 13 Mar 2022 14:16:39 -0700 Subject: [PATCH 15/20] Make a distinction between iter_children and iter_descendants Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- rfcs/53-consistent-hierarchy.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rfcs/53-consistent-hierarchy.md b/rfcs/53-consistent-hierarchy.md index ae51ef0c..85c46fa1 100644 --- a/rfcs/53-consistent-hierarchy.md +++ b/rfcs/53-consistent-hierarchy.md @@ -114,8 +114,7 @@ querying for items in the hierarchy: query parameters that do not have a parent. - `parent(_mut)` - returns single result from the parent of an entity. - `iter_children(_mut)` - returns an iterator over `(parent, child)` results - from a query parameter. This does a depth-first traversal over the hierarchy - starting at some entity. + from a query parameter. This does not return Children of Children. - `iter_descendants(_mut)` - returns an iterator over `(parent, child)` results from a query parameter. This does a depth-first traversal over the hierarchy starting at some entity. From d2f6153c1df74838a15591a49043ffe50ccf86bd Mon Sep 17 00:00:00 2001 From: James Liu Date: Sun, 13 Mar 2022 14:16:58 -0700 Subject: [PATCH 16/20] Fix typo Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- rfcs/53-consistent-hierarchy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/53-consistent-hierarchy.md b/rfcs/53-consistent-hierarchy.md index 85c46fa1..e20f62c8 100644 --- a/rfcs/53-consistent-hierarchy.md +++ b/rfcs/53-consistent-hierarchy.md @@ -4,7 +4,7 @@ The current transform hierarchy operations currently allow for it to remain in a inconsistent state. This RFC proposes a few changes to make it globally consistent at any given time, as well as make it easier to support non-Tranform -uese cases. +use cases. ## Motivation Hierarchies are present in all parts of game development. The one that From fc4eed2135741e33195803d9b6ecc2fd8ae0a28d Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 13 Mar 2022 14:21:08 -0700 Subject: [PATCH 17/20] Clarify more drawbacks --- rfcs/53-consistent-hierarchy.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rfcs/53-consistent-hierarchy.md b/rfcs/53-consistent-hierarchy.md index e20f62c8..b11d14da 100644 --- a/rfcs/53-consistent-hierarchy.md +++ b/rfcs/53-consistent-hierarchy.md @@ -196,7 +196,12 @@ transforms for 2D and UI. issue, and all hiearchy traversals require heavy random access into memory. - Updates are still not immediately visible within a stage and deferred until the next command buffer flush. - - Hierarchy updates are now single threaded. + - Hierarchy updates are now single threaded. Commands can be generated from + multiple systems at the same time, but they now require exclusive access to + apply. + - Each of the changed commands do a lot more work before and are signifgantly + more branch heavy. This could negatively impact the performance of + command-heavy workloads (i.e. scene loading) - The commands applied for controlling the hierarchy are computationally more expensive, requiring more branching access and more `World::get` calls. - Some potential implementations of this hierarchy requires normal ECS commands From 232582940eabcf70369ed90caf14228c321f009a Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 13 Mar 2022 14:25:45 -0700 Subject: [PATCH 18/20] Add future work note about differing transform systems --- rfcs/53-consistent-hierarchy.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rfcs/53-consistent-hierarchy.md b/rfcs/53-consistent-hierarchy.md index b11d14da..d9c72968 100644 --- a/rfcs/53-consistent-hierarchy.md +++ b/rfcs/53-consistent-hierarchy.md @@ -378,6 +378,13 @@ footgun of using `EntityCommands::remove` on either `Parent` or `Children` via add/remove/changed hooks. This is currently mitigatable via documentation, but a complete solution prevents users from breaking the invariant in the first place. +### Dedicated RectTransform and Transform2D systems +Transform/GlobalTransform is not particularly well suited for encoding the +hierarchy of UIs and 2D-only scenes. By splitting out the hierarchy from the +exsisting transform system, dedicated transform types and systems for these two +different types of hierarchies. This could also enable further parallelism by +allowing each transform system to independently handle propagation. + ### Multiple Hierarchies via Type Markers It may be possible to include type markers as a part of the hierarchy components to differentiate multiple hierachies from each other. This would allow an entity From 365ff084182ec6f9dfc3db46eb4d02ded1185dea Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 23 May 2022 19:22:40 -0700 Subject: [PATCH 19/20] Address some review comments --- rfcs/53-consistent-hierarchy.md | 83 ++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 26 deletions(-) diff --git a/rfcs/53-consistent-hierarchy.md b/rfcs/53-consistent-hierarchy.md index d9c72968..dbd56ac4 100644 --- a/rfcs/53-consistent-hierarchy.md +++ b/rfcs/53-consistent-hierarchy.md @@ -131,31 +131,47 @@ any system that needs some form of hierachy traversal. These queries can be quit expensive to run, so the documentation for them should reflect this and discourage misuing it. -### HierarchyEvents -Three new events should be added here, each corresponding to a specific change -in the hierarchy. +### HierarchyEvent +One new event should be added here, with enum variants corresponding to a +specific change in the hierarchy: -```rust -pub enum ChildAdded { - pub parent: Entity, - pub child: Entity, -} + - `ChildAdded` - Fired when a previous root entity has been added as a child to + a entity in the hierarchy. + - `ChildRemoved` - Fired when an entity is removed from the hierarchy and is now + a root entity. + - `ChildMoved` - Fired when an child is moved from one parent in the hierarchy + to another. -pub enum ChildRemoved { - pub parent: Entity, - pub child: Entity, -} +These can be used to detect changes where `PreviousParent` is currently used. +Each event is exclusive, meaning that any change only generates one event. A move +will not fire additional separate `ChildAdded` and `ChildRemoved` events. + +Only one event type is used here to ensure that hierarchy alterations are +properly sequenced. -pub enum ChildMoved { - pub parent: Entity, - pub previous_parent: Entity, - pub new_parent: Entity, +As hierarchy edits typically happen in bursts (i.e. scene loads). There may be +large number of these events generated at once. To reclaim this memory, it may be +necessary to add support for automatic garbage collection to events. + +```rust +#[derive(Clone)] +pub enum HierarchyEvent { + ChildAdded { + parent: Entity, + child: Entity, + }, + ChildRemoved { + parent: Entity, + child: Entity, + }, + ChildMoved { + parent: Entity, + previous_parent: Entity, + new_parent: Entity, + } } ``` -Both `ChildAdded` and `ChildRemoved` will be fired when a child moved occurs. -These can be used to detect changes where `PreviousParent` is currently used. - ### `bevy_hierarchy`-`bevy_transform` split. These hierarchy types should be moved to a new `bevy_hierarchy` crate to avoid requiring compiling the hierachy alongside the default "f32-based affine @@ -229,9 +245,10 @@ pub struct Hierarchy { ``` This would be strikingly similar to the `SparseSet` used in `bevy_ecs`. The main -difference here is that the parallel `Vec`s are kept in topological order. When -iterating over the hierarchy, children that are found before their parent will -swapped with their parents. +difference here is that the parallel `Vec`s are roughly kept in topological order. +Inserting should be `O(1)` as it just updates the new parent. Removing should be +`O(n)` with the number of children of that `Entity`. At least once per frame, a +system should run a topological sort over the resource if it's dirty. An alternative version of this hierarchy resource only stores parents. This may speed up iteration as individual relations are smaller during swaps, but finding @@ -243,15 +260,29 @@ can be kept to O(n) in the next depth. - Iteration in hierarchy order (parents first) is entirely linear. This may make transform and other parent-reliant propagation signfigantly faster. - - Can be used with the command-driven interface proposed in this design. + - Finding roots in the sorted hierarchy is both linear and compacted tightly in + memory. + - Removes `Parent` and `Children` ECS components entirely. Could further reduce + archetype fragmentation. The hierarchy is stored densely in a single location + and is not spread between multiple archetypes/tables. + - Can still be used with the command-driven interface proposed in this design + instead of using ECS storage. #### Drawbacks - Merging and splitting hierarchies loaded from scenes can be quite difficult to maintain. - - Iteration requires mutative access to the resource, which can limit - parallelism of systems traversing the hierarchy. The alternative requires a - dedicated O(n) sweep over the hierarchy whenever it's mutated. + - The non-hierarchy components are still stored in an unordered way in ECS data. + Iterating over the hierarchy will still incur random access costs for anything + non-trivial. + - Requires a dedicated topological sort system. Mitigated by only sorting when + the hierarchy is dirty, or simply tracking which entities in the hierarchy are + dirty. + - Editing the hierarchy requires exclusive access to the resource, which can + will serialize all systems editing the hierarchy. This also prevents internal + parallelism (i.e. via `Query::par_for_each`) as `ResMut` cannot be cloned. + This can be partially mitigated by deferring edits via commands, as proposed + in this RFC. ### Alternative: Linked List Hierarchy Instead of using two separate components to manage the hierarchy, this From c47a51a538597374e9ca9238d892441e0bb7c863 Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 30 May 2022 14:58:50 -0700 Subject: [PATCH 20/20] Mention parallel commands options --- rfcs/53-consistent-hierarchy.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rfcs/53-consistent-hierarchy.md b/rfcs/53-consistent-hierarchy.md index dbd56ac4..c939cec1 100644 --- a/rfcs/53-consistent-hierarchy.md +++ b/rfcs/53-consistent-hierarchy.md @@ -213,8 +213,9 @@ transforms for 2D and UI. - Updates are still not immediately visible within a stage and deferred until the next command buffer flush. - Hierarchy updates are now single threaded. Commands can be generated from - multiple systems at the same time, but they now require exclusive access to - apply. + multiple systems, [or from multiple threads in the same + system](https://github.com/bevyengine/bevy/pull/4749) at the same time, but + they now require exclusive access to apply. - Each of the changed commands do a lot more work before and are signifgantly more branch heavy. This could negatively impact the performance of command-heavy workloads (i.e. scene loading)