agoda-com / Kakao

This repo is no longer supported. Please visit a https://github.com/KakaoCup/Kakao
Apache License 2.0
1.11k stars 102 forks source link

hasDrawable doesn't work when ImageView has android:tint #179

Closed tdekoning closed 3 years ago

tdekoning commented 4 years ago

Steps to reproduce:

  1. Create a layout with an
    <ImageView android:src="R.drawable.my_icon" android:tint="R.color.white" android:layout_width="40dp" android:layout_height="40dp"/>
  2. Create a test that uses hasDrawable(R.drawable.my_icon)

Observed Results:

The method hasDrawable does not see the drawables as equal

Expected Results:

I expected the drawables to be equal, or provide a way to add the expected tint with the hasDrawable call

Possible workaround:

If i would comment out lines 86 until 88 (the canvas drawing) in DrawableMatcher.kt the comparing works, but i'm not sure if that will break anything.

Unlimity commented 4 years ago

Hi there! Thanks for reaching out. Unfortunately, we do not support tinting matching at the moment. But you can submit a PR and extend the functionality of DrawableMatcher by checking if the target view has tint and applying it to the canvas that is being asserted.

tdekoning commented 4 years ago

Thanks for the tip, i'll see what i can do!

Vacxe commented 4 years ago

@tdekoning I think that this improvement can help you. Also, thank for let us know about this lack!

Cheers!

tdekoning commented 4 years ago

@Vacxe Thanks for the change and the quick response!

tdekoning commented 4 years ago

@Vacxe I've tried to test the code from your pull request, but it doesn't seem to work. In my case i'm setting the android:tint and the (vector) drawable using android:src. The call bitmap.sameAs(otherBitmap) returns false and if i inspect the generated bitmaps, they aren't the same. It looks like an anti-alias or scaling issue.

I've tried to fix the issue myself, but i guess i do not understand imageview rendering well enough. Can you look into it?

Vacxe commented 4 years ago

@tdekoning sure, I'll try

Vacxe commented 4 years ago

@tdekoning BTW, does it work without tint? Just wanna find any clue.

tdekoning commented 4 years ago

@Vacxe I've just checked my tests and it seems that the tint checking works in some cases. As far as i can see, if the android:width and android:height of the <vector> node in the drawable is lower than the dimensions of the ImageView with padding, the test will fail.

For example, take this vector:

<vector xmlns:android="http://schemas.android.com/apk/res/android"
    android:width="18dp"
    android:height="18dp"
    android:viewportWidth="18"
    android:viewportHeight="18">
    <path
        android:pathData="M16.3892,18L0.6068,18C0.2715,18 0,17.7313 0,17.3994L0,7.389C0,7.2148 0.0761,7.0494 0.2092,6.9357L8.1068,0.1474C8.3285,-0.044 8.6574,-0.0496 8.886,0.1338L12.4437,3.143L12.8519,3.4697C13.0396,3.6199 13.2419,3.7348 13.2755,3.9522C13.3208,4.2413 13.2034,5.0672 13.1562,5.2047C13.1089,5.3422 12.8782,5.186 12.8519,5.1652L11.6807,4.2756L8.5,1.8124L1.41,7.6629L1.41,16.6014L15.58,16.6014L15.58,10.8566L15.58,10.2432C15.58,10.21 15.6481,9.9985 15.8972,10.0847C16.1463,10.171 16.6845,10.4912 16.8835,10.7078C17.0328,10.8703 16.996,11.0982 16.996,11.3372C16.996,11.3372 16.996,11.4562 16.996,10.4562L16.996,17.3994C16.996,17.7313 16.7245,18 16.3892,18ZM17.457,2.536C17.5827,2.7842 17.8673,3.1864 17.9192,3.4933C17.958,3.7234 17.7894,3.9048 17.6471,4.1173L14.0763,9.4541C13.8428,9.8031 13.3286,9.8438 13.0393,9.5329L10.6259,6.9304C10.4987,6.8158 10.4987,6.6465 10.7161,6.6465C10.9335,6.6465 11.6337,6.6158 11.9525,6.6465C12.1916,6.6697 12.3295,6.8748 12.505,7.0635L13.4381,8.0642L17.1428,2.4716C17.1628,2.442 17.3313,2.2879 17.457,2.536Z"
        android:fillColor="#ED8C00" />
</vector>

If i would place this vector in the following ImageViews, the tests succeed:

<ImageView
                                android:layout_width="16dp"
                                android:layout_height="16dp"
                                android:tint="@color/white"/>
<ImageView
                                android:layout_width="20dp"
                                android:layout_height="20dp"
                                android:padding="4dp"
                                android:tint="@color/white"/>

However, if i would place it in the following ImageView, the test would fail:

<ImageView
                                android:layout_width="20dp"
                                android:layout_height="20dp"
                                android:tint="@color/white"/>

:edit: We are also seeing assertion failures on various, but not all, devices (with various dpi) when using an android:width and android:height that matches the ImageView. Increasing that android:width and android:height in the <vector> node to like 2x the ImageView dimensions seem to resolve that. So my guess is that there is a scaling issue somewhere.

tdekoning commented 4 years ago

@Vacxe is above enough information to trace the issue?

Vacxe commented 4 years ago

@tdekoning not yet. also with vector setup not sure that is possible. Let figure out how we can do it.

Vacxe commented 4 years ago

@tdekoning should be fixed. Could u test it or we can close ticket?

tdekoning commented 4 years ago

@Vacxe thanks! We will test this next week, i will get back to you to report the results

Vacxe commented 3 years ago

@tdekoning can I close the ticket?

tdekoning commented 3 years ago

Sure, close it 👍