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

Experimental native Wayland Support #96

Closed
wants to merge 3 commits into from

Conversation

vchernin
Copy link
Contributor

@vchernin vchernin commented Jul 21, 2021

#85 is an older version, ignore that one.

Adds native Wayland support thanks to the work that's gathered here. Nothing is changed by default, a user specifically has to opt in by adding a JVM flag in the launcher. Should work in 1.13+, including 1.17 (see picture).

This is done so the GLFW patches can be tested. The goal is not to have to ship a patched GLFW, but to help get them upstreamed. If possible, please test out this build following these instructions and report issues where appropriate.

Fixes #84

Minecraft in Native Wayland

To use:

  1. git clone my fork and then flatpak-builder --user --install build-dir com.mojang.Minecraft.json
  2. Within the Minecraft launcher’s "Installations" menu, hover over an installation and click the 3 dots. Click edit, and then open advanced options near the bottom. Append -Dorg.lwjgl.glfw.libname=/app/lib/libglfw.so to your JVM arguments. Ensure to leave a space in between arguments.
  3. Click play!

Issues:

  • No proper decorations on GNOME Wayland.
  • I have been unable to include the patch for libdecoration. It appears the patch itself is broken, but I am trying to confirm that.
  • Until then GNOME users will likely not have a great experience, since Minecraft does not have CSD support without the patch.
  • Even when this patch is merged we still won't have gtk-style window decorations, to do so the GLFW patch needs to be updated to use this libdecoration change.
  • Weird rectangle that shows up on my HIDPI monitor in GNOME Wayland.
  • Only occurs when libdecor is not included.
  • On my HIDPI setup, Minecraft seems to incorrectly always pick my smaller and lower resolution monitor when clicking fullscreen within the game settings. The only way to override this behaviour is to disable the lower resolution monitor.
  • Sometimes the cursor gets displaced when full screening. If this happens you can try unfullscreening by using tab and arrow keys to navigate controls.
  • There is a general blur to their entire window when libdecor is there. Compare them using this diff, click "Swipe" at the bottom. The image on the right is with the libdecor patch, the one on the left is without. You might need to zoom in if it's not obvious.

Unimportant (since long-term we shouldn't want any custom glfw):

  • The only way I know to get the JVM to use the patched GLFW is with the instructions above. I don't know how to set JVM flags within the Flatpak config. For now we shouldn't anyhow, but it is something I'd like to figure out.
  • The flag appended requires manually setting your user, $USER doesn't work. This should be improved somehow.
  • The way I give Minecraft permission in df5340f to read the patched library is not ideal. It took me a while to figure out what exactly I needed to give it. I feel like this is a bug, since shouldn't the Flatpak always have permission to read it's own directories?
  • The .local directory is given in case it's installed with for a user, the other is in case it's installed system-wide.

Patch Info:

The Flatpak uses the patches gathered here (libdecoration branch), which were originally from the following GLFW changes:

Patch Original PR/Commit Purpose
0001 PR Add libdecoration support to get CSD on GNOME
0002 PR Fixes freeze caused by race condition
0003 Commit Don't crash on window focus or icons
0004 Commit Fixes broken Mutter (GNOME) OpenGL screenshots

Todo:

  • Debug issues and report them upstream.

vchernin added 2 commits July 21, 2021 16:41
This should not be necessary, but seems to be for the patched glfw to run.
I will try and see if this is a Flatpak bug or I'm just not understanding Flatpak directory permissions correctly.
Adds necessary dependencies as well as the patches themselves.
Note the patched glfw is only used with an opt-in JVM argument.
@vchernin vchernin mentioned this pull request Jul 21, 2021
7 tasks
@flathubbot
Copy link
Contributor

Started test build 54019

@flathubbot
Copy link
Contributor

Build 54019 failed

@AsciiWolf
Copy link
Collaborator

bot, build

@flathubbot
Copy link
Contributor

Queued test build for com.mojang.Minecraft.

@flathubbot
Copy link
Contributor

Started test build 54045

@flathubbot
Copy link
Contributor

Build 54045 failed

"sources": [
{
"type": "archive",
"url": "https://gitlab.gnome.org/jadahl/libdecoration/-/archive/master/libdecoration-master.tar.gz",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using archive made from latest master is not a good idea. ;-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, it looks like the repository has moved to https://gitlab.gnome.org/jadahl/libdecor - that has a tagged release you could use. ;-)

Copy link
Contributor Author

@vchernin vchernin Jul 28, 2021

Choose a reason for hiding this comment

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

Thanks for pointing that out, this took me a minute to figure out, turns out all I needed to do was update the libdecor patch (yay juggling patches).

I do hope to try and debug the issues on the issue list at some point, because that's probably a much better use of time, especially the last one. Since libdecor got into SDL hopefully the patches get reviewed sooner than later.

@flathubbot
Copy link
Contributor

Started test build 54708

@flathubbot
Copy link
Contributor

Build 54708 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/52654/com.mojang.Minecraft.flatpakref

@AsciiWolf
Copy link
Collaborator

Thanks! I will test this when I have more time. However, I am not sure whether I want this in master since it is an unofficial, unsupported patch that can potentially bring some issues. Also, this is definitely incorrect and should not be in the manifest:

        "--filesystem=~/.local/share/flatpak/app/com.mojang.Minecraft:ro",
        "--filesystem=/var/lib/flatpak/app/com.mojang.Minecraft:ro"

At least in my opinion.

I could make a beta branch and merge it there, but I already have too much work with maintaining this Flatpak (and my other ones) since I am the only active maintainer, so I am not interested in maintaining another branch. :-/

@vchernin
Copy link
Contributor Author

I don't think this should be merged at all, even if I find an actual solution for that file permission trick. It doesn't seem right to put a bunch of patches in here, even if users have to pass a JVM flag to the launcher to get it working.

This PR should only be for the sake of testing and experimentation--I'd like to help get the patches merged properly upstream, and this seems like a nice way of testing it. Therefore a beta branch wouldn't be necessary if there's no intention of merging this.

Thank you for your work on this package, I'd like to help more where I can. I don't think I can currently commit to actively maintaining this unfortunately, but hopefully I can be helpful.

BTW, I noticed today Minecraft is on the approved Flatpaks list for Fedora's "Filtered Flathub" initiative. So users who enable third party repos on Fedora 35 will have this Flatpak in the software center. So you can probably expect more users...

@vchernin
Copy link
Contributor Author

vchernin commented Jun 6, 2022

this never made much progress, this still needs upstream support

@vchernin vchernin closed this Jun 6, 2022
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.

Enable native Wayland support
3 participants