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

Added resizing with keeping aspect ratio when dimension is 0. #663

Merged
merged 1 commit into from
Oct 24, 2014

Conversation

mcharmas
Copy link
Contributor

I have added possibility to resize with keeping aspect ratio as discussed in #613 to be able to write alignTopCrop transformation efficiently. There are some issues like #226 that are solved with this pull request.

To resize keeping aspect ratio one should pass 0 as unknown resize dimension to resize method of builder.

@JakeWharton
Copy link
Collaborator

Thanks! We'll try to look soon. Both of us presenting at a conference this
weekend.

On Fri, Sep 19, 2014 at 9:50 AM, Michał Charmas notifications@github.com
wrote:

I have added possibility to resize with keeping aspect ratio as discussed
in #613 #613 to be able to write
alignTopCrop transformation efficiently. There are some issues like #226
#226 that are solved with this

pull request.

You can merge this Pull Request by running

git pull https://github.com/mcharmas/picasso master

Or view, comment on, or merge it at:

#663
Commit Summary

  • Added resizing with keeping aspect ratio when dimension is 0.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#663.

@JakeWharton
Copy link
Collaborator

This looks great. I would like to see a test that exercises the actual resize logic though. I think there's four cases: resizing up and resizing down for both width-fixed and height-fixed aspect ratios. Do you think you can add those?

@JakeWharton
Copy link
Collaborator

Needs a rebase.

@dnkoutso dnkoutso added this to the Picasso 2.4 milestone Sep 23, 2014
@mcharmas
Copy link
Contributor Author

mcharmas commented Oct 1, 2014

Glad it works for you. My implementation of crop that aligns to top transformation looks like this:

    private class CropAlignTopTransformation implements Transformation {
        private final int targetWidth;
        private final int targetHeight;

        public CropAlignTopTransformation(int targetWidth, int targetHeight) {
            this.targetWidth = targetWidth;
            this.targetHeight = targetHeight;
        }

        @Override
        public Bitmap transform(Bitmap source) {
            Bitmap transformed;
            if (source.getHeight() == targetHeight) {
                return source;
            } else if (source.getHeight() >= targetHeight) {
                transformed = Bitmap.createBitmap(source, 0, 0, targetWidth, targetHeight);
            } else {
                float scaleFactor = (float) targetHeight / source.getHeight();
                int drawWidth = (int) (source.getWidth() / scaleFactor);
                int xOffset = (source.getWidth() - drawWidth) / 2;
                Matrix matrix = new Matrix();
                matrix.preScale(scaleFactor, scaleFactor);
                transformed = Bitmap.createBitmap(source, xOffset, 0, drawWidth, source.getHeight(), matrix, false);
            }

            if (transformed != source) {
                source.recycle();
                source = transformed;
            }
            return source;
        }

        @Override
        public String key() {
            return "crop_align_top_" + targetWidth + "x" + targetHeight;
        }
    }

Api call that uses this transformation looks like this:

requestCreator.resize(width, 0).transform(new CropAlignTopTransformation(width, height))

So to sum up:

  • image that is transformed has already size that is equal to desired width
  • there are 3 cases:
    • image has desired height - returning original
    • image has bigger height - cropping from top
    • image has smaller heigh - image is scaled to match heigh and cropped on the sides

@JakeWharton
Copy link
Collaborator

Still missing tests that show the actual behavior (mentioned above).

@dnkoutso
Copy link
Collaborator

@mcharmas can you please add more tests? We are wrapping up 2.4 and I can't accept this change without more tests.

@mcharmas
Copy link
Contributor Author

Yep, I'll add these tests in few hours.

@mcharmas
Copy link
Contributor Author

Tests added.

@dnkoutso
Copy link
Collaborator

Thank you! looking now.

dnkoutso added a commit that referenced this pull request Oct 24, 2014
Added resizing with keeping aspect ratio when dimension is 0.
@dnkoutso dnkoutso merged commit 4d4a79c into square:master Oct 24, 2014
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.

3 participants