But it should redraw the frame at max_render_time irrespective if the cursor moved or not. […] it stutters because it’s not redrawing / rendering at the minimum framerate anymore.

It sounds like you think max_render_time does something different than what it does? It doesn’t configure the time to wait between renders. It configures how late before a monitor refresh to defer rendering when a frame is needed. If you set it to 1 ms sway will try to render the next frame 1 ms before the monitor refreshes, if there’s anything to render.

And sure you could argue that the cursor moving is “something to render”. But if you let the cursor trigger that frame, it will lead to stutter. That’s literally what this issue you’re commenting on is about: Making the cursor not trigger frames. If the cursor moves and it causes sway to render/present its frame, if the game then finishes rendering its frame shortly after that, that game frame can’t be shown until the next smallest monitor refresh interval.

This is what one would want, where every time a game finishes rendering, sway can immediately render it and the monitor can immediately present it. Silky smooth gameplay.

Now say I move the cursor. And I let sway trigger a frame for that. Since this example monitor supports up to 100 Hz, with max_render_time set to 1, sway would try to render it after 9 ms from the previous frame so it can have the frame ready to go at 10 ms. Since that’s the earliest the monitor can refresh. But if it does, you get stutter.

Note the second 17 ms frame now being presented for 20 ms, and the third 17 ms frame being presented for 13 ms (numbers are rounded). The goal is to instead have it do this, while playing a game. Back to silky smooth gameplay.

So not having the cursor trigger a render is the entire point of my patch, as well as the point of this whole discussion. Obviously once the frame rate drops below something acceptable for interactivity it needs to start triggering frames for cursors again and you have to deal with the stutter. But for that, the time to wait with rendering would be better to base on the lowest refresh rate supported rather than the highest. On this example monitor it would then start letting the cursor trigger frames at 40 Hz instead. Or better yet, let the user configure the limit if they want.

the function you probably want to modify is wlr_output_cursor_move(), it calls drm_connector_move_cursor and sets the damage for the software cursor.

The damage is mainly irrelevant. For one, only software cursors cause damage and most people very likely aren’t using software cursors. And secondly, as I already described above, the entire point of this issue/patch is to make cursor moves not trigger frames. The damage caused by software cursors just have to be reapplied before actually rendering.

I was trying to make sway not trigger new frames when moving the mouse a couple of months ago. Turns out it’s not too difficult to achieve if your needs aren’t very high. That said, I imagine most people have more needs than I do, but I haven’t managed to find the time to make it more generally usable. So instead I’m posting the patches here in case someone else wants to finish it and clean up the API a bit (and probably make it an option).

Specifically: With the patches applied, mouse input will never trigger a redraw if sway has a view fullscreened, and adaptive sync is enabled. If adaptive sync is not enabled, or nothing is fullscreened, mouse input triggers redraws as usual. Where that behavior falls a bit short of being “generally usable” is that it probably makes sense to limit it to some minimum refresh rate. As is, if you were to fullscreen a fully static window, you would never see your mouse cursor move. For my use that just happens to not be a problem since I only enable adaptive sync when I launch a game, and disable it again after (using scripts).

I’ve been using these patches for the past two months without issues. I also posted an earlier version of them in an AMD driver bug report before, and people replied to say it’s working for them too. (Though please don’t comment about the patches in that report, it just adds noise.)

Two things worth noting:

• These attached patches also have calls to wlr_output_lock_software_cursors. I only added those to work around that AMD driver bug. They’re not necessary, and may not even be desirable as they prevent direct scanout. For my particular use case, preventing direct scanout was just the lesser of two evils.
• The addition in handle_damage in output.c assumes that the function only gets called for cursors. As far as I could tell, that was true at the time – and may still be true – but it’s probably a bad assumption to make.

Github won’t let me attach the patch files for some reason, so I posted them to a gist instead: https://gist.github.com/rhoot/dcead8b195054387c067919fef499c5c

@rhoot I was just saying that because the implementation started out wrong, you should have approached it from a different angle (as per my original comment saying you shouldn’t turn it on and off in timer_handler).

I’ll leave it be now that you inspired me to write my own patch that does it the correct way™

@EVERYONE: VRR fix

This patch is for v1.9, so feel free to test it.

This supports any focused window! and allows you to set a max latency like so:

$ swaymsg '[title="Factorio"] max_cursor_latency 8333'

The value is in microseconds, this will force Factorio (a 60fps locked game) to run at 120fps when moving the cursor.

Right now there no way to read the EDID info for lowest support Hz so I’ve just hard coded 30fps as the limit for now.

https://github.com/YellowOnion/sway/commit/deferred-cursor-move

I do not see the AMD driver bug issue, but maybe I’m blind or just lucky with my card of choice. But I do see an issue with Scan out driving monitor at max fps, so might need to run with -Dnoscanout

I’ve opened up a MR on wlroots, https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4340

If I understand what you linked correctly, displaying hardware cursors with VRR is still being worked on and the behaviour is not ideal, as it currently always prioritises the cursor. This is an issue, but that should only happen when the hardware cursor is visible right, or am I misunderstanding something?

From this issue:

From what I understand, this behavior would be desired if a hardware cursor was displayed. However, it also occurs when there is no hardware cursor.

Doesn’t that mean that this is a seperate issue or a bug with sway? I also found this comment, which seems to be in line with what I think.

@GrabbenD I think you need to write a script to do that. swaymsg -m -t SUBSCRIBE '["window"]

@YellowOnion What the formula to calculate 8333 for a 120Hz display?

It’s not dependant on your display, you can pick any number you want if you want the cursor to update at a specific rate:

1000000/Hz

$ echo $((1000000 / 120))
8333

Looks like you rebased the latest version cool 😃, neither wayland, wlroots, or sway have access to the edid VVR info (like minimum, or horizontal khz), which could help with better timing, I noticed some of the spikes myself (worst being 48hz app, driving monitor at 95hz lol), I did try to improve the accuracy after noticing a bug/oversight, but then replaced it with some other code that might make it worse off.

But yeah the current code is designed to deliver cursor updates in a timely manner, not a smooth manner.

For the sway user control, I was just trying to get some minimal implementation working, tbh I don’t like integer fps differences and how they lose precision the lower the fps is, lots of monitors are not actually 60fps (it depends on horizontal frequency, resolution, and the vblank time), especially with VRR on (I still need to workout the math for my monitor to confirm this), and all feature film is not 24fps (they’re officially defined as 24000/1001), TV also has a similar denominator, using time/latency has better precision for low fps scenarios, so I’m thinking about how to get better accuracy before just doing a the naive _fps parameter.

I have a few ideas that could be combined.

• _fps parameter obviously should be included for a quick and easy option.
• could include a _denominator with a default parameter to 1 that can be changed for more accuracy.
• could explicitly round _fps internally to the nearest integer division of the monitor, or something timed accurately with the edid VVR info.
• dito but rounded to the FPS of the video you’re watching (no clue how to get accurate timing from the video client tho, estimator could work).
• implement a smoothing variant that could be included for watching video (I’m not sure how to do this and make it objectively better than the current implementation).

It’s also worth mention that I’m pretty sure Sway and wlroots don’t make any attempts to smooth updates in general either, from the code I read, which might explain why there’s reports of flicking in sway, if you have a windowed app open, and swaybar or a chat client updates for example, both force the display to update. This patch only changes the behavior of cursor redraws so it’s a little outside the scope of cursor management.

max_render_time also has a heavy impact on timing, it’s currently using the max hz of display, and a low-res wayland timer, I was surprised that I could pull an extra 20% FPS setting it to 6ms, instead of “off”, which has me curious about how the entire graphics pipe interacts here.

I updated your commits to work on latest master of sway/wlroots: https://gist.github.com/rhoot/5d47b0aabe96f6d44659d2f40329860f

Seems to be working pretty well, once configured for a view. I tried running gamescope -r 60 vkcube with this and while my monitor did jump up to 65 Hz once or twice, that could also have been due to other factors. It only seemed to happen shortly after fullscreening it and then go back down to 60 and stay there. I’ll try running this for a while. 👍

That said, I think it’d be nicer to make the config be specified in terms of updates per second instead of microseconds per frame. So something like [title="Factorio"] min_cursor_fps 120. I think max_cursor_latency makes sense in the API, but in the user facing config thinking in terms of FPS is just way simpler.

Don’t get me wrong: I never said there’s not a solution. I merely said I haven’t added one in my patches. The fact that it’s not “generally usable” is why it’s not a PR. I’m still waiting for the AMD bug on this to be fixed before I may have the motivation to try to do it properly if no one does it earlier.

Why not limit it to the highest multiple of your current frame rate with respect to the display’s refresh rate? For example, we could have the cursor update 50 times a second if the game is running at 25 fps on a 60hz monitor

Game frame time (more important than FPS for presenting) fluctuates. Even if you’re going at a rock solid 25 FPS, it will fluctuate by some 10-100 µs in one direction or the other. Add frame spikes and varying GPU loads between frames and you have a range of frame times that the game renders in as opposed to just one.

In order to apply your suggestion, sway would have to predict how long it will be from the previous frame before the game renders the next. If it mispredicts and the next frame is ready way sooner than sway had expected, it could cause the game frame to be delayed as sway had just triggered a refresh to update the cursor position, and refreshes can’t happen immediately after they have already happened. And the result would be that the game stutters for a frame, since the game had already assumed the time between the previous frame and this new frame.

Personally I would much rather have a dumb but reliable solution than one that tries to be smart. It will only have an effect when the game runs at a really low frame rate anyway, which (hopefully) is not typical. Basically just using the minimum rate supported by the monitor, or some user configurable value with some sensible default if that’s too low (e.g my minimum is 1 Hz). KWin’s PR on this appears to be leaning in the same direction, with using the maximum of 40 or the monitor’s lowest VRR rate.

Edit: I guess I should at least acknowledge games that intentionally limit rendering to something low, like 30 FPS. But those games typically don’t use a mouse since it would be a horrible experience, so cursor updates don’t apply to those games.

Sway already has max_render_time variable. I suspect this patch is going about limiting the redraw rate the wrong way as it effectively breaks the point of the timer.

Did you look at the code in the patch? It doesn’t affect the timer at all. All it does is:

1. Make wlroots not request a new frame to be rendered when the cursor moves.
2. When software cursors are enabled, ignore the damage they cause when determining if it’s time to render, instead relying solely on client damage. Cursor damage gets reapplied just before rendering instead.
3. Toggle it on/off depending on if adaptive sync is enabled, and if something is fullscreened.

It doesn’t limit the redraw rate at all.

In case anyone is actually using my patches, I’ve updated the ones in the gist. Changes from before:

• It no longer switches to software cursors to work around that amdgpu bug. Instead I’ve switched over to using the legacy DRM API (WLR_DRM_NO_ATOMIC=1). Previously doing so caused it to trigger redraws again when moving the mouse, but it seems that’s no longer the case.
• I realized if you moved the mouse right after a frame had been drawn while using software cursors, it would end up adding damage to the output’s damage ring, causing the next frame to immediately be requested, and repeated until you stop moving the cursor. Immediately noticeable with a 1 ms poll rate. Instead I made it handle software cursor damage separately from other damage, and not add it to the damage ring until it’s time to actually render.
• The wlroots patch was just updated to work with more recent mainline commits.