Skip to content

image: Allow any kind of data that implements AsRef<[u8]> for the i…#1551

Merged
hecrj merged 2 commits intoiced-rs:masterfrom
sdroege:image-data-refcounting
Feb 17, 2023
Merged

image: Allow any kind of data that implements AsRef<[u8]> for the i…#1551
hecrj merged 2 commits intoiced-rs:masterfrom
sdroege:image-data-refcounting

Conversation

@sdroege
Copy link
Contributor

@sdroege sdroege commented Nov 20, 2022

…mage data

It's not required anywhere for it to be a plain slice or a Vec and this makes it possible to use data allocated in a different way without copying.

Also store the data in an Arc to allow cheap cloning.

@13r0ck
Copy link
Contributor

13r0ck commented Nov 20, 2022

Image handle already uses an arc

pub struct Handle {

https://github.com/iced-rs/iced/blob/master/examples/pokedex/src/main.rs#L113

@sdroege
Copy link
Contributor Author

sdroege commented Nov 20, 2022

Ah I see, thanks! Then this can also become a plain Box<dyn _> instead.

Is the Clone impl on Data then actually needed? If it's needed this needs a bit more code (as Clone is not object-safe) but also not much of a problem.

@sdroege sdroege force-pushed the image-data-refcounting branch from 2ebb688 to 0ae1342 Compare November 21, 2022 08:56
@sdroege sdroege force-pushed the image-data-refcounting branch from 0ae1342 to 2009e68 Compare November 21, 2022 08:58
sdroege and others added 2 commits February 17, 2023 14:23
…mage data

It's not required anywhere for it to be a plain slice or a `Vec` and
this makes it possible to use data allocated in a different way without
copying.
@hecrj hecrj force-pushed the image-data-refcounting branch from 2009e68 to d7c8308 Compare February 17, 2023 13:37
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.

Thanks for this!

I have moved the Arc from Handle to Bytes since the other fields in Data should be fairly cheap to Clone and we can get rid of the awkward trait this way.

@hecrj hecrj added feature New feature or request performance image labels Feb 17, 2023
@hecrj hecrj added this to the 0.8.0 milestone Feb 17, 2023
@hecrj hecrj enabled auto-merge February 17, 2023 13:42
@hecrj hecrj merged commit f75e020 into iced-rs:master Feb 17, 2023
ImageBytes(self.0.clone_boxed())
impl Bytes {
/// Creates new [`Bytes`] around `data`.
pub fn new(data: impl AsRef<[u8]> + Clone + Send + Sync + 'static) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You don't really need the Clone here anymore as the Arc already gives you that :)
Do you want me to submit a PR for removing the trait bound or will you take care of the change?

Copy link
Member

Choose a reason for hiding this comment

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

Good catch. Just opened #1717.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@sdroege sdroege deleted the image-data-refcounting branch February 19, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request image performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments