-
Notifications
You must be signed in to change notification settings - Fork 118
Use layer placeholder #1766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use layer placeholder #1766
Conversation
92bc553 to
31d7915
Compare
igordmn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks better now 👍.
Though, we should find the performance regression reason.
| @@ -1000,7 +1000,7 @@ public final class androidx/compose/ui/graphics/SkiaGraphicsContext : androidx/c | |||
| public fun <init> (Z)V | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested FPS on run1 (x8 App). We have a new regression:
62 -> 55.
11% compared to jb-main (plus we have the previous 3% regression)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, the regression is too big for merging. We can't be sure there is an easy fix, as it can depend on Skia internals.
We should figure it out before merging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see different numbers locally. I did +- obvious optimization with matrices’ caching, so fps is a bit higher than on jb-main for me now. But since I didn't observe such a drop previously it's hard to tell how it will behave on your devices. Let's continue offline to figure it out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also:
PC
wasm
reduced run1, x8 App: 99 -> 74
js
reduced run1, x8 App: 101 -> 75
desktop
widget gallery x160: 68 -> 42
codeviewer x20: 144 -> 92
run1 OPENGL, SOFTWARE: similar regression
run1 x8 App: 62 -> 55, 64 -> 57 (new test)
run1 x8 App + System.setProperty("skiko.gpu.priority", "integrated"): 49 -> 46
Windows laptop (only integrated GPU)
desktop
run1 x3 App. 55 -> 53
...-graphics/src/skikoMain/kotlin/androidx/compose/ui/graphics/layer/SkiaGraphicsLayer.skiko.kt
Outdated
Show resolved
Hide resolved
New `PictureFilterCanvas` class that allows override picture rendering during drawing Required for JetBrains/compose-multiplatform-core#1766
Reverts shadow behavior changes from #1754 until #1766 In the current intermediate state it has the next problems: - Missing shadows updates inside inner layers (might be fixed by introducing more states d3b624c) - Incorrect positioning of shadows in a scrollable list: position inside scroll inside position on screen (no easy fix, only #1766 or revert)
This reverts commit 38ad3c2.
439aa42 to
49f7817
Compare
|
Superseded by #1784 |
Implement rendering using layers placeholders approach. It aligns caching (display list recording) with Android