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

Unload meshes from the CPU automatically #8132

Closed
wants to merge 2 commits into from

Conversation

JMS55
Copy link
Contributor

@JMS55 JMS55 commented Mar 20, 2023

Objective

  • Meshes are currently automatically uploaded to the GPU. However, once on the GPU, copies of all vertex data and indices are still kept around in RAM, which is a waste of memory.
  • Furthermore, doing the Mesh->GpuMesh asset lookup every frame for rendering is a minor waste of CPU time.
  • Supersedes Mesh: added static and dynamic mesh for less runtime memory usage #1782.

Solution

  • Mesh no longer implements RenderAsset.
  • A new system called transfer_meshes_to_gpu will take any entity with a Handle<Mesh> component, upload the mesh to the GPU and attach a GpuMesh, and then remove the handle.
    • If this handle is the last instance, refcounting will then delete the underlying mesh data.
  • Render systems now query for GpuMesh directly.

TODO

  • Fix examples
  • Check that the GLTF loader isn't keeping a different copy of the data in memory
  • Extend this PR to textures
    • Maybe keep RenderAsset around, but revamp it to convert handles instead
  • Write changelog and migration guide

Changelog

This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

  • What changed as a result of this PR?
  • If applicable, organize changes under "Added", "Changed", or "Fixed" sub-headings
  • Stick to one or two sentences. If more detail is needed for a particular change, consider adding it to the "Solution" section
    • If you can't summarize the work, your change may be unreasonably large / unrelated. Consider splitting your PR to make it easier to review and merge!

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.

@JMS55 JMS55 added this to the 0.11 milestone Mar 20, 2023
@JMS55 JMS55 added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times labels Mar 20, 2023
@github-actions
Copy link
Contributor

Example no_renderer failed to run, please try running it locally and check the result.

@vallrand
Copy link
Contributor

Given that certain assets have to be kept in RAM anyway (shaders for recompilation when specializing, dynamic meshes that are mutated from the CPU side), asset itself might need to have an ability to be "unloaded" when consumed by internal system.
I would however try to look for ways to keep end user API consistent.

A new system called transfer_meshes_to_gpu will take any entity with a Handle component, upload the mesh to the GPU and attach a GpuMesh, and then remove the handle.
If this handle is the last instance, refcounting will then delete the underlying mesh data.

If Handle<Mesh> is how mesh is attached to entity, end user would expect to be able to call commands.entity(entity).remove::<Handle<Mesh>>(); later.

Also assets are usually managed separately (load/unload on new scene), so handle will never be removed (since it's kept inside some "resource"), And when removed, you might as well unload GPU data as well.
See bevy_asset_loader

@vallrand vallrand mentioned this pull request Mar 22, 2023
@JMS55
Copy link
Contributor Author

JMS55 commented Apr 4, 2023

I'm going to close this for now and come back to this after the asset rework happens.

@JMS55 JMS55 closed this Apr 4, 2023
@JMS55 JMS55 removed this from the 0.11 milestone Apr 4, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants