Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the render instance logic in #9903 so that it's easier for other components to adopt. #10002

Merged
merged 7 commits into from
Oct 8, 2023

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Oct 3, 2023

Objective

Currently, the only way for custom components that participate in rendering to opt into the higher-performance extraction method in #9903 is to implement the RenderInstances data structure and the extraction logic manually. This is inconvenient compared to the ExtractComponent API.

Solution

This commit creates a new RenderInstance trait that mirrors the existing ExtractComponent method but uses the higher-performance approach that #9903 uses. Additionally, RenderInstance is more flexible than ExtractComponent, because it can extract multiple components at once. This makes high-performance rendering components essentially as easy to write as the existing ones based on component extraction.


Changelog

Added

A new RenderInstance trait is available mirroring ExtractComponent, but using a higher-performance method to extract one or more components to the render world. If you have custom components that rendering takes into account, you may consider migration from ExtractComponent to RenderInstance for higher performance.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use C-Performance A change motivated by improving speed, memory usage or compile times labels Oct 3, 2023
@pcwalton
Copy link
Contributor Author

pcwalton commented Oct 3, 2023

This is kind of a prerequisite to GI, as that adds a whole bunch of components that need to be mirrored into the render world and I think it'd be ugly to be constantly reinventing this logic.

…sier for

other components to adopt.

Currently, the only way for custom components that participate in
rendering to opt into the higher-performance extraction method in bevyengine#9903
is to implement the `RenderInstances` data structure and the extraction
logic manually. This is inconvenient compared to the `ExtractComponent`
API.

This commit creates a new `ExtractToRenderInstance` trait that mirrors
the existing `ExtractComponent` method but uses the higher-performance
approach that bevyengine#9903 uses. This makes high-performance rendering
components essentially as easy to write as the existing ones based on
component extraction.
@JMS55 JMS55 added this to the 0.13 milestone Oct 7, 2023
@JMS55
Copy link
Contributor

JMS55 commented Oct 7, 2023

I also want this for gpu driven rendering.

examples/wasm/assets Outdated Show resolved Hide resolved
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the symlink mistakes are fixed then I’ll approve and merge this.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 8, 2023
/// This is essentially the same as
/// [`ExtractComponent`](crate::extract_component::ExtractComponent), but
/// higher-performance because it avoids the ECS overhead.
pub trait ExtractToRenderInstance: Component {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m wondering if there is a case for a more flexible version where you give it a query, filter, and instance, and a method as a way of mapping from the query to the instance but it isn’t implemented for a single Component. I’m a bit concerned that doing this per component one wants to extract will be detrimental to performance because it will result in more, independent lookups.

@pcwalton pcwalton closed this Oct 8, 2023
auto-merge was automatically disabled October 8, 2023 08:59

Pull request was closed

@pcwalton
Copy link
Contributor Author

pcwalton commented Oct 8, 2023

Ok, I'll close this.

@pcwalton pcwalton reopened this Oct 8, 2023
auto-merge was automatically disabled October 8, 2023 10:14

Head branch was pushed to by a user without write access

@superdump superdump added this pull request to the merge queue Oct 8, 2023
Merged via the queue into bevyengine:main with commit e67d63a Oct 8, 2023
21 checks passed
@jancespivo
Copy link
Contributor

Hi, I wanted to try refactor some usage ExtractComponent to RenderInstance (basically to gain insight whether it is replacement or complement to ExtractComponent) and I've found there is already struct with the same naming RenderInstance in https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/src/renderer/mod.rs#L115C5-L115C5 It might be a bit confusing.

@JMS55 JMS55 removed this from the 0.13 milestone Oct 9, 2023
@superdump
Copy link
Contributor

Yeah, that's unfortunate. Modern graphics APIs call one of the handles on a particular GPU an 'instance'. We'll have to think about this a bit and rename something.

regnarock pushed a commit to regnarock/bevy that referenced this pull request Oct 13, 2023
…sier for other components to adopt. (bevyengine#10002)

# Objective

Currently, the only way for custom components that participate in
rendering to opt into the higher-performance extraction method in bevyengine#9903
is to implement the `RenderInstances` data structure and the extraction
logic manually. This is inconvenient compared to the `ExtractComponent`
API.

## Solution

This commit creates a new `RenderInstance` trait that mirrors the
existing `ExtractComponent` method but uses the higher-performance
approach that bevyengine#9903 uses. Additionally, `RenderInstance` is more
flexible than `ExtractComponent`, because it can extract multiple
components at once. This makes high-performance rendering components
essentially as easy to write as the existing ones based on component
extraction.

---

## Changelog

### Added

A new `RenderInstance` trait is available mirroring `ExtractComponent`,
but using a higher-performance method to extract one or more components
to the render world. If you have custom components that rendering takes
into account, you may consider migration from `ExtractComponent` to
`RenderInstance` for higher performance.
ameknite pushed a commit to ameknite/bevy that referenced this pull request Nov 6, 2023
…sier for other components to adopt. (bevyengine#10002)

# Objective

Currently, the only way for custom components that participate in
rendering to opt into the higher-performance extraction method in bevyengine#9903
is to implement the `RenderInstances` data structure and the extraction
logic manually. This is inconvenient compared to the `ExtractComponent`
API.

## Solution

This commit creates a new `RenderInstance` trait that mirrors the
existing `ExtractComponent` method but uses the higher-performance
approach that bevyengine#9903 uses. Additionally, `RenderInstance` is more
flexible than `ExtractComponent`, because it can extract multiple
components at once. This makes high-performance rendering components
essentially as easy to write as the existing ones based on component
extraction.

---

## Changelog

### Added

A new `RenderInstance` trait is available mirroring `ExtractComponent`,
but using a higher-performance method to extract one or more components
to the render world. If you have custom components that rendering takes
into account, you may consider migration from `ExtractComponent` to
`RenderInstance` for higher performance.
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
…sier for other components to adopt. (bevyengine#10002)

# Objective

Currently, the only way for custom components that participate in
rendering to opt into the higher-performance extraction method in bevyengine#9903
is to implement the `RenderInstances` data structure and the extraction
logic manually. This is inconvenient compared to the `ExtractComponent`
API.

## Solution

This commit creates a new `RenderInstance` trait that mirrors the
existing `ExtractComponent` method but uses the higher-performance
approach that bevyengine#9903 uses. Additionally, `RenderInstance` is more
flexible than `ExtractComponent`, because it can extract multiple
components at once. This makes high-performance rendering components
essentially as easy to write as the existing ones based on component
extraction.

---

## Changelog

### Added

A new `RenderInstance` trait is available mirroring `ExtractComponent`,
but using a higher-performance method to extract one or more components
to the render world. If you have custom components that rendering takes
into account, you may consider migration from `ExtractComponent` to
`RenderInstance` for higher performance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A simple quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants