Skip to content
This repository has been archived by the owner on Jan 27, 2023. It is now read-only.

CENTER_CROP is not precisely centered if border_width > 0 #8

Closed
yeusin opened this issue Mar 22, 2013 · 15 comments
Closed

CENTER_CROP is not precisely centered if border_width > 0 #8

yeusin opened this issue Mar 22, 2013 · 15 comments
Assignees

Comments

@yeusin
Copy link

yeusin commented Mar 22, 2013

It should be translate along with border_width to be drawn correctly.

RoundedDrawable.java

private void setMatrix() {
...
...
CENTER_CROP:
...
...
mShaderMatrix.postTranslate((int) (dx + 0.5f) + mBorderWidth, (int) (dy + 0.5f) + mBorderWidth); // Translate by border_width to both x, y.

@vinc3m1
Copy link
Owner

vinc3m1 commented Mar 22, 2013

mind opening this in a pull request?

@alwaysthecritic
Copy link

I'm still seeing this bug, or a slightly different bug. When I use the following XML, the white border causes the image itself to not be centred within the border circle: it's moved down and to the right by the looks of it. In our case this makes the border feature unusable, which is a shame.

<com.makeramen.roundedimageview.RoundedImageView
            xmlns:app="http://schemas.android.com/apk/res-auto"
            android:layout_width="60dp"
            android:layout_height="60dp"
            android:src="@drawable/profile_avatar_default"
            android:scaleType="centerCrop"
            app:riv_oval="false"
            app:riv_corner_radius="30dp"
            app:riv_border_width="3dp"
            app:riv_border_color="@color/white"/>

@alwaysthecritic
Copy link

A little experimentation shows that this would work, where we simply add /2 after each mBorderWidth, to ensure the image is centred within the border:

mShaderMatrix.postTranslate((int) (dx + 0.5f) + mBorderWidth/2, (int) (dy + 0.5f) + mBorderWidth/2);

An alternative approach would be to scale the image down a bit (by border width presumably), so the border doesn't overlap its edges at all.

@vinc3m1 vinc3m1 reopened this Jun 15, 2015
@vinc3m1
Copy link
Owner

vinc3m1 commented Jun 15, 2015

I actually think borderWidth should not be part of the postTransate at all in this case since mBorderRect is already inset by the necessary amount. I think I will just remove that.

There will still needs to be overlap, which is needed for a clean line between the border and the image. Otherwise you see small bits of white between the border and the image due to antialiasing.

@vinc3m1
Copy link
Owner

vinc3m1 commented Jun 15, 2015

Here's a test with super thick and transparent borders.

Current Behavior
screen shot 2015-06-14 at 7 50 31 pm

New behavior:
screen shot 2015-06-14 at 7 52 23 pm

@alwaysthecritic
Copy link

Hmmm - for me this fix doesn't work correctly, but my suggestion with mBorderWidth/2 does. Maybe there's something screwy with my images, but I don't believe so. I'm using avatar images, that by default are a centred, symmetrical graphic (e.g. the Twitter egg in this example) but I can see that the image does not end up centred within the border with the current fix.

device-2015-06-15-092812

@vinc3m1 vinc3m1 reopened this Jun 15, 2015
@vinc3m1
Copy link
Owner

vinc3m1 commented Jun 15, 2015

Hm, that looks to be off more than borderwidth/2... Any chance you can provide some sample code to reproduce?

@alwaysthecritic
Copy link

If I add in + mBorderWidth/2 to the translation it appears to centre perfectly.

device-2015-06-15-094751

The XML in question is as follows:

        <com.makeramen.roundedimageview.RoundedImageView
            xmlns:app="http://schemas.android.com/apk/res-auto"
            android:id="@+id/avatar_image"
            android:layout_width="60dp"
            android:layout_height="60dp"
            app:riv_corner_radius="30dp"
            android:layout_gravity="center"
            android:layout_marginTop="15dp"
            style="@style/BigAvatarWithBorder"/>

A whole canned example would take a bit of effort as the images are fetched from the network etc.

@vinc3m1
Copy link
Owner

vinc3m1 commented Jun 17, 2015

Do you have a copy of the image I could play with?

@alwaysthecritic
Copy link

Here's an example of one of the default Twitter avatars, where the egg is perfectly centred within the image: https://abs.twimg.com/sticky/default_profile_images/default_profile_2_normal.png

@vinc3m1 vinc3m1 self-assigned this Aug 23, 2015
@aftabsikander
Copy link

Is there a workaround for this issue?

@alwaysthecritic
Copy link

For now I have my own versions of RoundedImageView and RoundedDrawable which I use in preference to the ones in the library, so that I can apply the fix (at least the fix that works for me, as detailed above).

The fixed code is in my RoundedDrawable, but you need RoundedImageView too, so that it uses the modified RoundedDrawable throughout.

@aftabsikander
Copy link

I have implemented your solution but the image is not centred when scale type is centre.
This happens only for some images.
Sample 1 image is not centred.
sample_1

Sample 2 image is centred
sample_2

Link to modified file
https://gist.github.com/aftabsikander/2a9a994e9d721cb4a36d

Edited:
The XML is as follows:

  <com.test.RoundedImageView
        xmlns:app="http://schemas.android.com/apk/res-auto"
        android:id="@+id/avatar_image"
        android:layout_width="90dp"
        android:layout_height="90dp"
        android:layout_gravity="center"
        android:scaleType="center"
        android:src="@drawable/test1"
        app:riv_border_color="@color/colorPrimary"
        app:riv_border_width="2dp"
        app:riv_corner_radius="180dp"
        app:riv_oval="true" />

@ivanviragine
Copy link

news on this?

@vinc3m1
Copy link
Owner

vinc3m1 commented Oct 19, 2015

After taking a second look, looks like @alwaysthecritic had the right fix the whole time. Not sure why that didn't click the first time.

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

No branches or pull requests

5 participants