Bitmap extensions by feiyanke · Pull Request #418 · android/android-ktx

feiyanke · 2018-03-09T07:47:46Z

some useful functions of Bitmap:

Bitmap.toBytes
Bitmap.clip
Bitmap.skew
Bitmap.rotate

googlebot · 2018-03-09T07:47:49Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

feiyanke · 2018-03-09T08:12:39Z

I signed it!

googlebot · 2018-03-09T08:12:41Z

CLAs look good, thanks!

hanggrian · 2018-03-09T08:37:35Z

I'm liking toBytes, but it really should be toByteArray. Also, I'm not sure if JPG shoud be the default format over PNG. And maybe use IntRange annotation in param quality.

romainguy

See my comments, but also missing from this PR:

running the updateApi task
tests

romainguy · 2018-03-09T18:25:30Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param quality Hint to the compressor, 0-100. 0 meaning compress for small size, 100 meaning compress for max quality.
+ * @return ByteArray
+ */
+fun Bitmap.toBytes(format:CompressFormat = JPEG, quality: Int=100):ByteArray


Compressing a bitmap can be done in many ways and compressing into a byte array is just a highly specific use case. I'd rather remove this method.

romainguy · 2018-03-09T18:26:42Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param height The height.
+ * @return the clipped bitmap
+ */
+fun Bitmap.clip(x:Int, y:Int, width: Int, height: Int):Bitmap


I'm not sure this method is necessary. It doesn't save much over the Bitmap.createBitmap() call for an operation that might not be very common. At least it should be inline, better documented and the return type doesn't need to be specified.

clip is of more clear meaning than createBitmap for bitmap operation. I added inline for all methods and updated docs.

romainguy · 2018-03-09T18:26:50Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param py  The y coordinate of the pivot point.
+ * @return the skewed bitmap
+ */
+fun Bitmap.skew(kx:Float, ky:Float, px:Float = 0f, py:Float = 0f):Bitmap


This is not a common usage, please remove.

I perfer add this method in order to let operations of bitmap more complete

romainguy · 2018-03-09T18:27:26Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param py      The y coordinate of the pivot point.
+ * @return the rotated bitmap
+ */
+fun Bitmap.rotate(degrees:Int, px: Float, py: Float):Bitmap


I'm not entirely convinced by how common this use case is. (Also same comments that I've mentioned on clip() apply here).

feiyanke · 2018-03-12T01:58:19Z

@hendraanggrian It more make sense to use toByteArray, I have fixed it and added IntRange for quality. PNG is a common use format, but I perfer JPG as it could make smaller size for most bitmaps.

2. add inline for functions

romainguy · 2018-03-12T03:26:54Z

api/current.txt


  public final class ContextKt {
    ctor public ContextKt();
+    method public static java.util.List<java.lang.String> getPermissions(android.content.Context);


Why this change?

sorry, it's a mistake, I have already removed it

romainguy · 2018-03-12T03:27:02Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param py  The y coordinate of the pivot point.
+ * @return the skewed bitmap
+ */
+inline fun Bitmap.skew(kx:Float, ky:Float, px:Float = 0f, py:Float = 0f):Bitmap


Remove, this is not useful

romainguy · 2018-03-12T03:27:10Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param py      The y coordinate of the pivot point.
+ * @return the rotated bitmap
+ */
+inline fun Bitmap.rotate(degrees:Float, px: Float, py: Float)


Not sure this is usefuul

romainguy · 2018-03-12T03:27:49Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param height The height.
+ * @return the clipped bitmap
+ */
+inline fun Bitmap.clip(x:Int, y:Int, width: Int, height: Int)


You should run klint, your formatting is inconsistent (x:Int vs with: Int for instance)
I would rather call this method crop()

romainguy · 2018-03-12T03:28:12Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @param quality Hint to the compressor, 0-100. 0 meaning compress for small size, 100 meaning compress for max quality.
+ * @return ByteArray
+ */
+inline fun Bitmap.toByteArray(format:CompressFormat = CompressFormat.JPEG, @IntRange(from = 0, to = 100) quality: Int=100)


I still don't find compressing to a byte array useful. Compressing directly into a stream seems the more common case

romainguy · 2018-03-13T16:47:19Z

src/main/java/androidx/graphics/Bitmap.kt

+ * @return ByteArrayInputStream
 */
-inline fun Bitmap.toByteArray(
+inline fun Bitmap.toStream(


That is not what I meant. What I meant is that there shouldn't be an extension on Bitmap to compress to a byte array or a stream, etc. Bitmap.compress() already accepts an OuputStream which is generic enough to cover all use cases. It shouldn't be specialized here.

romainguy · 2018-03-13T16:47:53Z

src/main/java/androidx/graphics/Bitmap.kt

 * @return the clipped bitmap
 */
-inline fun Bitmap.clip(x: Int, y: Int, width: Int, height: Int) =
+inline fun Bitmap.clip(x: Int, y: Int, width: Int, height: Int): Bitmap =


The return type is not needed. Please rename this method to crop().

romainguy · 2018-05-16T16:48:03Z

src/main/java/androidx/graphics/Bitmap.kt

+
+/**
+ * Creates a new bitmap, rotated from this bitmap by [degrees] - the specified number of degrees,
+ * with a pivot point at ([px], [py]). The pivot point is the coordinate that should remain


There's no pivot point in the API

sorry for that, i added it.

Thanks for the change. The pivot point should probably be set to width/2.0f and height/2.0f by default.

I added the default pivot point.

romainguy · 2018-06-08T19:42:40Z

I am not against those two extensions, they are actually kind of useful, but they don't really leverage any feature of Kotlin, beyond being simple extensions. rotate with a default pivot point would.

add some extension functions for Bitmap

5717646

Merge branch 'master' into master

acfc8b4

romainguy reviewed Mar 9, 2018

View reviewed changes

feiyanke added 4 commits March 12, 2018 10:03

1. modify toBytes to toByteArray

1e6bbfa

2. add inline for functions

Merge branch 'master' of github.com:feiyanke/android-ktx

29b2a18

update docs

d1f464d

run updateApi

02fb227

romainguy reviewed Mar 12, 2018

View reviewed changes

feiyanke added 3 commits March 12, 2018 11:43

all check pass

8086a2d

add test and do small change of api

4f57ec8

remove skew() and modify toByteArray() to toStream()

e421387

romainguy reviewed Mar 13, 2018

View reviewed changes

remove toStream(), rename clip() to crop() and remove the return type

4a22496

romainguy reviewed May 16, 2018

View reviewed changes

add pivot point parameters for rotate method

3517ae4

add default pivot point for rotate method

b855f4c

Conversation

feiyanke commented Mar 9, 2018

Uh oh!

googlebot commented Mar 9, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

feiyanke commented Mar 9, 2018

Uh oh!

googlebot commented Mar 9, 2018

Uh oh!

hanggrian commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romainguy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feiyanke commented Mar 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romainguy commented Jun 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hanggrian commented Mar 9, 2018 •

edited

Loading