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

wayland: Check for frog-fifo-v1 to enable Wayland by default #10937

Closed
wants to merge 1 commit into from

Conversation

Kontrabant
Copy link
Contributor

Enable Wayland by default if frog-fifo-v1 is supported.

See #9383

Copy link
Contributor

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, this should probably land once the KWin MR lands.

@mahkoh
Copy link

mahkoh commented Oct 1, 2024

This change doesn't make sense to me. The original detection logic is already questionable and this is on another level.

Let me first explain the problem with the original detection logic and please correct me if I'm missing a critical insight:

@misyltoad's original objection to wayland-by-default-in-SDL3 was that the wayland WSI implemented in graphics drivers is fundamentally broken in fifo mode and cannot be fixed without additional protocols. Defaulting to wayland would therefore give users a worse experience than defaulting to X11.

Let's take that as a given.

wp-fifo and wp-commit-timing allow graphics drivers to fix this. If these protocols are available, graphics drivers can implement fifo in a way that works as well or better than X11.

However, the logic in SDL that looks for these protocols misses the point. Graphics drivers CAN implement a better fifo but they don't have to. Wayland compositors and graphics drivers are not released in lockstep. Even if the compositor supports these protocols, the graphics driver might not make use of them yet or might have no plans to ever use them. In that case, the mere presence of these protocols changes nothing for users.

Therefore, in such situations, SDL will once again give users a bad experience by defaulting to Wayland. If that experience is unacceptable, then the detection logic is also unacceptable.

This is not a theoretical concern. The Steam flatpak upgrades its base package once per year as far as I know. This means that SDL games running inside the Steam flatpak will use graphics drivers that are up to 1 year old. Even if compositors and graphics drivers were released in lockstep, that would have no impact on Steam flatpak users.

The only place where this does make sense is on the Steam Deck where valve ships both gamescope and mesa in a coordinated fashion and where valve is free to apply patches to the mesa WSI.

Therefore I would say that SDL should do one of the following things:

  • If the current situation isn't so bad after all and subjecting users to it (as for example described above in the Steam flatpak situation) is acceptable, then SDL should once again unconditionally default to wayland.
  • If the situation is unacceptable and some detection is desired, then SDL should not only look for these protocols but also check the graphics driver versions and only enable wayland if the graphics driver version is known to include support for the protocols.
  • If the situation is unacceptable and looking at the graphics driver version is also undesirable, then SDL should never default to wayland.

As for this PR in particular: No graphics driver has even indicated that they are going to support this protocol. Compositors can advertise it all they want but it changes nothing.

Copy link
Collaborator

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine to merge once it lands in Mesa - I'm not really worried about compositors exposing this as long as Mesa has it; the NV proprietary driver might fall behind a bit but that's nothing new (and for Turing+ we have NVK anyway).

@mahkoh
Copy link

mahkoh commented Oct 1, 2024

I'm not really worried about compositors exposing this as long as Mesa has it

This PR does exactly the opposite. It's only concerned about compositor exposing it and ignores whether Mesa has it.

@flibitijibibo
Copy link
Collaborator

I'm not really worried about compositors exposing this as long as Mesa has it

This PR does exactly the opposite. It's only concerned about compositor exposing it and ignores whether Mesa has it.

I would expect that a system with an updated compositor will also have an updated Mesa driver - in fact I would almost expect compositors to also hold off on exposing this until Mesa/NV exposed it first. Flathub's drivers may(?) complicate things but I'd have to look at it more thoroughly. If you want you can patch in the driver check but I dunno how that would work without knowing in advance which Vulkan/EGL device is getting used.

@mahkoh
Copy link

mahkoh commented Oct 1, 2024

I would expect that a system with an updated compositor will also have an updated Mesa driver - in fact I would almost expect compositors to also hold off on exposing this until Mesa/NV exposed it first.

Impossible since compositors cannot know which graphics drivers clients are using. Graphics drivers used by clients are completely independent from graphics drivers used by compositors. And even if they were not, compositors would have no idea what protocols the graphics drivers are using except by doing the same kind of version checks.

Flathub's drivers may(?) complicate things but I'd have to look at it more thoroughly.

The steam flatpak was last updated 2 weeks ago. The last update before that was a year before. Little chance of it getting a new mesa version in less than 12 months if this trend holds: flathub/com.valvesoftware.Steam@45ff9df

@Conan-Kudo
Copy link
Contributor

I would expect that a system with an updated compositor will also have an updated Mesa driver - in fact I would almost expect compositors to also hold off on exposing this until Mesa/NV exposed it first.

Impossible since compositors cannot know which graphics drivers clients are using. Graphics drivers used by clients are completely independent from graphics drivers used by compositors. And even if they were not, compositors would have no idea what protocols the graphics drivers are using except by doing the same kind of version checks.

What are you talking about? Compositors are in control of what clients get attached to in the first place. They absolutely do know what graphics drivers clients are using.

@mahkoh
Copy link

mahkoh commented Oct 1, 2024

Compositors are in control of what clients get attached to in the first place. They absolutely do know what graphics drivers clients are using.

That makes no sense at all. But please show me code or an API that would allow the compositor to know which graphics driver a client is using.

@eszlari
Copy link

eszlari commented Oct 10, 2024

The steam flatpak was last updated 2 weeks ago. The last update before that was a year before. Little chance of it getting a new mesa version in less than 12 months if this trend holds: flathub/com.valvesoftware.Steam@45ff9df

Flatpak runtimes are updated all the time. The freedesktop 23.08 branch has Mesa 24.2.3 right now.

@mahkoh
Copy link

mahkoh commented Oct 10, 2024

You're right. I was going off the fact that the X11 WSI in steam flatpak does not use explicit sync which the X11 WSI used by steam not-flatpak does. But there can be many other reasons for that: They might not be shipping an up-to-date libdrm or libxcb or many of the other dependencies of mesa or they might be building mesa incorrectly.

@Kontrabant
Copy link
Contributor Author

The upstream FIFO and commit timing protocols were merged, so this doesn't seem necessary anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants