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

[Merged by Bors] - Faster gltf loader (re-merge of #3165) #3189

Closed
wants to merge 1 commit into from

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Nov 25, 2021

See #3165 and #3175

Objective

  • @superdump was having trouble with this loop in the GLTF loader.

Solution

  • Make it probably linear.
  • Measured times:
  • Old: 40s, new: 200ms

I think there's still room for improvement. For example, I think making the nodes be in Arcs could be a significant gain, since currently there's duplication all the way down the tree.

Co-authored-by: Carter Anderson mcanders1@gmail.com

# Objective

- @superdump was having trouble with this loop in the GLTF loader.

## Solution

- Make it probably linear.
- Measured times: 
- Old: 40s, new: 200ms

I'm sure there's still room for improvement. For example, I think making the nodes be in `Arc`s could be a significant gain, since currently there's duplication all the way down the tree.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Nov 25, 2021
@mockersf
Copy link
Member

bors r+

@mockersf
Copy link
Member

merging directly as it was already reviewed and merged

bors bot pushed a commit that referenced this pull request Nov 25, 2021
See #3165 and #3175

# Objective

- @superdump was having trouble with this loop in the GLTF loader.

## Solution

- Make it probably linear.
- Measured times: 
- Old: 40s, new: 200ms

I think there's still room for improvement. For example, I think making the nodes be in `Arc`s could be a significant gain, since currently there's duplication all the way down the tree.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@mockersf mockersf added A-Assets Load files from disk to use for things like images, models, and sounds C-Performance A change motivated by improving speed, memory usage or compile times and removed S-Needs-Triage This issue needs to be labelled labels Nov 25, 2021
@bors bors bot changed the title Faster gltf loader (re-merge of #3165) [Merged by Bors] - Faster gltf loader (re-merge of #3165) Nov 25, 2021
@bors bors bot closed this Nov 25, 2021
@DJMcNab DJMcNab deleted the faster-gltf branch November 25, 2021 18:56
HackerFoo pushed a commit to HackerFoo/bevy that referenced this pull request Nov 26, 2021
See bevyengine#3165 and bevyengine#3175

# Objective

- @superdump was having trouble with this loop in the GLTF loader.

## Solution

- Make it probably linear.
- Measured times: 
- Old: 40s, new: 200ms

I think there's still room for improvement. For example, I think making the nodes be in `Arc`s could be a significant gain, since currently there's duplication all the way down the tree.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@cart cart added this to the Bevy 0.6 milestone Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds 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.

3 participants