lyft / scissors

✂ Android image cropping library
https://eng.lyft.com/scissors-an-image-cropping-library-for-android-a56369154a19
Apache License 2.0
1.84k stars 235 forks source link

Image not centered wrt viewport #53

Closed rwliang closed 8 years ago

rwliang commented 8 years ago

Hi! Thanks for open-sourcing this amazing library!

I noticed that if I have a thin viewport ratio, the underlying bitmp is not initially centered: device-2016-06-21-143417

I can get the centered image by doing position.set (availableWidth / 2, availableHeight / 2) here - https://github.com/lyft/scissors/blob/master/scissors/src/main/java/com/lyft/android/scissors/TouchManager.java#L88

Is this the appropriate workaround?

mhousser commented 8 years ago

Wondering the same thing. Taking a picture with the camera, and then after the camera activity result, I'm doing:

cropView.extensions().load(new File(newImageFilename));

But the image is bottom aligned to the base of the 'cropping square'...

eveliotc commented 8 years ago

Thanks for reporting this @liangricha / @mhousser do you have a sample image I can use to reproduce this?

mhousser commented 8 years ago

@eveliotc well actually I'm launching the camera, then upon camera ActivityResult, I'm running this line:

cropView.extensions().load(new File(newImageFilename));

So I don't have a sample image per se - but let me try and get a screenshot.

mhousser commented 8 years ago

First of all, my XML is pretty standard:

    <com.lyft.android.scissors.CropView
        android:id="@+id/crop_view"
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        app:cropviewViewportRatio="1"/>

Immediately after taking a picture with my camera and running this line:

cropView.extensions().load(new File(newImageFilename));

It's bottom-aligning to the crop square:

screenshot_20160624-172556

I would expect it to be vertically centred with the square, just like this (I dragged the image down to make it look like this):

screenshot_20160624-172627

mhousser commented 8 years ago

@eveliotc and another alignment issue, this time when passing in a Uri (from the chooser gallery) of an image that is in landscape mode.

It's right aligning, instead of being horizontally centered.

This time, it's via the line cropView.extensions().load(galleryImageUri);

Some screens. The gallery:

screenshot_20160624-173204

And after I run load, it's aligning the right of the image with the right of the viewport/screen:

screenshot_20160624-173211

mhousser commented 8 years ago

@eveliotc maybe this is just a setup issue.

I'm using Picasso, and so Scissors should be using Picasso to do its loading.

Am I missing somewhere a specific call to tell Picasso and/or Scissors to load things in 'center inside' mode (with respect to the scissors viewport)?

Starting to feel like this is just a setup issue (I missed a call somewhere), instead of an actual bug...

rwliang commented 8 years ago

For the above gif, I used http://earthzine.org/wp-content/uploads/2015/08/image2.jpeg and called setViewportRatio(4.f) in onCreate(...)

ZoroYouth commented 8 years ago

The same problem. If setViewportRatio() before the image is loaded,it's bottom-aligning to the crop square.If setViewportRatio() after the image is loaded,it will be center inside. so should i add a callback to image load?

mhousser commented 8 years ago

@ZoroYouth are you saying there's currently an easy workaround? Just call setViewportRatio(1.0f) after Picasso finishes loading an image, for example?

ZoroYouth commented 8 years ago

now i just wait 1 second to setViewportRatio. it solved my problem,but the way is terrible. mCropView.postDelayed(new Runnable() { @Override public void run() { mCropView.setViewportRatio(mRatio); } },1000);

nshmura commented 8 years ago

I have same problem. Maybe this issue is caused by TouchManager#ensureInsideViewport() method call. https://github.com/lyft/scissors/blob/master/scissors/src/main/java/com/lyft/android/scissors/TouchManager.java#L95

I sent the pull request #57 .

ZoroYouth commented 8 years ago

@nshmura Good job.I have tried your method and it works well,thank you

mhousser commented 8 years ago

@eveliotc can we please accept the fix above and push out a new version? This library is quite broken when all the taken images are not centered by default..

eveliotc commented 8 years ago

@mhousser Fix was included for 1.1.1 @nshmura Thanks for the PR, please sign Lyft's CLA https://oss.lyft.com/cla/ to make it possible to incorporate your changes in the future.

mhousser commented 8 years ago

@eveliotc is 1.1.1 available for Gradle inclusion in yet?

i.e. compile 'com.lyft:scissors:1.1.1'

mhousser commented 8 years ago

Nevermind, I see now that 1.1.1 is published and I have included it in Android Studio.

It works! Finally this library works very well for me and chosen/taken pictures are centred nicely in the cropping area by default.

nshmura commented 8 years ago

@eveliotc Thank you. I done!