Skip to content
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

[aquamarine] try to fix direct scanout #6875

Merged
merged 10 commits into from
Jul 19, 2024

Conversation

ikalco
Copy link
Contributor

@ikalco ikalco commented Jul 14, 2024

Describe your PR, what does it fix/add?

will try to fix direct scanout and linux-dmabuf protocol for #6608

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Is it ready for merging, or does it need work?

Footnotes

  1. after reading some of weston's code this might be related to plane damaging (drm.c@470)? this is a guess so far tho

  2. https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/main/stable/linux-dmabuf/feedback.rst

    Compositors should not send feedback parameters if they don't have a fallback path. For instance, compositors shouldn't send a format/modifier supported for direct scan-out but not supported by the rendering API for texturing.

    after some more reading the ds tranche is allowed to have formats that the egl doesnt support as long as there is a fallback tranche that the renderer does support

  3. the format_table contains all the formats from egl and planes but some wl clients only use format_table to find formats instead of tranches
    only "bad" client i could find was mpv so I just bug reported there

@ikalco
Copy link
Contributor Author

ikalco commented Jul 15, 2024

Found an interesting repro case for the random artifacts during ds, xonotic
if run with SDL_VIDEODRIVER=x11 xonotic-sdl there are some arifacts
but run with SDL_VIDEODRIVER=wayland xonotic-sdl there is major artifacting, looks related damaging

edit:
for wayland the anti aliasing option almost completely removed artifacting, with vsync option on aswell its gone
for x11 just enabling vsync completely removes artifacting

@ikalco
Copy link
Contributor Author

ikalco commented Jul 16, 2024

another repro case for artifacting
https://github.com/ascent12/compositor-killer.git

edit:
this patch.txt makes it stand out more, and it kind of looks like its something the client is rendering??? idfk

@ikalco
Copy link
Contributor Author

ikalco commented Jul 16, 2024

gamescope has a bug report that looks a lot like the artifacting
ValveSoftware/gamescope#1368

edit:
can repro that lowering frame rate in game (xonotic) to below vsync removed all artifacts

@ikalco ikalco mentioned this pull request Jul 17, 2024
42 tasks
@vaxerski
Copy link
Member

so is that a kernel bug?

@ikalco
Copy link
Contributor Author

ikalco commented Jul 17, 2024

so is that a kernel bug?

not entirely sure, it supposedly works on nvidia and igpu without artifacting but amd gets artifacting
but then again amd doesnt get artifacting on sway so maybe they have a mitigation related to below

edit:
can repro that lowering frame rate in game (xonotic) to below vsync removed all artifacts

not sure how to test how many fps the client is actually rendering to test if this is true though

edit:
the gamescope bug also mentions that completely disabling explicit sync fixed the issue
i tried doing it through config and nothing changed, i might try testing ripping out all the explicit sync suff just to make sure tho

@Agent00Ming
Copy link
Contributor

recorded on this branch, intel iGPU

direct_scanout_tearing.mp4

@ikalco
Copy link
Contributor Author

ikalco commented Jul 18, 2024

After a lot of testing the artifacts dont seem to be a kernel/driver/mesa bug
its definitely related to vsync, the more accurate the fps is to the refresh rate the less the artifacts
I've tested a few games and enabling vsync will make most artifacts go away

PS:
I dont understand enough about how Hyprland maintains fps and rr to be able to fix the artifacting
how is fps and rr maintained by Hyprland? is page flip event called by driver exactly right to line up with refresh rate and therefore renderMonitor?
either way I think something in attemptDirectScanout isn't the same as the rest of renderMonitor in relation to vsync/fps/rr which causes the bug

@vaxerski
Copy link
Member

@phonetic112 in 6608 said the glitches go away on legacy drm api, perhaps something there is off?

how is fps and rr maintained by Hyprland?

render ---> submit with flip event

submit submits a frame to be presented on next vblank (monitor changes image = vblank)
then, when that vblank happens and our image is presented, we get an event (see handlePF in aq) in which (if applicable) we send a .frame event to signify its a good time to render the next frame. If there is no damage, and a frame isnt needed, we dont. This stops the cycle and will resume it the next time something schedules a frame (e.g. damage)

@vaxerski
Copy link
Member

wild guess, do the glitches happen without hardware cursors but when the cursor isnt on the screen?

(if that doesnt allow DS then just patch it out, probably in attemptDirectScanout)

@ikalco
Copy link
Contributor Author

ikalco commented Jul 18, 2024

no disabling hw cursors doesnt fix artifacting

btw to get artifacting to go away i used vrrtest, vsync on, fps cap 1 fps above vsync, and busy wait on (busy wait says it more accurately tracks vsync)
you'll also see that the closer you get to vsync the artifacting will slow down

edit:
legacy drm doesn't seem to fix artifacting for me :(

@ikalco
Copy link
Contributor Author

ikalco commented Jul 18, 2024

@phonetic112 in 6608 said the glitches go away on legacy drm api, perhaps something there is off?

how is fps and rr maintained by Hyprland?

render ---> submit with flip event

submit submits a frame to be presented on next vblank (monitor changes image = vblank) then, when that vblank happens and our image is presented, we get an event (see handlePF in aq) in which (if applicable) we send a .frame event to signify its a good time to render the next frame. If there is no damage, and a frame isnt needed, we dont. This stops the cycle and will resume it the next time something schedules a frame (e.g. damage)

wait, so how do we make sure we dont schedule more than, lets say, 60 fps?

@vaxerski
Copy link
Member

backend will err out if you commit a new buffer before handlePF

with this commit artifacting now only occurs when the
client's fps is above the refresh rate of the monitor
@ikalco ikalco force-pushed the fix_direct_scanout_maybe branch from 043e229 to 9c24ec6 Compare July 19, 2024 01:19
@ikalco
Copy link
Contributor Author

ikalco commented Jul 19, 2024

last commit was able to make artifacting only appear when vrrtest frame cap is above monitor refresh rate except when starting it up when it happens for a few frames

also arch ci seems to be broken, something with updating packages

@ikalco
Copy link
Contributor Author

ikalco commented Jul 19, 2024

the wlroots ds pr mentions holding client buffers longer

We need to hold client buffers for longer than usual. When compositing, we need to hold buffers until the buffer swap (and the kernel will take care of the rest with implicit synchronization). When scanning out, we need to hold buffers as long as they're visible on screen.

didn't have time to check this out but if we arent then that could be causing artifacts

@vaxerski
Copy link
Member

heh I did once make a patch to do that but it didnt stop the glitches, I can do it again

@vaxerski
Copy link
Member

committed the new infra impl here, feel free to drop it if it doesnt work

@ikalco
Copy link
Contributor Author

ikalco commented Jul 19, 2024

committed the new infra impl here, feel free to drop it if it doesnt work

it worked!!! artifacting is now completely gone on AMD, should be gtg other than testing on nvidia and igpu

@ikalco ikalco marked this pull request as ready for review July 19, 2024 13:28
@Agent00Ming
Copy link
Contributor

Agent00Ming commented Jul 19, 2024

works on nvidia but something is going wrong with the cursor hanging the whole desktop nvm, the cursor simply does not cause any damage to happen

@ikalco
Copy link
Contributor Author

ikalco commented Jul 19, 2024

yeah i just saw that, i think i might know that it is

edit:
try now @Agent00Ming

btw if you get a crash related to sendReleaseWithSurface i think i found the bug

@Agent00Ming
Copy link
Contributor

Agent00Ming commented Jul 19, 2024

Yeah, works on nvidia. Occasionally spams entered/left direct scanout in logs when initially starting a fullscreen window or when system is under heavy load when starting the direct scanout but that's a non-issue and stabilizes in a few seconds. Will test intel in a moment.

Unsurprisingly, it also works on intel 👍

@ikalco ikalco force-pushed the fix_direct_scanout_maybe branch from 202fec0 to 1c543b2 Compare July 19, 2024 14:53
@vaxerski
Copy link
Member

ready to merge then? or no

@ikalco
Copy link
Contributor Author

ikalco commented Jul 19, 2024

yes this is ready

@vaxerski
Copy link
Member

thanks!

@vaxerski vaxerski merged commit c3ceacb into hyprwm:aquamarine Jul 19, 2024
9 of 11 checks passed
vaxerski added a commit that referenced this pull request Jul 20, 2024
* make linux-dmabuf use monitor primary plane formats for ds

* add support for monitor connects/disconnects in linux-dmabuf

* the weird bug was because ds was global, and disabled on one monitor but enabled on the other

* forgot to remove attemptDirectScanout from hyprRenderer

* add renderer fallback tranche on ds and a few other improvements

* use set when making format table

* mitigate ds artifacting and update linux-dmabuf comments
with this commit artifacting now only occurs when the
client's fps is above the refresh rate of the monitor

* use new backend release infra

* revert earlier artifacting mitigation that broke stuff ;)

* ... i should test before push ;-;

---------

Co-authored-by: Vaxry <[email protected]>
vaxerski added a commit that referenced this pull request Jul 20, 2024
* make linux-dmabuf use monitor primary plane formats for ds

* add support for monitor connects/disconnects in linux-dmabuf

* the weird bug was because ds was global, and disabled on one monitor but enabled on the other

* forgot to remove attemptDirectScanout from hyprRenderer

* add renderer fallback tranche on ds and a few other improvements

* use set when making format table

* mitigate ds artifacting and update linux-dmabuf comments
with this commit artifacting now only occurs when the
client's fps is above the refresh rate of the monitor

* use new backend release infra

* revert earlier artifacting mitigation that broke stuff ;)

* ... i should test before push ;-;

---------

Co-authored-by: Vaxry <[email protected]>
@ikalco ikalco deleted the fix_direct_scanout_maybe branch July 27, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants