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

Add Extract/Clone apis for manipulating deconstructing images from from #344

Merged
merged 4 commits into from
Sep 21, 2017

Conversation

tocsoft
Copy link
Member

@tocsoft tocsoft commented Sep 19, 2017

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This adds the new APIs to Image<TPixel> for cloning and extracting frames from an image.

New APIs

  • Image<TPixel>.Clone(int frameIndex)
  • Image<TPixel>.CloneAs<TPixel2>(int frameIndex)
  • Image<TPixel>.Extract(int frameIndex)

TODO

  • Add Tests
  • Resolve API location
    should these APIs be members of Image<T> of ImageFrameCollection?
    i.e. do you call image.Clone(index) or image.Frames.CloneFrame(index)
  • Resolve API names
    should the apis have the Frame postfix? I feel this depended on if the API lives on the frames collection or the image

resolves #340

@JimBobSquarePants
Copy link
Member

The Frame suffix seems nice and explicit to me but I don't think that the APIs should live in the ImageFrameCollection. (Don't ask me why it just doesn't feel right)

@antonfirsov
Copy link
Member

@JimBobSquarePants I think you have this feeling because all of them are returning an Image<T> instance. This makes them more bound to Image<T> than ImageFrameCollection<T> where everything operates on frame instances only.

@JimBobSquarePants
Copy link
Member

@antonfirsov Yeah, that describes it well.

@tocsoft tocsoft changed the title [WIP] Add Extract/Clone apis for manipulating deconstructing images from from Add Extract/Clone apis for manipulating deconstructing images from from Sep 21, 2017
@antonfirsov
Copy link
Member

antonfirsov commented Sep 21, 2017

@tocsoft All LGTM. Like that we manage the lifecycle of frames through our API now, preserving valid state for Image<T>.

We probably need a gif manipulator example, because the behavior of some functions (eg. AddFrame(otherFrame)) is not trivial now.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

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

Very happy with this. It's come together very well!

@JimBobSquarePants JimBobSquarePants merged commit 60b7ab1 into master Sep 21, 2017
@tocsoft tocsoft deleted the tocsoft/extract-clone-frames branch September 21, 2017 17:59
@antonfirsov antonfirsov mentioned this pull request Sep 22, 2017
4 tasks
antonfirsov pushed a commit to antonfirsov/ImageSharp that referenced this pull request Nov 11, 2019
…-frames

Add Extract/Clone apis for manipulating deconstructing images from from
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

Successfully merging this pull request may close these issues.

Clone/Extract frames from image
3 participants