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

implement Default for DynamicImage #1552

Closed
experiment9123 opened this issue Sep 6, 2021 · 2 comments
Closed

implement Default for DynamicImage #1552

experiment9123 opened this issue Sep 6, 2021 · 2 comments

Comments

@experiment9123
Copy link

why - to be able to include DynamicImage in other composed structs that you want to auto derive default() for. Sometimes you do want to be able to allocate space that is filled in later

I would suggest the default DynamicImage could be an an empty ARGB8888 image (i.e. zero size) ; alternatively a specific 'Empty image' could be added to the enum

@ForgottenMaster
Copy link
Contributor

Looking into this, it seems that ImageBuffer already implements Default which creates a zero size buffer, so extending this out to include DynamicImage should be easy enough - it's just a question of which variant we want to use for the default. I would say too that ImageRgba8 is probably the most common so could be a sensible default value.

I think the choice doesn't matter too much however as it will be a zero size image by default and so, if someone wanted to actually initialize it to something else, they'd have to make a new one anyway and create an allocation at that point (or otherwise make an ImageBuffer from some preallocated container).

alternatively a specific 'Empty image' could be added to the enum

Adding a custom Empty variant would be a breaking change I believe, since someone could have pattern matched on a DynamicImage in an exhaustive way, and so adding an additional variant would break such cases, which we don't need to do just to add a Default implementation.

Implementing Default for DynamicImage would let composing structs auto derive Default, however the default DynamicImage may be kind of useless to them as nothing would be preallocated (due to the 0 size) and would have to be allocated later.

I'm thinking that on top of implementing Default for DynamicImage itself, we could have a wrapper type that also implements Default, but whose implementation can preallocate a set pixel format, and width/height (specified at the type level). We should be able to do this now we have const generics, and could enable the kind of preallocation you're mentioning.

I'm envisioning something like (when defining a composing structure):

#[derive(Default)]
struct Foo {
        image: PreallocatedDynamicImage<Rgba<u8>, 100, 100>
}

That is, the ability to still auto derive Default in the composing structure, but actually allocate a fixed size buffer of a particular pixel format in that implementation (in the example, a 100x100 RGBA image)

The PreallocatedDynamicImage is just a thin wrapper around DynamicImage that implements the Default trait and depending on the pixel format, and the width/height generic parameters will construct the correct variant of DynamicImage

We can then just provide access to that underlying DynamicImage when it's needed.

Do you think it'd be worth doing such an API in addition?

@5225225 5225225 mentioned this issue Nov 8, 2021
22 tasks
@ForgottenMaster
Copy link
Contributor

Then again, such a wrapper necessitates encoding the pixel format and size at the type level in order to allow for the Default implementation to be used to construct the underlying DynamicBuffer

And at that point, we're putting a type level restriction around something that's designed to be polymorphic, so actually that sounds like it would be a wrapper around an ImageBuffer if anything which already is locked down to a given pixel format

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants