Skip to content

Commit

Permalink
Bug 1829493 - [wayland] Don't keep firing vsync at idle rate once idl…
Browse files Browse the repository at this point in the history
…e. r=rmader

Once we detect we're idle, remove the idle timer (until the frame callback
fires again, which would start the timer).

That makes vsync not fire at 1Hz on occluded windows (it makes vsync not fire
at all for those). Matching the windows behavior too.

We had this so that we didn't leak in bug 1786247, but the right fix is
bug 1828587.

Differential Revision: https://phabricator.services.mozilla.com/D176282
  • Loading branch information
emilio committed Apr 24, 2023
1 parent 1f4f99a commit b2637ab
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
7 changes: 7 additions & 0 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15348,6 +15348,13 @@
type: bool
value: true
mirror: once

# Whether to keep firing vsync at layout.throttled_frame_rate after we've been
# occluded.
- name: widget.wayland.vsync.keep-firing-at-idle
type: bool
value: false
mirror: always
#endif

#ifdef MOZ_WIDGET_GTK
Expand Down
30 changes: 20 additions & 10 deletions widget/gtk/WaylandVsyncSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# include "mozilla/ScopeExit.h"
# include "nsGtkUtils.h"
# include "mozilla/StaticPrefs_layout.h"
# include "mozilla/StaticPrefs_widget.h"
# include "nsWindow.h"

# include <gdk/gdkwayland.h>
Expand Down Expand Up @@ -229,12 +230,17 @@ void WaylandVsyncSource::SetupFrameCallback(const MutexAutoLock& aProofOfLock) {
wl_display_flush(WaylandDisplayGet()->GetDisplay());

if (!mIdleTimerID) {
mIdleTimerID = (int)g_timeout_add(
mIdleTimerID = g_timeout_add(
mIdleTimeout,
[](void* data) -> gint {
auto* vsync = static_cast<WaylandVsyncSource*>(data);
vsync->IdleCallback();
return true;
RefPtr vsync = static_cast<WaylandVsyncSource*>(data);
if (vsync->IdleCallback()) {
// We want to fire again, so don't clear mIdleTimerID
return G_SOURCE_CONTINUE;
}
// No need for g_source_remove, caller does it for us.
vsync->mIdleTimerID = 0;
return G_SOURCE_REMOVE;
},
this);
}
Expand All @@ -243,7 +249,7 @@ void WaylandVsyncSource::SetupFrameCallback(const MutexAutoLock& aProofOfLock) {
mCallbackRequested = true;
}

void WaylandVsyncSource::IdleCallback() {
bool WaylandVsyncSource::IdleCallback() {
LOG("WaylandVsyncSource::IdleCallback");
MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());

Expand All @@ -258,13 +264,14 @@ void WaylandVsyncSource::IdleCallback() {
// here without setting up a new frame callback.
LOG(" quit, mVsyncEnabled %d mMonitorEnabled %d", mVsyncEnabled,
mMonitorEnabled);
return;
return false;
}

guint duration = static_cast<guint>(
(TimeStamp::Now() - mLastVsyncTimeStamp).ToMilliseconds());
if (duration < mIdleTimeout) {
return;
// We're not idle, we want to fire the timer again.
return true;
}

LOG(" fire idle vsync");
Expand All @@ -279,11 +286,14 @@ void WaylandVsyncSource::IdleCallback() {
// This could disable vsync.
window->NotifyOcclusionState(OcclusionState::OCCLUDED);

if (window->IsDestroyed()) {
return false;
}
// Make sure to fire vsync now even if we get disabled afterwards.
// This gives an opportunity to clean up after the visibility state change.
if (!window->IsDestroyed()) {
NotifyVsync(lastVSync, outputTimestamp);
}
// FIXME: Do we really need to do this?
NotifyVsync(lastVSync, outputTimestamp);
return StaticPrefs::widget_wayland_vsync_keep_firing_at_idle();
}

void WaylandVsyncSource::FrameCallback(wl_callback* aCallback, uint32_t aTime) {
Expand Down
5 changes: 3 additions & 2 deletions widget/gtk/WaylandVsyncSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class WaylandVsyncSource final : public gfx::VsyncSource {
void DisableMonitor();

void FrameCallback(wl_callback* aCallback, uint32_t aTime);
void IdleCallback();
// Returns whether we should keep firing.
bool IdleCallback();

TimeDuration GetVsyncRate() override;

Expand Down Expand Up @@ -86,9 +87,9 @@ class WaylandVsyncSource final : public gfx::VsyncSource {
RefPtr<NativeLayerRootWayland> mNativeLayerRoot MOZ_GUARDED_BY(mMutex);
TimeDuration mVsyncRate MOZ_GUARDED_BY(mMutex);
TimeStamp mLastVsyncTimeStamp MOZ_GUARDED_BY(mMutex);
guint mIdleTimerID MOZ_GUARDED_BY(mMutex) = 0;
wl_callback* mCallback MOZ_GUARDED_BY(mMutex) = nullptr;

guint mIdleTimerID = 0; // Main thread only.
nsWindow* const mWindow; // Main thread only, except for logging.
const guint mIdleTimeout;
};
Expand Down

0 comments on commit b2637ab

Please sign in to comment.