Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Image not centered wrt viewport #53

Closed
rwliang opened this issue Jun 21, 2016 · 17 comments
Closed

Image not centered wrt viewport #53

rwliang opened this issue Jun 21, 2016 · 17 comments
Assignees
Labels
Milestone

Comments

@rwliang
Copy link

rwliang commented Jun 21, 2016

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
Copy link

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 eveliotc added the bug label Jun 25, 2016
@eveliotc eveliotc added this to the Next milestone Jun 25, 2016
@eveliotc eveliotc self-assigned this Jun 25, 2016
@eveliotc
Copy link
Contributor

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

@mhousser
Copy link

@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
Copy link

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
Copy link

@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
Copy link

@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
Copy link
Author

rwliang commented Jun 25, 2016

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

@ZoroYouth
Copy link

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
Copy link

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

@ZoroYouth
Copy link

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
Copy link

nshmura commented Jul 4, 2016

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
Copy link

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

@mhousser
Copy link

@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
Copy link
Contributor

@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
Copy link

mhousser commented Jul 21, 2016

@eveliotc is 1.1.1 available for Gradle inclusion in yet?

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

@mhousser
Copy link

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
Copy link

nshmura commented Jul 22, 2016

@eveliotc Thank you. I done!

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

Successfully merging a pull request may close this issue.

5 participants