Skip to content

Cold specialization#16420

Closed
tychedelia wants to merge 13 commits intobevyengine:mainfrom
tychedelia:cold-specialization
Closed

Cold specialization#16420
tychedelia wants to merge 13 commits intobevyengine:mainfrom
tychedelia:cold-specialization

Conversation

@tychedelia
Copy link
Copy Markdown
Member

@tychedelia tychedelia commented Nov 17, 2024

Objective

  • Describe the objective or issue this PR addresses.
  • If you're fixing a specific issue, say "Fixes #X".

Solution

  • Describe the solution used to achieve the objective above.

Testing

  • Did you test these changes? If so, how?
  • Are there any parts that need more testing?
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?

Showcase

This section is optional. If this PR does not include a visual change or does not add a new feature, you can delete this section.

  • Help others understand the result of this PR by showcasing your awesome work!
  • If this PR adds a new feature or public API, consider adding a brief pseudo-code snippet of it in action
  • If this PR includes a visual change, consider adding a screenshot, GIF, or video
    • If you want, you could even include a before/after comparison!
  • If the Migration Guide adequately covers the changes, you can delete this section

While a showcase should aim to be brief and digestible, you can use a toggleable section to save space on longer showcases:

Click to view showcase 
println!("My super cool code.");

Migration Guide

This section is optional. If there are no breaking changes, you can delete this section.

  • If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes
  • Simply adding new functionality is not a breaking change.
  • Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change.

@tychedelia tychedelia added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Investigation This issue requires detective work to figure out what's going wrong labels Nov 17, 2024
@tychedelia tychedelia mentioned this pull request Dec 13, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Dec 17, 2024
@tychedelia @nicopap
@pcwalton @alice-i-cecile @chescock
Add AssetChanged query filter (#16810)
df14443 
# Objective

Implement a new `AssetChanged` query filter that allows users to query
for entities whose related assets may have changed.

- Closes #5069
- Unblocks #16420. Currently, `cold-specialization`, a key rendering
optimization for unlocking ancillary benefits of the retained render
world, is blocked on being unable detect all scenarios in which an
entity's mesh/material changes using events and observers. An
`AssetChanged` filter will drastically simplify our implementation and
be more robust to future changes.

Originally implemented by @nicopap in #5080.

## Solution

- Adds a new `AssetChanged` query filter that initializes a
`AssetChanges<A>` resource that tracks changed assets and ticks in
`asset_events`.
- ~Reverts #13343 and changes the api of `get_state` to accept `impl
Into<UnsafeWorldCell<'w>>` to allow accessing the `AssetChanges<A>`
resource.~
- Adds a `AsAssetId` trait used for newtype handle wrappers (e.g.
`Mesh3d`) that allows associating a component with the underlying
`Asset` it represents.

## Testing

- Tests are added for `AssetChanged`.
- TBD on performance. We are going to add this `Mesh3d` and
`MeshMaterial3d` (etc) in the renderer. Long term wins in render
performance this unblocks should swamp tracking overhead for any
realistic workload.

## Migration Guide

- The `asset_events` system is no longer public. Users should order
their systems relative to the `AssetEvents` system set.

---------

Co-authored-by: Nicola Papale <nico@nicopap.ch>
Co-authored-by: Patrick Walton <pcwalton@mimiga.net>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
@tychedelia @nicopap
@pcwalton @alice-i-cecile @chescock @ecoskey
Add AssetChanged query filter (bevyengine#16810)
53b44ec 
# Objective

Implement a new `AssetChanged` query filter that allows users to query
for entities whose related assets may have changed.

- Closes bevyengine#5069
- Unblocks bevyengine#16420. Currently, `cold-specialization`, a key rendering
optimization for unlocking ancillary benefits of the retained render
world, is blocked on being unable detect all scenarios in which an
entity's mesh/material changes using events and observers. An
`AssetChanged` filter will drastically simplify our implementation and
be more robust to future changes.

Originally implemented by @nicopap in bevyengine#5080.

## Solution

- Adds a new `AssetChanged` query filter that initializes a
`AssetChanges<A>` resource that tracks changed assets and ticks in
`asset_events`.
- ~Reverts bevyengine#13343 and changes the api of `get_state` to accept `impl
Into<UnsafeWorldCell<'w>>` to allow accessing the `AssetChanges<A>`
resource.~
- Adds a `AsAssetId` trait used for newtype handle wrappers (e.g.
`Mesh3d`) that allows associating a component with the underlying
`Asset` it represents.

## Testing

- Tests are added for `AssetChanged`.
- TBD on performance. We are going to add this `Mesh3d` and
`MeshMaterial3d` (etc) in the renderer. Long term wins in render
performance this unblocks should swamp tracking overhead for any
realistic workload.

## Migration Guide

- The `asset_events` system is no longer public. Users should order
their systems relative to the `AssetEvents` system set.

---------

Co-authored-by: Nicola Papale <nico@nicopap.ch>
Co-authored-by: Patrick Walton <pcwalton@mimiga.net>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
@tychedelia tychedelia closed this Jan 27, 2025
@alice-i-cecile
Copy link
Copy Markdown
Member

alice-i-cecile commented Jan 27, 2025

Redone in #17567.

mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
@tychedelia @nicopap
@pcwalton @alice-i-cecile @chescock @mrchantey
Add AssetChanged query filter (bevyengine#16810)
66e622f 
# Objective

Implement a new `AssetChanged` query filter that allows users to query
for entities whose related assets may have changed.

- Closes bevyengine#5069
- Unblocks bevyengine#16420. Currently, `cold-specialization`, a key rendering
optimization for unlocking ancillary benefits of the retained render
world, is blocked on being unable detect all scenarios in which an
entity's mesh/material changes using events and observers. An
`AssetChanged` filter will drastically simplify our implementation and
be more robust to future changes.

Originally implemented by @nicopap in bevyengine#5080.

## Solution

- Adds a new `AssetChanged` query filter that initializes a
`AssetChanges<A>` resource that tracks changed assets and ticks in
`asset_events`.
- ~Reverts bevyengine#13343 and changes the api of `get_state` to accept `impl
Into<UnsafeWorldCell<'w>>` to allow accessing the `AssetChanges<A>`
resource.~
- Adds a `AsAssetId` trait used for newtype handle wrappers (e.g.
`Mesh3d`) that allows associating a component with the underlying
`Asset` it represents.

## Testing

- Tests are added for `AssetChanged`.
- TBD on performance. We are going to add this `Mesh3d` and
`MeshMaterial3d` (etc) in the renderer. Long term wins in render
performance this unblocks should swamp tracking overhead for any
realistic workload.

## Migration Guide

- The `asset_events` system is no longer public. Users should order
their systems relative to the `AssetEvents` system set.

---------

Co-authored-by: Nicola Papale <nico@nicopap.ch>
Co-authored-by: Patrick Walton <pcwalton@mimiga.net>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Investigation This issue requires detective work to figure out what's going wrong

Projects

No open projects
Rendering (Old)
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tychedelia @alice-i-cecile