Don't use explicit sync fences if we can tear #8645
Conversation
This fixes the issue where with explicit sync on no tearing is possible even when the requirements for it are fully met. This is similar to how KWIN handles it.
| @@ -1531,7 +1531,7 @@ bool CHyprRenderer::commitPendingAndDoExplicitSync(PHLMONITOR pMonitor) { | |||
| if (inFD >= 0) | |||
| pMonitor->output->state->setExplicitInFence(inFD); | |||
| auto explicitOptions = getExplicitSyncSettings(); | |||
| if (explicitOptions.explicitEnabled && explicitOptions.explicitKMSEnabled) | |||
| if (explicitOptions.explicitEnabled && explicitOptions.explicitKMSEnabled && !pMonitor->tearingState.canTear) | |||
There was a problem hiding this comment.
this should be activelyTearing not canTear.
There was a problem hiding this comment.
I tried that first and it didn't work
There was a problem hiding this comment.
canTear is definitely wrong, but I don't thing we even use the out fence, can we drop it?
|
generally yes this looks correct apart from the mistaken variable. Out fence doesn't matter while tearing, though we still have to give back feedback. Considering it's working I assume it's done somewhere else and works. |
|
yeah looks like it. we release when backend releases regardless |
|
|
|
What does
https://drmdb.emersion.fr/properties/3435973836/OUT_FENCE_PTR Seems reasonable not to use it with tearing. kwin doesn't use |
|
we could disable IN_FENCE, but I don't think that's a good idea: explicit sync + scanout without IN_FENCE will mess up. |
|
Is it possible to get DS with tearing? Hyprland/src/render/Renderer.cpp Line 1315 in f9e4998 |
|
hm, in our code, maybe not, but ideally yea it should be perfectly possible |
|
patch.txt |
|
Doesn't that patch undo 967fe76? |
|
yes. @UjinT34 what was the point? I'm running the patch and it seems fine. IDK about nvidia. |
|
the cursor does look choppy when tearing, but oh well. |
|
With explicit sync enabled HL expects to receive the fence from AQ and complains I don't know how this fence should be used. Looks like it should be waited either before committing the next frame or before reusing the current framebuffer. Waiting for it immediately after the commit feels too early. Not using it at all is probably the same as waiting for it before reusing the fb because it is very unlikely to block at that point. Maybe there is no point in this fence with double buffering. |
|
yeah I don't think there is. |
|
We can go with my patch, do you want me to commit it to main or will you apply it here and we roll with this MR? |
I think Just stick yer patch in main and I'll close this |
Describe your PR, what does it fix/add?
Fixes #8482
This fixes the issue where with explicit sync on no tearing is possible even when the requirements for it are fully met.
This is similar to how KWIN handles it.
Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)
I am not 100% sure this is ultimately the correct fix but it retains fences and allows tearing to happen when requested. It also seems to be what other compositors are doing too.
Also been running this patch on my distro with a good number of users with no issues reported for around 3 weeks.
Is it ready for merging, or does it need work?
Yea do it.