Merge lp:~vanvugt/gtk/fix-1698270 into lp:~ubuntu-desktop/gtk/ubuntugtk3

Proposed by Daniel van Vugt
Status: Rejected
Rejected by: Jeremy Bícha
Proposed branch: lp:~vanvugt/gtk/fix-1698270
Merge into: lp:~ubuntu-desktop/gtk/ubuntugtk3
Diff against target: 112 lines (+92/-0)
3 files modified
debian/changelog (+8/-0)
debian/patches/Fix-irregular-gdk_frame_clock_get_frame_time-v3-ubuntu.patch (+83/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp:~vanvugt/gtk/fix-1698270
Reviewer Review Type Date Requested Status
Jeremy Bícha Disapprove
Ubuntu Sponsors Pending
Review via email: mp+331846@code.launchpad.net

Commit message

Fix irregular gdk_frame_clock_get_frame_time

This fixes stuttering in animations that rely on the regularity of
gdk_frame_clock_get_frame_time. (LP: #1698270)

BEFORE

gdkgears: 58 FPS and visibly stuttering

gnome-maps on a 59.95Hz monitor:
"paint" gdk_frame_clock_get_frame_time +17278μs
"paint" gdk_frame_clock_get_frame_time +17426μs
"paint" gdk_frame_clock_get_frame_time +17600μs
^^^ misses about 5% of frames, or stutters 3 times per sec ^^^

AFTER

gdkgears: 60 FPS and smoother

gnome-maps on a 59.95Hz monitor:
"paint" gdk_frame_clock_get_frame_time +16680μs
"paint" gdk_frame_clock_get_frame_time +16680μs
"paint" gdk_frame_clock_get_frame_time +16680μs
^^^ misses no frames ^^^

To post a comment you must log in.
lp:~vanvugt/gtk/fix-1698270 updated
579. By Daniel van Vugt

Merge latest 'ubuntugtk3' and fix changelog conflict.

580. By Daniel van Vugt

Merge latest 'ubuntugtk3' and fix changelog conflict.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

This patch has now landed upstream (on master GTK+4.0 only I think)
https://git.gnome.org/browse/gtk+/commit/?id=3b2f9395905ec2d9696bcf51497781236c95ec63

Revision history for this message
Jeremy Bícha (jbicha) wrote :

I'm rejecting this merge proposal because we got the fix upstream in 3.22.28 which has now landed in bionic:
https://launchpad.net/ubuntu/+source/gtk+3.0/3.22.28-1ubuntu1

Thanks!

review: Disapprove

Unmerged revisions

580. By Daniel van Vugt

Merge latest 'ubuntugtk3' and fix changelog conflict.

579. By Daniel van Vugt

Merge latest 'ubuntugtk3' and fix changelog conflict.

578. By Daniel van Vugt

First import

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2017-11-28 22:56:45 +0000
+++ debian/changelog 2017-12-12 07:56:30 +0000
@@ -1,3 +1,11 @@
1gtk+3.0 (3.22.26-2ubuntu2) UNRELEASED; urgency=medium
2
3 * Fix-irregular-gdk_frame_clock_get_frame_time-v3-ubuntu.patch:
4 This fixes stuttering in animations that rely on the regularity of
5 gdk_frame_clock_get_frame_time. (LP: #1698270)
6
7 -- Daniel van Vugt <daniel.van.vugt@canonical.com> Fri, 03 Nov 2017 15:41:31 +0800
8
1gtk+3.0 (3.22.26-2ubuntu1) bionic; urgency=medium9gtk+3.0 (3.22.26-2ubuntu1) bionic; urgency=medium
210
3 * Merge with Debian. Remaining changes:11 * Merge with Debian. Remaining changes:
412
=== added file 'debian/patches/Fix-irregular-gdk_frame_clock_get_frame_time-v3-ubuntu.patch'
--- debian/patches/Fix-irregular-gdk_frame_clock_get_frame_time-v3-ubuntu.patch 1970-01-01 00:00:00 +0000
+++ debian/patches/Fix-irregular-gdk_frame_clock_get_frame_time-v3-ubuntu.patch 2017-12-12 07:56:30 +0000
@@ -0,0 +1,83 @@
1Description: Fix irregular gdk_frame_clock_get_frame_time
2 This fixes stuttering in animations that rely on the regularity of
3 gdk_frame_clock_get_frame_time.
4 .
5 BEFORE
6 gdkgears:
7 58 FPS and visibly stuttering
8 gnome-maps on a 59.95Hz monitor:
9 "paint" g_get_monotonic_time +17278μs, gdk_frame_clock_get_frame_time +17278μs
10 "paint" g_get_monotonic_time +17449μs, gdk_frame_clock_get_frame_time +17426μs
11 "paint" g_get_monotonic_time +17620μs, gdk_frame_clock_get_frame_time +17600μs
12 .
13 AFTER
14 gdkgears:
15 60 FPS and smoother
16 gnome-maps on a 59.95Hz monitor:
17 "paint" g_get_monotonic_time +18228μs, gdk_frame_clock_get_frame_time +16680μs
18 "paint" g_get_monotonic_time +15010μs, gdk_frame_clock_get_frame_time +16680μs
19 "paint" g_get_monotonic_time +17134μs, gdk_frame_clock_get_frame_time +16680μs
20Author: Daniel van Vugt <daniel.van.vugt@canonical.com>
21Bug-Ubuntu: https://bugs.launchpad.net/bugs/1698270
22Bug: https://bugzilla.gnome.org/show_bug.cgi?id=787665
23Forwarded: yes
24Last-Update: 2017-09-15
25
26---
27 gdk/gdkframeclockidle.c | 31 ++++++++++++++++++++++++++++++-
28 1 file changed, 30 insertions(+), 1 deletion(-)
29
30diff --git a/gdk/gdkframeclockidle.c b/gdk/gdkframeclockidle.c
31index 12897f4236..a0ca0ca1b9 100644
32--- a/gdk/gdkframeclockidle.c
33+++ b/gdk/gdkframeclockidle.c
34@@ -123,6 +123,7 @@ gdk_frame_clock_idle_init (GdkFrameClockIdle *frame_clock_idle)
35 frame_clock_idle->priv = priv =
36 gdk_frame_clock_idle_get_instance_private (frame_clock_idle);
37
38+ priv->frame_time = g_get_monotonic_time (); /* more sane than zero */
39 priv->freeze_count = 0;
40 }
41
42@@ -350,9 +351,37 @@ gdk_frame_clock_paint_idle (void *data)
43 case GDK_FRAME_CLOCK_PHASE_BEFORE_PAINT:
44 if (priv->freeze_count == 0)
45 {
46- priv->frame_time = compute_frame_time (clock_idle);
47+ gint64 frame_interval = FRAME_INTERVAL;
48+ gint64 reset_frame_time;
49+ gint64 smoothest_frame_time;
50+ gint64 frame_time_error;
51+ GdkFrameTimings *prev_timings =
52+ gdk_frame_clock_get_current_timings (clock);
53+
54+ if (prev_timings && prev_timings->refresh_interval)
55+ frame_interval = prev_timings->refresh_interval;
56+
57+ /* We are likely not getting precisely even callbacks in real
58+ * time, particularly if the event loop is busy.
59+ * This is a documented limitation in the precision of
60+ * gdk_threads_add_timeout_full and g_timeout_add_full.
61+ *
62+ * In order to avoid this imprecision from compounding between
63+ * frames and affecting visual smoothness, we correct frame_time
64+ * to more precisely match the even refresh interval of the
65+ * physical display. This also means we proactively avoid (most)
66+ * missed frames before they occur.
67+ */
68+ smoothest_frame_time = priv->frame_time + frame_interval;
69+ reset_frame_time = compute_frame_time (clock_idle);
70+ frame_time_error = ABS (reset_frame_time - smoothest_frame_time);
71+ if (frame_time_error >= frame_interval)
72+ priv->frame_time = reset_frame_time;
73+ else
74+ priv->frame_time = smoothest_frame_time;
75
76 _gdk_frame_clock_begin_frame (clock);
77+ /* Note "current" is different now so timings != prev_timings */
78 timings = gdk_frame_clock_get_current_timings (clock);
79
80 timings->frame_time = priv->frame_time;
81--
822.14.1
83
084
=== modified file 'debian/patches/series'
--- debian/patches/series 2017-11-28 22:56:26 +0000
+++ debian/patches/series 2017-12-12 07:56:30 +0000
@@ -25,3 +25,4 @@
25unity-headerbar-maximized-mode.patch25unity-headerbar-maximized-mode.patch
26gtksocket-unscale-before-sending-configurenotify.patch26gtksocket-unscale-before-sending-configurenotify.patch
27no_content_hub.patch27no_content_hub.patch
28Fix-irregular-gdk_frame_clock_get_frame_time-v3-ubuntu.patch

Subscribers

People subscribed via source and target branches