google / horologist

Horologist is a group of libraries that aim to supplement Wear OS developers with features that are commonly required by developers but not yet available.
https://google.github.io/horologist/
Apache License 2.0
543 stars 87 forks source link

Add support and a test for image transparency #2255

Closed yschimke closed 3 weeks ago

yschimke commented 3 weeks ago

WHAT

Support format for tile image conversions to support transparency.

WHY

Fixing https://github.com/google/horologist/issues/251

HOW

Checklist :clipboard:

Tolriq commented 3 weeks ago

After a short night of sleep, I think it's simpler and better to drop the bitmap conversion and leave it to the user with doc explaining it. Most conversion up or down should be done with dithering and specific options to ensure better result depending on the kind of image anyway.

That function should get the bitmap config and simply have a mapping for the ImageFormat from the bitmap.

yschimke commented 3 weeks ago

OK, I'll land this. Agreed, I was struggling for how the library can know what the right thing to do is.

yschimke commented 3 weeks ago

@Tolriq care to give me a LGTM?

Tolriq commented 3 weeks ago

@yschimke what I mean is that toImageResource should be parameter less and not do any conversion at all.

It should just convert the bitmap.config to an Imageformat to pass down and have proper javadoc about it.

yschimke commented 3 weeks ago

It needs to fail if your Bitmap has the wrong format then. I don't see how that helps.

I can document that you should ensure it's right before calling, but that seems like a sharp edge.

yschimke commented 3 weeks ago

OK, I see what you mean. Yep, that is a better idea.

Tolriq commented 3 weeks ago

It needs to fail if your Bitmap has the wrong format then. I don't see how that helps.

This makes things explicit, automatic bitmap modifications makes it hard for the dev to figure out what is happening when there is an issue. I took me a lot of time to search on that function and not on the bitmap generation itself why cropping was not working.

Worse is that it could work with artifact not seen on the test images and being ugly when in prod and actual user images.