Fix ComposePanel background
#2670
Conversation
| /** | ||
| * Asserts that the two colors are within the given tolerance of each other, on each component. | ||
| */ | ||
| fun assertColorsWithinTolerance(expected: java.awt.Color, actual: java.awt.Color, tolerance: Int = 5) { |
There was a problem hiding this comment.
Why do we need tolerance here? It should be a strict match in cases of Int values
There was a problem hiding this comment.
The pixel colors returned by java.awt.Robot don't exactly match the colors drawn by skia.
There was a problem hiding this comment.
It uses different color spaces, you need to convert it into equal instead of such comparison
There was a problem hiding this comment.
Couldn't find the right conversion after trying several things:
- The color space of the
java.awt.Colorreturned byRobot. - The color space of
frame.graphicsConfiguration.
Changed the way the test is performed. Compare to a known correct screen color value instead of using tolerance.
| get() = contentComponent.transparency | ||
| set(value) { | ||
| contentComponent.transparency = value | ||
| if (value && !windowContext.isWindowTransparent && renderApi == GraphicsApi.METAL) { |
There was a problem hiding this comment.
Where is this METAL check now?
There was a problem hiding this comment.
I don't think there a need for it anymore.
Before the change in skiko
contentComponent.transparency = true would set the background color to Color(0, 0, 0, 0) which was the wrong value for the case of !windowContext.isWindowTransparent && renderApi == GraphicsApi.METAL (it needed to be null in this case).
After the change in skiko
contentComponent.transparency = true no longer modifies the background; now the code here sets it by itself on the next line. For the case when !windowContext.isWindowTransparent it will set it to null, just like before.
The difference
The difference will only be when all 3 below are true:
transparency = true!windowContext.isWindowTransparentrenderApiis notMETAL.
Before the change the background was set toColor(0, 0, 0, 0)and now it will be set tonull.
But what is the correct value in this case? When does transparency && !windowContext.isWindowTransparent even happen?
There was a problem hiding this comment.
When does transparency && !windowContext.isWindowTransparent even happen?
Experimental interop on a regular (non transparent) window
There was a problem hiding this comment.
Can you give me a sample to try/test?
There was a problem hiding this comment.
Changed the code such that the background is null when !windowContext.isWindowTransparent && (renderApi == GraphicsApi.METAL), as before.
m-sasha
force-pushed
the
m-sasha/fix-ComposePanel-background
branch
from
2ddf1c0 to
5a56a2f
Compare
compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/awt/ComposePanelTest.kt
Outdated
Show resolved
Hide resolved
…mate color comparison.
m-sasha
force-pushed
the
m-sasha/fix-ComposePanel-background
branch
from
5a56a2f to
6e1b704
Compare
…en `!windowContext.isWindowTransparent && (renderApi == GraphicsApi.METAL)`
Fix
ComposePaneldrawing the background that was set on it.This follows Skiko's Fix SkiaLayer background drawing
Fixes https://youtrack.jetbrains.com/issue/CMP-8738
Testing
Added a unit test and tested manually.
This should be tested by QA
Release Notes
Fixes - Desktop
ComposePaneldrawing the background that was set on it.