-
Notifications
You must be signed in to change notification settings - Fork 326
backend: change VRR behavior #2420
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ use crate::{ | |
| state::SurfaceDmabufFeedback, | ||
| utils::prelude::*, | ||
| wayland::handlers::{ | ||
| compositor::recursive_frame_time_estimation, | ||
| compositor::{FULLSCREEN_IMMEDIATE_RENDER, recursive_frame_time_estimation}, | ||
| image_copy_capture::{FrameHolder, PendingImageCopyData, SessionData, submit_buffer}, | ||
| }, | ||
| }; | ||
|
|
@@ -74,7 +74,7 @@ use smithay::{ | |
| }, | ||
| wayland_server::protocol::wl_surface::WlSurface, | ||
| }, | ||
| utils::{Clock, Monotonic, Physical, Point, Rectangle, Transform}, | ||
| utils::{Clock, IsAlive, Monotonic, Physical, Point, Rectangle, Transform}, | ||
| wayland::{ | ||
| dmabuf::{DmabufFeedbackBuilder, get_dmabuf}, | ||
| image_copy_capture::{ | ||
|
|
@@ -92,7 +92,7 @@ use std::{ | |
| collections::{HashMap, HashSet, hash_map}, | ||
| mem, | ||
| sync::{ | ||
| Arc, RwLock, | ||
| Arc, LazyLock, RwLock, | ||
| atomic::{AtomicBool, Ordering}, | ||
| mpsc::{Receiver, SyncSender}, | ||
| }, | ||
|
|
@@ -105,6 +105,16 @@ pub use self::timings::Timings; | |
|
|
||
| use super::{drm_helpers, render::gles::GbmGlowBackend}; | ||
|
|
||
| static FULLSCREEN_SKIP_OTHER_SURFACE: LazyLock<bool> = LazyLock::new(|| { | ||
| crate::utils::env::bool_var("COSMIC_FULLSCREEN_SKIP_OTHER_SURFACE").unwrap_or(true) | ||
| }); | ||
|
|
||
| static FULLSCREEN_SKIP_OTHER_SURFACE_ALWAYS: LazyLock<bool> = LazyLock::new(|| { | ||
| crate::utils::env::bool_var("COSMIC_FULLSCREEN_SKIP_OTHER_SURFACE_ALWAYS").unwrap_or(false) | ||
| }); | ||
|
|
||
| const _30_HZ: Duration = Duration::from_nanos(1_000_000_000 / 30); | ||
|
|
||
| #[cfg(feature = "debug")] | ||
| use smithay_egui::EguiState; | ||
|
|
||
|
|
@@ -153,6 +163,9 @@ pub struct SurfaceThreadState { | |
| loop_handle: LoopHandle<'static, Self>, | ||
| clock: Clock<Monotonic>, | ||
|
|
||
| min_vrr: Option<u32>, | ||
| min_vrr_frame_time: Option<Duration>, | ||
|
|
||
| #[cfg(feature = "debug")] | ||
| egui: EguiState, | ||
|
|
||
|
|
@@ -186,7 +199,10 @@ pub enum QueueState { | |
| /// A redraw is queued. | ||
| Queued(RegistrationToken), | ||
| /// We submitted a frame to the KMS and waiting for it to be presented. | ||
| WaitingForVBlank { redraw_needed: bool }, | ||
| WaitingForVBlank { | ||
| redraw_needed: bool, | ||
| fullscreen_request: bool, | ||
| }, | ||
| /// We did not submit anything to KMS and made a timer to fire at the estimated VBlank. | ||
| WaitingForEstimatedVBlank(RegistrationToken), | ||
| /// A redraw is queued on top of the above. | ||
|
|
@@ -215,7 +231,7 @@ pub enum ThreadCommand { | |
| UpdateMirroring(Option<Output>), | ||
| UpdateScreenFilter(ScreenFilter), | ||
| VBlank(Option<DrmEventMetadata>), | ||
| ScheduleRender, | ||
| ScheduleRender(bool), | ||
| AdaptiveSyncAvailable(SyncSender<Result<VrrSupport>>), | ||
| UseAdaptiveSync(AdaptiveSync), | ||
| AllowFrameFlags(bool, FrameFlags), | ||
|
|
@@ -386,9 +402,11 @@ impl Surface { | |
| let _ = self.thread_command.send(ThreadCommand::VBlank(metadata)); | ||
| } | ||
|
|
||
| pub fn schedule_render(&self) { | ||
| pub fn schedule_render(&self, is_fullscreen: bool) { | ||
| if self.dpms { | ||
| let _ = self.thread_command.send(ThreadCommand::ScheduleRender); | ||
| let _ = self | ||
| .thread_command | ||
| .send(ThreadCommand::ScheduleRender(is_fullscreen)); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -455,7 +473,7 @@ impl Surface { | |
| if self.dpms != on { | ||
| self.dpms = on; | ||
| if on { | ||
| self.schedule_render(); | ||
| self.schedule_render(false); | ||
| } else { | ||
| let _ = self.thread_command.send(ThreadCommand::DpmsOff); | ||
| } | ||
|
|
@@ -550,6 +568,10 @@ fn surface_thread( | |
| shell, | ||
| loop_handle: event_loop.handle(), | ||
| clock: Clock::new(), | ||
|
|
||
| min_vrr: None, | ||
| min_vrr_frame_time: None, | ||
|
|
||
| #[cfg(feature = "debug")] | ||
| egui, | ||
|
|
||
|
|
@@ -587,12 +609,12 @@ fn surface_thread( | |
| Event::Msg(ThreadCommand::VBlank(metadata)) => { | ||
| state.on_vblank(metadata); | ||
| } | ||
| Event::Msg(ThreadCommand::ScheduleRender) => { | ||
| Event::Msg(ThreadCommand::ScheduleRender(is_fullscreen)) => { | ||
| if !startup_done.load(Ordering::SeqCst) { | ||
| return; | ||
| } | ||
|
|
||
| state.queue_redraw(false); | ||
| state.queue_redraw(false, is_fullscreen); | ||
| } | ||
| Event::Msg(ThreadCommand::UpdateMirroring(mirroring_output)) => { | ||
| state.update_mirroring(mirroring_output); | ||
|
|
@@ -700,18 +722,21 @@ impl SurfaceThreadState { | |
| .flatten(), | ||
| ) | ||
| }); | ||
| self.min_vrr = min_hz; | ||
| let interval = | ||
| Duration::from_secs_f64(1_000. / drm_helpers::calculate_refresh_rate(mode) as f64); | ||
| self.timings.set_refresh_interval(Some(interval)); | ||
|
|
||
| const SAFETY_MARGIN: u32 = 2; // Magic two frames margin taken from kwin to not trigger low-framerate-compensation | ||
| let min_min_refresh_interval = Duration::from_secs_f64(1. / 30.); // 30Hz | ||
| self.timings.set_min_refresh_interval(Some( | ||
| self.min_vrr_frame_time = Some( | ||
| min_hz | ||
| .map(|min| Duration::from_secs_f64(1. / (min + SAFETY_MARGIN) as f64)) | ||
| .unwrap_or(min_min_refresh_interval) // alternatively use 30Hz | ||
| .max(min_min_refresh_interval), | ||
| )); | ||
| .min(min_min_refresh_interval), | ||
| ); | ||
| self.timings | ||
| .set_min_refresh_interval(self.min_vrr_frame_time); | ||
|
|
||
| if crate::utils::env::bool_var("COSMIC_DISABLE_DIRECT_SCANOUT").unwrap_or(false) { | ||
| self.frame_flags.remove(FrameFlags::ALLOW_SCANOUT); | ||
|
|
@@ -872,10 +897,13 @@ impl SurfaceThreadState { | |
| } | ||
| } | ||
|
|
||
| let redraw_needed = match mem::replace(&mut self.state, QueueState::Idle) { | ||
| let (redraw_needed, is_fullscreen) = match mem::replace(&mut self.state, QueueState::Idle) { | ||
| QueueState::Idle => unreachable!(), | ||
| QueueState::Queued(_) => unreachable!(), | ||
| QueueState::WaitingForVBlank { redraw_needed } => redraw_needed, | ||
| QueueState::WaitingForVBlank { | ||
| redraw_needed, | ||
| fullscreen_request, | ||
| } => (redraw_needed, fullscreen_request), | ||
| QueueState::WaitingForEstimatedVBlank(_) => unreachable!(), | ||
| QueueState::WaitingForEstimatedVBlankAndQueued { .. } => unreachable!(), | ||
| }; | ||
|
|
@@ -886,7 +914,7 @@ impl SurfaceThreadState { | |
| .non_continuous_frame(self.vblank_frame_name); | ||
| self.vblank_frame = Some(vblank_frame); | ||
|
|
||
| self.queue_redraw(false); | ||
| self.queue_redraw(false, is_fullscreen); | ||
| } | ||
| self.send_frame_callbacks(); | ||
| } | ||
|
|
@@ -908,21 +936,44 @@ impl SurfaceThreadState { | |
| self.frame_callback_seq = self.frame_callback_seq.wrapping_add(1); | ||
|
|
||
| if force || self.shell.read().animations_going() { | ||
| self.queue_redraw(false); | ||
| self.queue_redraw(false, false); | ||
| } | ||
| self.send_frame_callbacks(); | ||
| } | ||
|
|
||
| fn queue_redraw(&mut self, force: bool) { | ||
| fn queue_redraw(&mut self, mut force: bool, is_fullscreen: bool) { | ||
| let Some(_compositor) = self.compositor.as_mut() else { | ||
| return; | ||
| }; | ||
|
|
||
| if let QueueState::WaitingForVBlank { .. } = &self.state { | ||
| let is_fullscreen_skip_other = *FULLSCREEN_SKIP_OTHER_SURFACE | ||
| && (self.timings.vrr() || *FULLSCREEN_SKIP_OTHER_SURFACE_ALWAYS) | ||
| && self.output.is_foreground_fullscreen_occupied().is_some() | ||
| && !force | ||
| && !is_fullscreen; | ||
|
|
||
| if *FULLSCREEN_SKIP_OTHER_SURFACE && is_fullscreen { | ||
| force = true; | ||
| } | ||
|
|
||
| let immediate = if *FULLSCREEN_IMMEDIATE_RENDER && self.timings.vrr() && is_fullscreen { | ||
| force = true; | ||
| true | ||
| } else { | ||
| false | ||
| }; | ||
|
|
||
| if let QueueState::WaitingForVBlank { | ||
| fullscreen_request, .. | ||
| } = &self.state | ||
| { | ||
| // We're waiting for VBlank, request a redraw afterwards. | ||
| self.state = QueueState::WaitingForVBlank { | ||
| redraw_needed: true, | ||
| }; | ||
| if !fullscreen_request { | ||
| self.state = QueueState::WaitingForVBlank { | ||
| redraw_needed: true, | ||
| fullscreen_request: is_fullscreen, | ||
| }; | ||
| } | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -939,7 +990,15 @@ impl SurfaceThreadState { | |
| } | ||
|
|
||
| let estimated_presentation = self.timings.next_presentation_time(&self.clock); | ||
| let render_start = self.timings.next_render_time(&self.clock); | ||
| let render_start = if is_fullscreen_skip_other { | ||
| // To prevent the fullscreen surface from unexpectedly stopping updates, register a fallback redraw request. | ||
| // If the fullscreen surface commits an update within the min_vrr interval, it will replace this fallback request. | ||
| self.min_vrr_frame_time.unwrap_or(_30_HZ) | ||
| } else if immediate { | ||
| Duration::ZERO | ||
| } else { | ||
| self.timings.next_render_time(&self.clock) | ||
| }; | ||
|
|
||
| let timer = if render_start.is_zero() { | ||
| trace!("Running late for frame."); | ||
|
|
@@ -955,7 +1014,7 @@ impl SurfaceThreadState { | |
| if let Err(err) = state.redraw(estimated_presentation) { | ||
| let name = state.output.name(); | ||
| warn!(?name, "Failed to submit rendering: {:?}", err); | ||
| state.queue_redraw(true); | ||
| state.queue_redraw(true, false); | ||
| } | ||
| TimeoutAction::Drop | ||
| }) | ||
|
|
@@ -1027,24 +1086,17 @@ impl SurfaceThreadState { | |
| let shell = self.shell.read(); | ||
| let animations_going = shell.animations_going(); | ||
| let output = self.mirroring.as_ref().unwrap_or(&self.output); | ||
| if let Some((_, workspace)) = shell.workspaces.active(output) { | ||
| let seat = shell.seats.last_active(); | ||
| if let Some(fullscreen_surface) = workspace.get_fullscreen(seat) { | ||
| const _30_FPS: Duration = Duration::from_nanos(1_000_000_000 / 30); | ||
| ( | ||
| true, | ||
| fullscreen_surface | ||
| .surface | ||
| .wl_surface() | ||
| .is_some_and(|surface| { | ||
| recursive_frame_time_estimation(&self.clock, &surface) | ||
| .is_some_and(|dur| dur <= _30_FPS) | ||
| }), | ||
| animations_going, | ||
| ) | ||
| } else { | ||
| (false, false, animations_going) | ||
| } | ||
| if let Some(fullscreen_surface) = output.is_foreground_fullscreen_occupied() | ||
| && fullscreen_surface.alive() | ||
| { | ||
| let min_vrr_frame_time = self | ||
| .min_vrr_frame_time | ||
| .unwrap_or(Duration::from_nanos(1_000_000_000 / 30)); | ||
| let drives_refresh_rate = fullscreen_surface.wl_surface().is_some_and(|surface| { | ||
| recursive_frame_time_estimation(&self.clock, &surface) | ||
| .is_some_and(|dur| dur <= min_vrr_frame_time) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I am not completely sure we want to use the min-frame-time here though. This check just serves to figure out, if the application roughly gives us a constantly updating screen, so that we don't turn on VRR e.g. for a fullscreen browser. If we enforce the check to be not lower than the minimum refresh rate, we might disable VRR if the game dips below the e.g. 48 hz for a second before recovering. Do you still see flickering, if this is not using min_vrr_frame_time?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on my understanding, temporarily disabling VRR is not a desirable solution. My OLED display suffers from noticeable brightness flicker when there are large fluctuations in refresh rate, and as far as I know, this is an inherent hardware limitation of current OLED panels. The brightness flickering issue is quite complex. While working on the I can consistently reproduce these issues in
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would this target rate be implemented? On my hardware er-patcher frame limiting for Elden Ring and dxvk-low-latency for DXVK helps quite a bit in terms of flickering.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My implementation requires using it together with fifo-v1. I did not introduce any additional latency in between, and theoretically, providing refresh rate control at the compositor level should achieve better results. At least in my tests, it performs better than MangoHud. My implementation is based on the existing infrastructure of cosmic-comp, with adjustments made to the prediction method. It calculates the time from rendering to vblank present for previous frames and uses that as statistical data to accurately estimate how long before the present time the compositor should start drawing and submitting the frame. In my solution, the evaluation is based on the target refresh rate configured by the user. For example, if the game is actually rendering at 120Hz (8.3ms) but the user requests 60Hz (16.6ms), the compositor will delay starting the rendering and submission of the current frame by 8.3ms. This effectively makes the actual refresh rate become 60Hz. additionally, through fifo-v1, the game will also wait for the compositor to signal that it is ready before submitting the next frame. If the prediction detects that the frame display time is longer than the target 60Hz (16.6ms) interval, the compositor will immediately render without waiting. In this case, the GPU can be utilized to the maximum extent, and there is no additional latency introduced. The main goal is to eliminate excessive chasing of dynamically changing frame rates in VRR scenarios. This can provide a smoother gaming experience while significantly reducing the occurrence of flickering. At the same time, It also provides a convenient way to adjust the target refresh rate at any time according to the requirements of the game scene. Users can fine-tune the desired balance between frame stability and smoothness depending on the game itself or even different situations within the same game.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my understanding, the approaches you mentioned, such as DXVK/VKD3D/MangoHud-level frame limiting, all intervene in the swap chain to achieve the goal. However, these methods cannot truly know when the compositor will present the frame to the display; they can only make indirect estimations. This introduces latency and instability, although they are still effective solutions. gamescope is somewhat different. It has a dedicated vblank manager to strictly control the presentation rate of frames. From an implementation perspective, it is closer to my approach. However, gamescope is based on a time-driven frame update model, while my current implementation still relies on the surface submission-driven presentation model provided by cosmic-comp. Because of this, gamescope can basically prevent refresh-rate spikes completely, which helps avoid flickering issues on OLED displays. The trade-off is that when the display refresh rate and the game's submission rate are not aligned, additional latency can be introduced. My approach, on the other hand, schedules rendering and presentation immediately after the game submits an update. Therefore, in theory, it can provide lower latency. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested your vrr implementation on this monitor on AC Black Flag Resynced and it definitely reduced the flickers a lot. It was a very smooth experience. Much smoother than on GNOME for sure, haven't tried KDE. I used it with Mangohud's frame limit set to 118 fps and early. For the smoothest experience what do you suggest? The game was hovering from 90-100fps and my monitor's refresh rate is set to 120hz. Thank you
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize you are using an Arch-based os; I have a similar AUR package too—haha:
I’ve included a similar fix for the VRR configuration, but your approach might be better—please keep up the good work.
Setting the VRR Target Rate to the maximum value means displaying frames as quickly as possible; this setting aims to minimize input lag. However, frame timing performance is likely to be poor, as GPU utilization is maxed out, leaving no headroom to maintain stable frame timings. If the game's frame rate range is 90–100, I personally prefer setting VRR Target Rate to 80–90; this results in more stable frame timing, especially in 3D games with significant scene changes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also like to clarify that this PR only covers VRR management for fullscreen mode and an experimental immediate rendering feature for fullscreen mode. It does not include the VRR Target Rate functionality. The Git branch specified in the script I provided includes everything from this PR, but also adds support for don't forget to run command: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @skygrango I found leaving VRR Target Rate at 118 to be more smoother, using the game's fps limiter is also looking smoother. Overall the patch from your other branch has helped a lot with the flicker and overall smoothness. Thank you for your work!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AdityaHebballe Thank you very much for helping with the testing. I'm glad that it is helpful. I believe that because everyone has different hardware performance, displays, game content, and sensitivity to VRR behavior, the ideal VRR strategy may vary from person to person. Therefore, I took inspiration from gamescope's approach of allowing users to specify the VRR rate, so that players can customize their gaming experience. If you have any further feedback or testing results, please move the discussion to VRR Target Rate Report. This will help keep this PR focused on technical discussions. |
||
| }); | ||
| (true, drives_refresh_rate, animations_going) | ||
| } else { | ||
| (false, false, animations_going) | ||
| } | ||
|
|
@@ -1340,6 +1392,7 @@ impl SurfaceThreadState { | |
| if x.is_ok() { | ||
| let new_state = QueueState::WaitingForVBlank { | ||
| redraw_needed: false, | ||
| fullscreen_request: false, | ||
| }; | ||
| match mem::replace(&mut self.state, new_state) { | ||
| QueueState::Idle => unreachable!(), | ||
|
|
||

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.
This doesn't seem correct. This would cause the 30hz duration to be used, even if the display reports e.g.
40Hzas a minimum, triggering low-framerate compensation.The current code ensures we never drop below 30hz, which is by design.
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.
my monitor min vrr rate is 48, so it should be 1. / 48.,1/48 is smaller than 1/30, which does indeed fix my problem. theoretically, we should try to stay within the VRR range, right?
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.
oh right, this is a duration. 🤦♀️ yeah makes sense. higher hz values would have smaller frame times. thanks