Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support vsync, refactor rendering, rendering tests #44

Merged
merged 13 commits into from Jan 20, 2021
Merged

Support vsync, refactor rendering, rendering tests #44

merged 13 commits into from Jan 20, 2021

Conversation

igordmn
Copy link
Collaborator

@igordmn igordmn commented Jan 15, 2021 •
edited

Better to look at every commit separately, not at PR entirely.

Results of ./gradlew runVsync in Compose with 5000 frames and 2 windows

Windows 10 19041.746, 100Hz display

After:

[Window 1] FPS 99.74, missed frames 0.26%
[Window 2] FPS 99.75, missed frames 0.24%

WindowsAfter

Before (FPS is low because on Windows we used system timer which has precision 15ms):

[Window 1] FPS 63.57, missed frames 57.30%
[Window 2] FPS 63.53, missed frames 57.38%

WindowsBefore

Ubuntu 20.04.1 LTS (X11 Window Manager), 100Hz display
After:

[Window 1] FPS 99.57, missed frames 0.42%
[Window 2] FPS 99.62, missed frames 0.38%

LinuxAfter

Before (everything was ok, but with two displays application was very laggy):

[Window 1] FPS 99.19, missed frames 0.80%
[Window 2] FPS 99.13, missed frames 0.86%

LinuxBefore

macOs Catalina, 60Hz display

After:

[Window 1] FPS 59.85, missed frames 0.30%
[Window 2] FPS 59.82, missed frames 0.26%

macOsAfter

Before (system timer has resolution 1-3ms):

[Window 1] FPS 53.86, missed frames 11.34%
[Window 2] FPS 53.94, missed frames 11.20%

macOsBefore

Deltas between frames with 3 opened windows (300 frames)

Windows (100Hz monitor)

Deltas [9.7, 10.1, 9.8, 10.0, 10.4, 9.8, 9.8, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.1, 9.9, 10.0, 10.0, 10.0, 9.9, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 10.1, 9.9, 10.0, 9.9, 10.2, 9.9, 9.9, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 40.2, 9.9, 9.9, 10.0, 9.9, 10.2, 9.8, 10.3, 9.7, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 10.0, 10.0, 10.1, 10.0, 10.1, 10.0, 10.0, 10.0, 10.0, 9.9, 10.0, 10.0, 10.0, 10.3, 9.8, 10.0, 10.1, 9.9, 10.1, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 9.9, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.3, 9.7, 10.1, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 10.0, 10.0, 10.0, 10.0, 10.2, 9.9, 10.1, 9.9, 9.9, 10.0, 10.1, 9.9, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 10.0, 9.9, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 10.0, 10.0, 10.2, 9.8, 10.1, 9.9, 10.0, 10.0, 13.2, 6.9, 10.1, 10.5, 9.4, 10.0, 9.9, 10.1, 10.0, 9.9, 10.0, 10.0, 10.0, 10.0, 10.1, 10.0, 10.1, 9.9, 9.9, 10.2, 9.8, 10.1, 10.0, 9.9, 10.0, 10.1, 9.9, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 9.9, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 10.0, 10.2, 9.8, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.1, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 10.0, 10.0, 10.0, 10.0, 10.2, 10.0, 9.9, 10.1, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 10.1, 10.0, 10.0, 10.1, 9.9, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 10.1, 9.9, 10.4, 9.9, 9.8, 10.2, 10.0, 10.0, 9.9, 9.9, 10.0, 10.0, 9.9, 10.1, 10.0, 10.0, 10.1, 9.9, 10.2, 9.8, 10.0, 10.1, 10.0, 9.9, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.8, 10.1, 9.8, 10.1, 10.0]
Average 10.10
Standard deviation 1.77
[10.5, 9.4] deviate by 5%
[] deviate by 15%
[13.2, 6.9] deviate by 30%
[40.2] deviate by 50%

Ubuntu (100Hz monitor)

Deltas [6.6, 9.5, 10.6, 9.2, 10.7, 9.6, 10.6, 9.5, 10.4, 9.5, 10.8, 9.7, 10.0, 9.6, 10.4, 9.5, 10.5, 9.5, 10.1, 10.0, 10.5, 9.7, 10.1, 9.6, 10.5, 9.5, 10.5, 9.5, 10.5, 9.5, 10.6, 9.3, 10.4, 9.6, 10.8, 9.5, 10.5, 9.4, 10.4, 9.5, 10.5, 9.5, 10.6, 9.4, 10.3, 10.0, 10.3, 9.5, 10.5, 9.4, 10.4, 9.5, 10.5, 9.6, 10.4, 9.5, 10.5, 9.8, 9.6, 10.3, 10.5, 9.5, 10.4, 9.5, 10.6, 9.4, 10.5, 9.5, 10.0, 10.0, 10.3, 10.8, 10.0, 10.1, 9.9, 11.6, 8.3, 9.5, 9.8, 10.0, 10.0, 9.9, 10.4, 9.7, 10.3, 9.4, 11.0, 9.3, 10.5, 9.5, 10.4, 9.6, 10.4, 9.5, 10.5, 9.6, 10.2, 9.7, 10.7, 10.0, 10.0, 9.9, 10.0, 9.5, 10.5, 9.5, 10.4, 10.1, 9.9, 9.3, 10.7, 9.6, 10.5, 10.1, 9.9, 9.5, 10.5, 10.0, 10.1, 9.9, 10.0, 10.0, 9.8, 9.6, 10.8, 9.5, 10.5, 9.4, 10.4, 9.7, 10.3, 10.0, 9.5, 10.0, 10.5, 10.0, 10.6, 9.6, 10.5, 10.3, 9.2, 10.1, 10.9, 9.3, 10.8, 8.7, 10.7, 9.3, 10.5, 9.9, 10.6, 9.3, 10.0, 10.0, 9.9, 9.5, 10.4, 10.0, 10.0, 9.5, 10.0, 10.1, 9.8, 10.2, 10.6, 10.0, 9.9, 9.5, 10.0, 10.0, 10.5, 10.1, 9.4, 10.1, 10.0, 9.9, 10.1, 10.0, 10.0, 10.0, 9.9, 10.0, 10.1, 9.9, 10.0, 10.0, 10.8, 10.0, 9.6, 9.8, 10.0, 10.0, 10.0, 10.0, 10.1, 10.0, 10.0, 10.0, 9.9, 10.0, 10.0, 10.0, 10.2, 10.0, 10.2, 9.8, 9.9, 10.4, 9.6, 10.3, 10.0, 10.4, 9.7, 9.4, 10.5, 9.6, 9.9, 10.0, 9.8, 10.3, 10.0, 10.0, 10.0, 9.9, 10.0, 10.0, 10.1, 10.4, 9.3, 10.3, 10.0, 10.0, 10.0, 9.9, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 10.6, 9.5, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.1, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 10.1, 9.9, 9.9, 10.0, 10.0, 10.1, 10.0, 10.0, 10.1, 9.9, 9.9, 10.0, 10.0, 10.0, 10.0, 10.1, 9.9, 10.0, 9.8, 10.4, 9.9, 10.0, 10.0, 9.9, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 9.8, 10.3, 10.0, 10.0, 10.0, 9.9]
Average 9.99
Standard deviation 0.44
[9.5, 10.6, 9.2, 10.7, 10.6, 9.5, 9.5, 10.8, 10.5, 9.5, 10.5, 9.5, 10.6, 9.3, 10.8, 9.5, 10.5, 9.4, 10.6, 9.4, 9.5, 10.5, 9.4, 9.5, 10.5, 9.5, 9.5, 10.6, 9.4, 10.8, 9.5, 9.4, 11.0, 9.3, 10.5, 9.5, 10.7, 9.3, 10.7, 10.5, 9.5, 10.5, 10.8, 9.5, 10.5, 9.4, 10.6, 10.5, 9.2, 10.9, 9.3, 10.8, 8.7, 10.7, 9.3, 10.5, 10.6, 9.3, 9.5, 10.6, 9.5, 9.4, 10.8, 9.4, 10.5, 9.3, 10.6, 9.5] deviate by 5%
[11.6, 8.3] deviate by 15%
[] deviate by 30%
[6.6] deviate by 50%

macOs (60Hz monitor)

Deltas [16.6, 17.1, 16.6, 16.6, 16.7, 16.7, 16.4, 17.0, 16.6, 17.1, 16.6, 16.5, 16.3, 17.1, 17.0, 16.5, 16.9, 16.4, 16.6, 17.1, 16.5, 16.8, 16.5, 16.5, 16.8, 16.4, 17.1, 16.1, 17.0, 16.8, 16.7, 16.2, 17.1, 16.4, 16.8, 16.4, 16.9, 16.7, 16.4, 16.7, 16.4, 17.3, 16.1, 17.1, 16.7, 16.6, 16.6, 16.0, 16.5, 17.1, 16.7, 16.6, 16.7, 16.6, 16.4, 17.0, 16.6, 16.7, 16.7, 16.7, 16.7, 16.7, 16.6, 16.7, 16.7, 16.7, 16.3, 17.0, 16.7, 16.7, 24.0, 9.1, 17.0, 16.6, 16.7, 16.6, 16.7, 16.3, 17.2, 17.0, 16.4, 16.8, 16.4, 16.9, 16.5, 16.0, 17.2, 17.0, 16.5, 16.8, 16.7, 16.6, 16.4, 16.3, 17.1, 16.8, 16.4, 17.2, 16.5, 16.7, 16.4, 16.7, 16.7, 16.5, 17.1, 16.7, 16.7, 16.7, 16.7, 16.5, 16.6, 16.7, 16.6, 16.9, 16.5, 16.7, 16.4, 16.5, 16.9, 16.8, 16.5, 16.9, 16.3, 17.0, 16.5, 16.9, 16.4, 16.8, 16.8, 16.3, 17.0, 16.7, 16.4, 16.6, 16.7, 16.7, 16.7, 16.6, 16.5, 16.5, 16.7, 16.4, 17.4, 16.4, 16.6, 16.6, 16.5, 17.1, 16.4, 16.6, 16.8, 16.7, 16.8, 16.6, 16.6, 16.7, 16.4, 16.5, 16.9, 16.6, 16.8, 16.6, 16.4, 16.9, 16.4, 17.0, 16.7, 16.4, 17.0, 16.6, 16.6, 16.7, 16.8, 16.7, 16.5, 16.7, 16.7, 16.2, 17.2, 16.4, 16.9, 16.2, 16.9, 16.9, 16.5, 16.8, 16.2, 17.0, 16.7, 16.8, 16.6, 16.7, 16.7, 16.4, 16.8, 16.7, 16.5, 16.2, 17.2, 16.4, 16.9, 16.4, 17.1, 16.6, 16.7, 17.0, 16.4, 16.7, 16.8, 16.6, 16.7, 16.7, 16.5, 16.0, 17.3, 16.9, 16.3, 16.8, 16.7, 16.6, 16.9, 16.6, 16.5, 16.8, 16.7, 16.5, 16.8, 16.7, 16.6, 16.6, 16.4, 17.0, 16.7, 16.7, 16.3, 17.1, 16.3, 16.7, 16.9, 16.7, 16.6, 16.7, 16.6, 16.3, 17.0, 16.6, 16.8, 16.7, 16.7, 16.7, 16.3, 16.6, 16.8, 16.6, 16.9, 16.5, 16.7, 16.6, 16.8, 16.5, 16.9, 16.5, 16.8, 16.7, 16.3, 16.8, 16.5, 16.8, 17.0, 16.6, 16.7, 16.5, 16.8, 16.7, 16.6, 16.6, 16.6, 16.6, 16.8, 16.2, 17.2, 16.3, 16.8, 16.8, 16.8, 16.5, 16.7, 16.8, 16.6, 16.5, 16.9, 16.6, 16.5, 16.9, 16.6, 16.7, 16.5, 16.7, 16.8]
Average 16.67
Standard deviation 0.65
[16.1, 16.1, 16.0, 16.0, 16.0] deviate by 5%
[] deviate by 15%
[24.0] deviate by 30%
[9.1] deviate by 50%

@igordmn
Support coroutines, migrate to Kotlin 1.21
8866df9
@igordmn
Build all source files for macOs inside jvmMain/objectiveC/macos
5c82039
@igordmn
FPSCounter
b662436
@igordmn
Draw into picture, refactor SkiaLayer
f970ae3 
1. Split onRender into performUpdate and performDraw.
performUpdate will be called from the Swing thread in upcoming commits (to avoid race conditions)

2. Remove canvas.scale by default

Motivation: automatic scaling is harmful.
Example:
- skiko returns us 100x100 canvas size.
- We load image from disk and reduce its size to 100x100.
- We draw this image onto canvas, and it is scaled 2x with artifacts.
If skiko returns us 200x200 then we reduce image to 200x200 and draw it onto canvas without scaling.

3. Cache contentScale
It is expensive operation on Linux to do it in every frame (we read database)

4. Remove onInit, onReshape, onDispose removed from SkiaRenderer.
It is hard to implement it in the right thread (on macOs we have AppKit and AWT thread). Besides we don't use them in Compose.
If they will be needed then we can implement them later.
@igordmn
Support vsync
620547a
@igordmn
Screenshot based testing
093421c
@igordmn
SkiaWindow tests
ef910e6
@igordmn igordmn mentioned this pull request Jan 15, 2021
Open
@igordmn igordmn changed the title Support vsync, refactor rendering Support vsync, refactor rendering, rendering tests Jan 15, 2021
olonho
olonho reviewed Jan 16, 2021
View reviewed changes
Copy link
Contributor

@olonho olonho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fullscreen seems to be broken with this PR.

olonho
olonho approved these changes Jan 19, 2021
View reviewed changes
Copy link
Contributor

@olonho olonho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me, if fullscreen is fixed.

@igordmn igordmn mentioned this pull request Jan 19, 2021
Open
@igordmn igordmn merged commit 87e0a74 into master Jan 20, 2021
copybara-service bot pushed a commit to androidx/androidx that referenced this pull request Jan 28, 2021
@igordmn
Desktop: migrate to Skiko 0.2.4, support vsync, refactor ComposeLayer
d4827cb 
- SkiaLayer now produces a frame every vsync signal,
  so we don't have to use a timer [FrameDispatcher, `delay`].
  The timer isn't precise and cause frame skipping.
- needRedraw/FrameDispatcher now are part of SkiaLayer in skiko
- onRender is now always called from AWT thread, so we don't have to create a temporarily picture [ComposeLayer, `preparePicture`]
- we can use "throw e" instead of System.exit inside onRender, because it is a Java thread now [ComposeLayer, `onRender`]
- use layer.contentScale instead of wrapped.graphicsConfiguration.defaultTransform.scaleX (in Linux it has wrong density) [ComposeLayer, `detectCurrentDensity`]
- move initialization of DesktopOwner to SkiaLayer.init (layer.contentScale is available only after init)
- now we don't have to manually call layer.reinit() and layer.updateLayer(), it is called automatically [ComposeWindow]
- remove FPSTracker. FPS can be measured by adding systemProperty("skiko.fps.enabled", "true") inside build.gradle [ComposeWindow]

Vsync test results:
JetBrains/skiko#44 (comment)

Change-Id: I45ccf56a325d9ea87af13c247f5fd959a9bc3618
Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true
@igordmn igordmn deleted the vsync branch January 28, 2021 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rsedaikin Rsedaikin Rsedaikin left review comments

@olonho olonho olonho approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@igordmn @olonho @Rsedaikin