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

Utilize bytes::Bytes for images #2356

Merged
merged 7 commits into from
May 1, 2024
Merged

Utilize bytes::Bytes for images #2356

merged 7 commits into from
May 1, 2024

Conversation

Bajix
Copy link
Contributor

@Bajix Bajix commented Mar 27, 2024

This PR is to replace image::Bytes with bytes::Bytes as it's more optimized/feature rich and is basically a drop in replacement.

core/src/image.rs Outdated Show resolved Hide resolved
@Bajix
Copy link
Contributor Author

Bajix commented Apr 6, 2024

Advantages of using bytes::Bytes:

  • Vec and other types can be converted from without there being extra indirection and without reallocating
  • Bytes can be loaded, cloned and converted into an image without reallocating. This makes it easier to author data loading behavior that converts into images or is otherwise deserialized with serde

Bajix and others added 3 commits May 1, 2024 00:28
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Looks great!

I made some further changes to use Bytes for the ImageBuffer data containers as well. This way, we avoid reallocating completely when Handle::from_pixels is used!

I have also removed the eager Data hashing and instead decided to use the Bytes pointer as the identifier of a Handle.

@hecrj
Copy link
Member

hecrj commented Apr 30, 2024

decided to use the Bytes pointer as the identifier of a Handle.

Terrible idea. It's common for allocators to reuse memory causing false cache hits in some edge cases. Changed it to use an auto-incremented unique Id with AtomicU64 in b52c7bb.

core/src/image.rs Show resolved Hide resolved
@Bajix
Copy link
Contributor Author

Bajix commented May 1, 2024

New changes look fantastic!

I think this work will tie in really nicely into a generalized asset loading solution whereby a generation arena stores load state, updates create new requests by initializing a loading state and getting an index, views read the current load state using the index, and the arena updates load state with bytes when processing responses. This will be super efficient both for loading images and for deserialization with borrows

@hecrj hecrj merged commit a11784f into iced-rs:master May 1, 2024
12 checks passed
@Bajix Bajix deleted the feature/bytes branch May 2, 2024 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants