https://kotlinlang.org logo
Join Slack
Powered by
When clip for large containers with `GenericShape`...
# compose
k
When clip for large containers with 
GenericShape
, click listener won't work after scroll to a certain position, which is a very urgent bug, and it has been there for a long time. Code is in🧵
VID_20251206_103551_492.mp4
Copy code
Column(
    Modifier
        .padding(16f.dp)
        .verticalScroll(rememberScrollState(100000))
        .fillMaxSize()
) {
    Column(
        Modifier
            .fillMaxSize()
            .clip(remember {
                GenericShape { size, layoutDirection ->
                    addRect(size.toRect())
                }
            })
            .background(Color.Cyan)
    ) {
        repeat(1000) {
            Text(
                it.toString(),
                Modifier
                    .clickable {}
                    .padding(160f.dp)
                    .fillMaxWidth()
            )
        }
    }
}
r
Try to switch to a lazy column that should fix it
Some of the internal types have limited range to avoid allocations
l
This is a known issue, there is a bug deep down in android graphics
k
Will it be the same in views?
l
I'm not sure, there might be a way we can work around this by using a different API to calculate path intersection
So maybe views also uses a different approach
r
@Louis Pullen-Freilich [G] I would expect this issue to be cause by loss of precision in floats when the magnitude gets higher. A solution to this would be to ensure that hit tests are done in the viewport coordinate space
@Louis Pullen-Freilich [G] And does it use 
PathHitTester
? This is basically a straight port to Kotlin of the Skia implementation, but you're still bound by float precision
l
From the previous investigation, we traced this down to an issue in skia path ops, but there should be a way we can implement this differently to handle this better
We don't currently use PathHitTester, I think this code long predates that. Maybe that is something we could use here instead? https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/[…]lin/androidx/compose/ui/platform/ShapeContainingUtil.kt;l=148
r
Yeah I wrote path hit tester specifically for this kind of use cases and it's how I tested it
Path ops have tons of edge cases (and rightly so it's a complex problem)
The hit tester basically does what one of the Skia folks said and iterates the path
It also uses an interval tree to test the smallest number of curves possible too
l
Makes sense, I can try to update to use this. Thanks for the pointer!
❤️ 1
Was looking into this, unfortunately there are quite a few subtle differences in the calculation. Maybe some of these are ok, but it's definitely a non-trivial behavior change that might break some things. E.g for a circle / round rect:
Copy code
@Test
fun test() {
    val path =
        Path().apply {
            addRoundRect(
                RoundRect(Rect(Offset.Zero, Size(2f, 2f)), cornerRadius = CornerRadius(1f, 1f))
            )
            // also same with below
            // addOval(Rect(Offset.Zero, Size(2f, 2f)))
        }
    val hitTester = PathHitTester(path)
    // true
    assertTrue(Offset(0.1f, 1f) in hitTester)
    // false
    assertTrue(Offset(0.01f, 1f) in hitTester)
    // false
    assertTrue(Offset(0f, 1f) in hitTester)
}
Maybe my expectations are just wrong here 😅
r
The last two make sense to me (one is outside, the other one is exactly on the circle which is considered outside, see the comment left in the implementation)
The first one should be false though I think?
Although wait, I'm assuming the shape is centered around 0 here. I need to draw this on screen to think about it lol
l
ah sorry yeah, it took me a while too haha. The shape is centered around 1,1 here
see the comment left in the implementation
I missed this, let me have a look
r
Yes. Only happens when the point is exactly on the curve (which for touch… meh 🙂
l
I guess we can hope that users are inaccurate 😛
r
the touch screen is anyway
l
yeah, trickier for mouse, but mostly in tests where it's likely that someone will inject an exact touch event that matches the outline
r
that can be fixed
l
you mean by checking if the point is exactly on the path?
r
yeah
oh so 0.1f/1f is inside the circle
l
ah yeah, for sure we can do that. Just requires some work
r
You can check the implementation in Skia and just port that
Another solution, but it's Android only, would be to convert the path to a region and do a hit test against the region
l
Yeah, might have some performance overhead / differences too that woudl need measuring
r
There will be differences compared to the current solution that assumes a point that's not a point
Of all of them, the Path.op version is most likely to be the least robust/exact
Turning a path into a region is more or less equivalent to drawing the path on the CPU
l
Yeah, the current one is definitely problematic
It's just difficult to change anything in flight like this
r
🤷 better than something that just breaks sometimes
You could also do a multi-tap: instead of testing 1 point, test a group of 4 to do something closer to the current behavior and if any point hits, there's a hit
l
yeah we need to fix this for sure
multi-tap also reasonable, need to benchmark though
r
PathHitTester
was built with multiple queries in mind, so it should be reasonably fast (I hope) and produce no allocations
What it does is perform a spatial query through an interval tree to only test the curves of the path that intersect the row of pixels where the hit is happening
(in the round rect case it should therefore test only 2 curves)
l
ah great, ok, maybe that's something we can do
4 Views
Open in Slack
PreviousNext
Powered by