Separated PR 3117 changes that have nothing to do with graphics#3164
Separated PR 3117 changes that have nothing to do with graphics#3164ApoloApps wants to merge 21 commits into
Conversation
…lections to avoid boxing, iterators and improve performance. (Similar change to PointerToPositionMap)
…e dirtyLayers.clear() inside the dirtyLayers.isNotEmpty() check as it does not make sense to clear if dirtyLayers is empty
…hod was iterating over a js value API in a kotlin loop and removing first child. This could cause freezes of up to 50 ms (observed during high-usage memory in a device). I observed a 5.8% reduction in time between main function call and Compose being composed, which amounted to around 7-8 ms (a frame, in my device). Also, the method used has been baseline since 2020 and it is the preferred way as it avoids crossing wasm to Js boundary over and over several times
…nstead Unspecified and Zero versions of it (compiles down to a simple primitive)
…init. (onPointerCallback was being created 6 times on init, delaying scene startup). It also avoid an iterator allocation on init
… from the buildLongSet method instead of the outer extension function, causing the Set to be empty each time
|
Ivan Matkov (@MatkovIvan) separated changes that have nothing to do with graphics (the other changes are blocked until #3126 is merged) to at least unblock them from merging from #3117 |
| // modifiers as the previous event. | ||
| // Note that missing move events for this event should have already been sent | ||
| fun areSameParams(e1: PointerInputEvent, e2: PointerInputEvent): Boolean { | ||
| if (e1.pressedIds().toSet() != e2.pressedIds().toSet()) return false |
There was a problem hiding this comment.
It seems it changes the logic here (remove duplicates)
Igor Demin (@igordmn) ptal
There was a problem hiding this comment.
ApoloApps, pressedIds can arrive in a different order, we need to check that.
There was a problem hiding this comment.
So do I need to readd toSet method call so that duplicates are cleared?? No pointer order has been changed, I was just trying to avoid 2 avoidable allocations (2 Sets)
There was a problem hiding this comment.
Platforms most probably can't send duplicates (if some platform can, it should be filtered on the ComposeScene level, not here).
But they can send pointers in another order, this is valid. If you know a way to compare without an allocation, you can use it, but it should be simple or extracted to a simple function.
There was a problem hiding this comment.
I was a bit tired and didn't get it. Set equality uses a loop with contains check which makes it so that order does not matter. List does care about order though. Got it! Refactored and added some overload to avoid intermediate Lists and instead return Set (added also some fast paths to avoid even more allocations). Done!
| * This is used to avoid [Rect] object allocations on hot paths | ||
| */ | ||
| @Stable | ||
| internal inline fun IntSize.contains(offset: Offset): Boolean { |
There was a problem hiding this comment.
it seems semantically wrong - "contains" is not something that we can ask from size.
It might be just naming issues, but I'm not sure what fits better here
There was a problem hiding this comment.
yep, I had the same dichotomy. encloses?? bounds??
if (size.bounds(offset)) bounds being used here as a verb
if (size.encloses(offset)) it is inferred (at least imo) that we are referring to the area between 0,0 and this Size's coordinates
There was a problem hiding this comment.
renamed to bounds and updated KDoc. Done
Ivan Matkov (MatkovIvan)
requested review from
Oleksandr Karpovich (eymar) and
Igor Demin (igordmn)
…th more optimized Collections and more fast paths that avoid allocations
4 participants
Separated the changes from #3117 that don't involve graphics (which are currently blocked until separation of Skiko from Compose Ui and migration to Runtime Registration) as to unblock them from merging into main
This includes changes SyntheticEventSender and more minor changes
Release Notes
N/A