Don't send frame callback before we have actually committed.

This avoids a problem where frame callback is reported before we have
actually committed to display a surface. This could lead to skipping
where application commits a new surface before the current surface will
be queued up for display. This breaks FIFO rules in e.g. Vulkan.
A scenario where this matters is when GPU is rendering slower than
refresh rate.

Also avoids a problem where the commit queue can grow large since
frame callbacks will be pumped faster than the GPU is able to process
the frames.

At least Mutter behavior is similar here, so I think this is the correct
interpretation.

Also fixes KHR_present_wait in Xwayland. Xwayland uses frame callback to
signal PRESENT_COMPLETE and this fixes that.

src/steamcompmgr.cpp

@@ -4553,24 +4553,21 @@ init_runtime_info()
 }
 
 static void
-steamcompmgr_send_done( steamcompmgr_win_t *w, uint64_t vblank_idx, struct timespec& now )
+steamcompmgr_flush_frame_done( steamcompmgr_win_t *w )
 {
-	wlr_surface *main_surface = w->main_surface();
 	wlr_surface *current_surface = w->current_surface();
-	bool bSendCallback = current_surface != nullptr;
+	if ( current_surface && w->unlockedForFrameCallback && w->receivedDoneCommit )
-
-	int nRefresh = g_nNestedRefresh ? g_nNestedRefresh : g_nOutputRefresh;
-	int nTargetFPS = g_nSteamCompMgrTargetFPS;
-	if ( g_nSteamCompMgrTargetFPS && steamcompmgr_window_should_limit_fps( w ) && nRefresh > nTargetFPS )
 	{
-		int nVblankDivisor = nRefresh / nTargetFPS;
+		// TODO: Look into making this _RAW
+		// wlroots, seems to just use normal MONOTONIC
+		// all over so this may be problematic to just change.
+		struct timespec now;
+		clock_gettime(CLOCK_MONOTONIC, &now);
 
-		if ( vblank_idx % nVblankDivisor != 0 )
+		wlr_surface *main_surface = w->main_surface();
-			bSendCallback = false;
+		w->unlockedForFrameCallback = false;
-	}
+		w->receivedDoneCommit = false;
 
-	if ( bSendCallback )
-	{
 		// Acknowledge commit once.
 		wlserver_lock();
 
@@ -4588,6 +4585,27 @@ steamcompmgr_send_done( steamcompmgr_win_t *w, uint64_t vblank_idx, struct times
 	}
 }
 
+static void
+steamcompmgr_latch_frame_done( steamcompmgr_win_t *w, uint64_t vblank_idx )
+{
+	bool bSendCallback = true;
+
+	int nRefresh = g_nNestedRefresh ? g_nNestedRefresh : g_nOutputRefresh;
+	int nTargetFPS = g_nSteamCompMgrTargetFPS;
+	if ( g_nSteamCompMgrTargetFPS && steamcompmgr_window_should_limit_fps( w ) && nRefresh > nTargetFPS )
+	{
+		int nVblankDivisor = nRefresh / nTargetFPS;
+
+		if ( vblank_idx % nVblankDivisor != 0 )
+			bSendCallback = false;
+	}
+
+	if ( bSendCallback )
+	{
+		w->unlockedForFrameCallback = true;
+	}
+}
+
 static void
 handle_property_notify(xwayland_ctx_t *ctx, XPropertyEvent *ev)
 {
@@ -5544,6 +5562,7 @@ bool handle_done_commit( steamcompmgr_win_t *w, xwayland_ctx_t *ctx, uint64_t co
 			// we can release all commits prior to done ones
 			w->commit_queue.erase( w->commit_queue.begin(), w->commit_queue.begin() + j );
 		}
+		w->receivedDoneCommit = true;
 		return true;
 	}
 
@@ -6532,7 +6551,7 @@ static bool g_bWasFSRActive = false;
 
 extern std::atomic<uint64_t> g_nCompletedPageFlipCount;
 
-void steamcompmgr_check_xdg()
+void steamcompmgr_check_xdg(bool vblank)
 {
 	if (wlserver_xdg_dirty())
 	{
@@ -6541,6 +6560,16 @@ void steamcompmgr_check_xdg()
 	}
 
 	handle_done_commits_xdg();
+
+	// When we have observed both a complete commit and a VBlank, we should request a new frame.
+	if (vblank)
+	{
+		for ( const auto& xdg_win : g_steamcompmgr_xdg_wins )
+		{
+			steamcompmgr_flush_frame_done(xdg_win.get());
+		}
+	}
+
 	check_new_xdg_res();
 }
 
@@ -6913,13 +6942,11 @@ steamcompmgr_main(int argc, char **argv)
 #endif
 		}
 
-		// TODO: Look into making this _RAW
-		// wlroots, seems to just use normal MONOTONIC
-		// all over so this may be problematic to just change.
-		struct timespec now;
-		clock_gettime(CLOCK_MONOTONIC, &now);
-
 		// Ask for a new surface every vblank
+		// When we observe a new commit being complete for a surface, we ask for a new frame.
+		// This ensures that FIFO works properly, since otherwise we might ask for a new frame
+		// application can commit a new frame that completes before we ever displayed
+		// the current pending commit.
 		if ( vblank == true )
 		{
 			static int vblank_idx = 0;
@@ -6929,13 +6956,13 @@ steamcompmgr_main(int argc, char **argv)
 				{
 					for (steamcompmgr_win_t *w = server->ctx->list; w; w = w->xwayland().next)
 					{
-						steamcompmgr_send_done( w, vblank_idx, now );
+						steamcompmgr_latch_frame_done( w, vblank_idx );
 					}
 				}
 
 				for ( const auto& xdg_win : g_steamcompmgr_xdg_wins )
 				{
-					steamcompmgr_send_done( xdg_win.get(), vblank_idx, now );
+					steamcompmgr_latch_frame_done( xdg_win.get(), vblank_idx );
 				}
 			}
 			vblank_idx++;
@@ -6944,7 +6971,18 @@ steamcompmgr_main(int argc, char **argv)
 		{
 			gamescope_xwayland_server_t *server = NULL;
 			for (size_t i = 0; (server = wlserver_get_xwayland_server(i)); i++)
+			{
 				handle_done_commits_xwayland(server->ctx.get());
+
+				// When we have observed both a complete commit and a VBlank, we should request a new frame.
+				if (vblank)
+				{
+					for (steamcompmgr_win_t *w = server->ctx->list; w; w = w->xwayland().next)
+					{
+						steamcompmgr_flush_frame_done(w);
+					}
+				}
+			}
 		}
 
 		// Handle presentation-time stuff
@@ -7032,7 +7070,7 @@ steamcompmgr_main(int argc, char **argv)
 			}
 		}
 
-		steamcompmgr_check_xdg();
+		steamcompmgr_check_xdg(vblank);
 
 		// Handles if we got a commit for the window we want to focus
 		// to switch to it for painting (outdatedInteractiveFocus)

src/steamcompmgr_shared.hpp

@@ -122,6 +122,9 @@ struct steamcompmgr_win_t {
 	bool nudged;
 	bool ignoreOverrideRedirect;
 
+	bool unlockedForFrameCallback;
+	bool receivedDoneCommit;
+
 	unsigned int mouseMoved;
 
 	std::vector< std::shared_ptr<commit_t> > commit_queue;