Icon.kt applies a transparent tint when Color.Unspecified is passed as the `tint` argument. [171624632]

Fixed

Bug

Status Update

No update yet.

Description

al...@gmail.com

created issue #1

Oct 25, 2020 09:03PM

When using Icon.kt, I personally would expect that passing Color.Unspecified as the tint would result in no tint being applied to the icon at all (in other words, a null color filter would be passed to the Painter and the colors that are rendered would just be whatever the VectorAsset or ImageAsset internally draws).

This is currently not the case, as passing Color.Unspecified results in a transparent color filter being applied (meaning the icon is not rendered at all).

This behavior is particularly important to support multi-colored icons, where you need to avoid a tint being applied all together.

// EXPECTED BEHAVIOR: no tint is applied to the vector asset
// ACTUAL BEHAVIOR: a transparent tint is applied to the vector asset
Icon(
    asset = someVectorAsset,
    tint = Color.Unspecified,
)

The code in question at the time this bug was filed is here: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-master-dev:compose/material/material/src/commonMain/kotlin/androidx/compose/material/Icon.kt;l=96;drc=989981f1b2a64419ed599cb82e2c589b6b41e46d

Comments

ma...@google.com <ma...@google.com> #2Oct 26, 2020 11:38AM

Reassigned to nj...@google.com.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

al...@gmail.com <al...@gmail.com> #3Oct 26, 2020 04:44PM

I can create a CL if Nader agrees it is a bug. :)

nj...@google.com <nj...@google.com> Oct 28, 2020 03:43AM

Accepted by nj...@google.com.

ap...@google.com <ap...@google.com> #4Oct 28, 2020 06:31PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 9590f8c79100ea4afc084bb577ebde48f18e2d9f
Author: Nader Jawad <njawad@google.com>
Date: Tue Oct 27 20:38:49 2020

Added support to render an icon without a tint

Relnote: "Updated Icon API to take in Color.Unspecified
as a possible tint color which will draw the provided
asset or painter without a ColorFilter. Previously
attempts to ignore tinting with Color.Unspecified
would tint with a transparent color ending up
with nothing rendered at all."

Fixes: 171624632
Test: Added test to IconTest
Change-Id: I049e2b7464204f1fd8965d31d6dfba811b30a2bb

M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Icon.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/IconTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Icon.kt

https://android-review.googlesource.com/1477167

ap...@google.com <ap...@google.com> #5Oct 28, 2020 11:26PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 4bb6e2852628c5152e17ce7d45e110d873fd4b55
Author: Nader Jawad <njawad@google.com>
Date: Wed Oct 28 14:11:09 2020

Updated documentation on Icon tinting

Updated kdocs on Icon tinting to
link to Color.Unspecified.
Updated test case on to indicate
a tint color is specified.

Bug: 171624632
Test: N/A
Change-Id: Ifac2cb2c67355477786e3362997d6ea0a02ff4ea

M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Icon.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/IconTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Icon.kt

https://android-review.googlesource.com/1479019

Issue 171624632