Skip to content

Conversation

@tychedelia
Copy link
Member

No description provided.

Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

First paragraph reads a little weird to me.

Users can already have per-material-instance data (that's kinda the whole point of AsBindGroup).

However separate material instances breaks instancing of course...

I think we should explain it like that, and also probably talk about how the material/AsBindGroup API is kinda flawed and doesn't support these kind of workflows atm, and that we're brainstorming how to improve it.

I feel like mesh tag might end up being more of a temporary fix long run, but who knows.

@tychedelia
Copy link
Member Author

Users can already have per-material-instance data (that's kinda the whole point of AsBindGroup).

I'm not sure I understand. Semantically to me, an instance of a material means a concrete entity. Per-instance data in that sense would be per-entity. AsBindGroup allows defining the data for the entire class of instances.

@JMS55
Copy link
Contributor

JMS55 commented Apr 7, 2025

Generally we use instance to mean a bevy entity, and material instance to mean a Handle.

@superdump
Copy link
Contributor

Yeah, we generally consider entities to be instances of drawn things, material instances would be instances of material assets, and we unfortunately sloppily refer to Frankie mesh entities as mesh instances. Per instance data is used exclusively for per drawn thing instance data though. I’ll now actually read the PR and comment more directly.

@@ -0,0 +1,32 @@
Bevy has powerful support for [automatic instancing] for any entities that share the same mesh and material. However, sometimes it can still be useful to reference data that is not the same across all instances of a material. Previously, this required either writing significant amount of custom rendering code or giving up the performance benefits of automatic instancing by creating more materials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just change it to ”all instances using the same material”.

@tychedelia
Copy link
Member Author

I do understand how there might be some confusion in terms of an instance of an asset type being a "material instance", but will note that we use "material instance" in our own code, e.g. here bevyengine/bevy#18734, to refer to collections of entities rather than an instance of an asset. I'll try to make it more clear.

@superdump superdump added this pull request to the merge queue Apr 11, 2025
Merged via the queue into bevyengine:main with commit 4e14451 Apr 11, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants