mate-desktop / pluma

A powerful text editor for MATE
http://www.mate-desktop.org
GNU General Public License v2.0
157 stars 65 forks source link

wayland: add guard backend-specific calls by an ifdef #532

Closed mbkma closed 4 years ago

mbkma commented 4 years ago

See https://developer.gnome.org/gdk3/stable/gdk3-Wayland-Interaction.html for details.

I have been playing with pluma (master) and weston (8.0.0-1) in a vm with Ubuntu MATE 20.04 lately. Although I don't know neither pluma nor weston a lot, it seems to work for me.

Test: install weston, launch weston in terminal or logout and go into a proper weston session (for me on debian stable and weston 5.0.0-3 I got a black screen) and compile and launch pluma.

Screenshot at 2020-04-02 16-47-55

raveit65 commented 4 years ago

@wmww Can you please do some testing and a review?

raveit65 commented 4 years ago

@mbkma Can you please rebase against master to get latest travis build environment commits?

Configure will fail if gdk-wayland-3.0 build-requires isn't installed, right? I think for the moment it's better to have it optional, because i am not sure if all distros ships it, especially BSD distros or slackware, etc..

And can you add wayland requires to build.yml please? It looks like that wayland-devel is already in docker images of fedora, archlinux, debian and ubuntu, but it doesn't hurt to add it to build.yml

raveit65 commented 4 years ago

Hmm, i see a critical warning about wayland in your screenshot. (Gdk_is_wayland_window failed)

wmww commented 4 years ago

Code all looks sensible and correct to me. I'll test it on Sway and Mir later and report back.

EDIT: making the Wayland dependency optional is indeed a good idea. The configure.ac section of my porting blog post (which I assume you've already seen) has details on how I did this.

rbuj commented 4 years ago

Test: enable x11 backend

$ ./autogen.sh --prefix=/usr --enable-x11 --disable-wayland && make

Test: enable wayland backend

$ ./autogen.sh --prefix=/usr --disable-x11 --enable-wayland && make

Test: enable x11 & wayland backends

$ ./autogen.sh --prefix=/usr --enable-x11 --enable-wayland && make

patch:

diff --git a/configure.ac b/configure.ac
index 6e9f927..cc05fc2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -167,19 +167,84 @@ PKG_CHECK_MODULES(PLUMA, [
    gtksourceview-3.0 >= $GTKSOURCEVIEW_REQUIRED
    libpeas-1.0 >= 1.2.0
    libpeas-gtk-1.0 >= 1.2.0
-   gdk-wayland-3.0
 ])

-PKG_CHECK_MODULES(X11, [x11])
+dnl **************************************************************************
+dnl X11/Wayland backends
+dnl **************************************************************************
+
+# $enable_x11 and $enable_wayland will be set to "yes", "no" or "auto"
+AC_ARG_ENABLE(x11,
+              [AC_HELP_STRING([--enable-x11],
+                              [Explicitly enable or disable X11 support
+                              (default is to enable only if X development libraries are detected)])],
+              [enable_x11=$enableval],
+              [enable_x11=auto])
+
+AC_ARG_ENABLE(wayland,
+              [AC_HELP_STRING([--enable-wayland],
+                              [Explicitly enable or disable Wayland support
+                              (default is to enable only if Wayland client development library is detected)])],
+              [enable_wayland=$enableval],
+              [enable_wayland=auto])
+
+# Check if we have gtk-layer-shell installed, and thus should build with Wayland support
+have_wayland=no
+if test "x$enable_wayland" != "xno"; then
+  PKG_CHECK_MODULES(GTK_LAYER_SHELL, gtk-layer-shell-0, have_wayland=yes, [
+        if test "x$enable_wayland" = "xyes"; then
+          AC_MSG_ERROR([Wayland enabled but GTK Layer Shell library not found])
+        fi
+    ])
+fi
+
+AM_CONDITIONAL(ENABLE_WAYLAND, [test "x$have_wayland" = "xyes"])
+
+if test "x$have_wayland" = "xyes"; then
+  AC_DEFINE(HAVE_WAYLAND, 1, [Have the Wayland development library])
+fi
+
+AC_SUBST(GTK_LAYER_SHELL_CFLAGS)
+AC_SUBST(GTK_LAYER_SHELL_LIBS)
+
+# Check if we have the X development libraries
+have_x11=no
+if test "x$enable_x11" != "xno"; then
+  PKG_CHECK_MODULES(X, x11 xau, have_x11=yes, [
+      if test "x$enable_x11" = "xyes"; then
+        AC_MSG_ERROR([X development libraries not found])
+      fi
+    ])
+fi
+
+AM_CONDITIONAL(ENABLE_X11, [test "x$have_x11" = "xyes"])

-PLUMA_CFLAGS="$PLUMA_CFLAGS $X11_CFLAGS"
-PLUMA_LIBS="$PLUMA_LIBS $X11_LIBS"
+if test "x$have_x11" = "xyes"; then
+  AC_DEFINE(HAVE_X11, 1, [Have the X11 development library])
+fi
+
+if test "x$have_x11" != "xyes" -a "x$have_wayland" != "xyes"; then
+  if test "x$enable_wayland" = "xno" -a "x$enable_x11" = "xno"; then
+    AC_MSG_ERROR([At least one backend must be enabled])
+  else
+    AC_MSG_ERROR([No usable backend found, install X11 or Wayland development libraries])
+  fi
+fi
+
+AC_SUBST(X_CFLAGS)
+AC_SUBST(X_LIBS)

+PLUMA_CFLAGS="$PLUMA_CFLAGS $X_CFLAGS $GTK_LAYER_SHELL_CFLAGS"
+PLUMA_LIBS="$PLUMA_LIBS $X_LIBS $GTK_LAYER_SHELL_LIBS"
 AC_SUBST(PLUMA_CFLAGS)
 AC_SUBST(PLUMA_LIBS)

-PKG_CHECK_MODULES(EGG_SMCLIENT, [sm >= 1.0.0])

+dnl **************************************************************************
+dnl SM
+dnl **************************************************************************
+
+PKG_CHECK_MODULES(EGG_SMCLIENT, [sm >= 1.0.0])
 AC_SUBST(EGG_SMCLIENT_CFLAGS)
 AC_SUBST(EGG_SMCLIENT_LIBS)

@@ -286,6 +351,9 @@ Configuration:
    Gvfs metadata enabled:  $enable_gvfs_metadata
    GObject Introspection:  ${have_introspection}
    Tests enabled:      $enable_tests
+
+   X11 backend:        $enable_x11
+   Wayland backend:    $enable_wayland
 "

 if expr ${PLUMA_MINOR_VERSION} % 2 > /dev/null; then
diff --git a/pluma/pluma-app.c b/pluma/pluma-app.c
index 12fe7ef..551b2f4 100644
--- a/pluma/pluma-app.c
+++ b/pluma/pluma-app.c
@@ -36,8 +36,14 @@
 #include <unistd.h>

 #include <glib/gi18n.h>
+
+#include <gdk/gdk.h>
+#ifdef HAVE_X11
 #include <gdk/gdkx.h>
+#endif
+#ifdef HAVE_WAYLAND
 #include <gdk/gdkwayland.h>
+#endif

 #include "pluma-app.h"
 #include "pluma-prefs-manager-app.h"
@@ -654,7 +660,7 @@ is_in_viewport (PlumaWindow  *window,
    x += vp_x;
    y += vp_y;

-   #ifdef GDK_WINDOWING_WAYLAND
+#ifdef HAVE_WAYLAND
    if (GDK_IS_WAYLAND_DISPLAY (gdk_display_get_default ()))
    {
        GdkMonitor *monitor = gdk_display_get_monitor(gdk_display_get_default (), 0);
@@ -662,15 +668,15 @@ is_in_viewport (PlumaWindow  *window,
        sc_height = gdk_monitor_get_height_mm(monitor);
    }
    else
-   #endif
-   #ifdef GDK_WINDOWING_X11
+#endif
+#ifdef HAVE_X11
    if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
    {
        sc_width = WidthOfScreen (gdk_x11_screen_get_xscreen (screen));
        sc_height = HeightOfScreen (gdk_x11_screen_get_xscreen (screen));
    }
    else
-   #endif
+#endif
        g_error ("Unsupported GDK backend");

    return x + width * .25 >= viewport_x &&
diff --git a/pluma/pluma-utils.c b/pluma/pluma-utils.c
index 6c5cf81..5b72b59 100644
--- a/pluma/pluma-utils.c
+++ b/pluma/pluma-utils.c
@@ -52,13 +52,16 @@
 #include "pluma-debug.h"

 /* For the workspace/viewport stuff */
-#ifdef GDK_WINDOWING_X11
+#ifdef HAVE_X11
 #include <gdk/gdkx.h>
 #include <X11/Xlib.h>
 #include <X11/Xutil.h>
 #include <X11/Xatom.h>
 #endif
+
+#ifdef HAVE_WAYLAND
 #include <gdk/gdkwayland.h>
+#endif

 #define STDIN_DELAY_MICROSECONDS 100000

@@ -847,7 +850,7 @@ pluma_utils_replace_home_dir_with_tilde (const gchar *uri)
 guint
 pluma_utils_get_current_workspace (GdkScreen *screen)
 {
-#ifdef GDK_WINDOWING_X11
+#ifdef HAVE_X11
 if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
 {
    GdkWindow *root_win;
@@ -900,7 +903,7 @@ if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
 guint
 pluma_utils_get_window_workspace (GtkWindow *gtkwindow)
 {
-#ifdef GDK_WINDOWING_X11
+#ifdef HAVE_X11
 if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
 {
    GdkWindow *window;
@@ -956,7 +959,7 @@ pluma_utils_get_current_viewport (GdkScreen    *screen,
                  gint         *x,
                  gint         *y)
 {
-#ifdef GDK_WINDOWING_X11
+#ifdef HAVE_X11
 if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
 {
    GdkWindow *root_win;
diff --git a/pluma/pluma.c b/pluma/pluma.c
index 4fda001..f19d7c3 100644
--- a/pluma/pluma.c
+++ b/pluma/pluma.c
@@ -379,7 +379,7 @@ on_message_received (const char *message,
    if (!gtk_widget_get_realized (GTK_WIDGET (window)))
        gtk_widget_realize (GTK_WIDGET (window));

-   #ifdef GDK_WINDOWING_X11
+   #ifdef HAVE_X11
    if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
    {
        if (startup_timestamp <= 0)
@@ -428,7 +428,7 @@ send_bacon_message (void)
    display_name = gdk_display_get_name (display);

    screen_number = 0;
-   #ifdef GDK_WINDOWING_X11
+   #ifdef HAVE_X11
    if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
        screen_number = gdk_x11_screen_get_screen_number (screen);
    #endif
@@ -525,7 +525,13 @@ main (int argc, char *argv[])
    bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8");
    textdomain (GETTEXT_PACKAGE);

+#if defined(HAVE_X11) && defined(HAVE_WAYLAND)
    gdk_set_allowed_backends ("wayland,x11");
+#elif defined(HAVE_WAYLAND)
+   gdk_set_allowed_backends ("wayland");
+#else
+   gdk_set_allowed_backends ("x11");
+#endif

    startup_timestamp = get_startup_timestamp();

diff --git a/pluma/smclient/eggdesktopfile.c b/pluma/smclient/eggdesktopfile.c
index 889325e..8cb8842 100644
--- a/pluma/smclient/eggdesktopfile.c
+++ b/pluma/smclient/eggdesktopfile.c
@@ -945,7 +945,7 @@ start_startup_notification (GdkDisplay     *display,
    return NULL;
     }

-  #ifdef GDK_WINDOWING_X11
+  #ifdef HAVE_X11
   if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
   {
     if (launch_time == (guint32)-1)
@@ -966,7 +966,7 @@ start_startup_notification (GdkDisplay     *display,
   screen_str = g_strdup_printf ("%d", screen);
   workspace_str = workspace == -1 ? NULL : g_strdup_printf ("%d", workspace);

-  #ifdef GDK_WINDOWING_X11
+  #ifdef HAVE_X11
   if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
   {
     gdk_x11_display_broadcast_startup_message (display, "new",
@@ -993,7 +993,7 @@ static void
 end_startup_notification (GdkDisplay *display,
              const char *startup_id)
 {
-  #ifdef GDK_WINDOWING_X11
+  #ifdef HAVE_X11
   if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
   {
     gdk_x11_display_broadcast_startup_message (display, "remove",
@@ -1202,7 +1202,7 @@ egg_desktop_file_launchv (EggDesktopFile *desktop_file,
       screen = gdk_display_get_default_screen (display);
     }
   screen_num = 0;
-  #ifdef GDK_WINDOWING_X11
+  #ifdef HAVE_X11
   if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
     screen_num = gdk_x11_screen_get_screen_number (screen);
   #endif
mbkma commented 4 years ago

Thanks, I added your suggested changes. In weston-session (8.0.0-1) it still works with gtk-layer-shell but the mate-wayland snap seems not to work in gnome-boxes (cannot open display error). Screenshot at 2020-04-14 23-41-46

sc0w commented 4 years ago

new warnings:

cppcheck warnings:

pluma/smclient/eggdesktopfile.c:972:17: style: Variable 'description' is allocated memory that is never used. [unusedAllocatedMemory]
  description = g_strdup_printf (_("Starting %s"), desktop_file->name);
                ^

pluma/smclient/eggdesktopfile.c:973:16: style: Variable 'screen_str' is allocated memory that is never used. [unusedAllocatedMemory]
  screen_str = g_strdup_printf ("%d", screen);
               ^

clang compiler warnings:

pluma-utils.c:891:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^

pluma-utils.c:945:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^

gcc compiler warnings:

pluma-utils.c: In function 'pluma_utils_get_current_workspace':
pluma-utils.c:891:1: warning: control reaches end of non-void function [-Wreturn-type]
  891 | }
      | ^

pluma-utils.c: In function 'pluma_utils_get_window_workspace':
pluma-utils.c:945:1: warning: control reaches end of non-void function [-Wreturn-type]
  945 | }
      | ^
rbuj commented 4 years ago

I think that @sc0w suggests these changes:

diff --git a/pluma/pluma-utils.c b/pluma/pluma-utils.c
index fd778d2..2ad76a6 100644
--- a/pluma/pluma-utils.c
+++ b/pluma/pluma-utils.c
@@ -883,11 +883,10 @@ if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
    XFree (current_desktop);
    return ret;
 }
-#else
+#endif
    /* FIXME: on mac etc proably there are native APIs
     * to get the current workspace etc */
    return 0;
-#endif
 }

 /**
@@ -937,11 +936,10 @@ if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
    XFree (workspace);
    return ret;
 }
-#else
+#endif
    /* FIXME: on mac etc proably there are native APIs
     * to get the current workspace etc */
    return 0;
-#endif
 }

 /**
diff --git a/pluma/smclient/eggdesktopfile.c b/pluma/smclient/eggdesktopfile.c
index cfdec00..5cd0b1c 100644
--- a/pluma/smclient/eggdesktopfile.c
+++ b/pluma/smclient/eggdesktopfile.c
@@ -925,8 +925,7 @@ start_startup_notification (GdkDisplay     *display,
 {
   static int sequence = 0;
   char *startup_id;
-  char *description, *wmclass;
-  char *screen_str, *workspace_str;
+  char *wmclass;

   if (g_key_file_has_key (desktop_file->key_file,
              EGG_DESKTOP_FILE_GROUP,
@@ -969,13 +968,17 @@ start_startup_notification (GdkDisplay     *display,
                sequence++,
                (unsigned long)launch_time);

-  description = g_strdup_printf (_("Starting %s"), desktop_file->name);
-  screen_str = g_strdup_printf ("%d", screen);
-  workspace_str = workspace == -1 ? NULL : g_strdup_printf ("%d", workspace);
-
 #ifdef HAVE_X11
   if (GDK_IS_X11_DISPLAY (gdk_display_get_default ()))
   {
+    char *description;
+    char *screen_str;
+    char *workspace_str;
+
+    description = g_strdup_printf (_("Starting %s"), desktop_file->name);
+    screen_str = g_strdup_printf ("%d", screen);
+    workspace_str = workspace == -1 ? NULL : g_strdup_printf ("%d", workspace);
+
     gdk_x11_display_broadcast_startup_message (display, "new",
                         "ID", startup_id,
                         "NAME", desktop_file->name,
@@ -986,12 +989,13 @@ start_startup_notification (GdkDisplay     *display,
                         "DESCRIPTION", description,
                         "WMCLASS", wmclass,
                         NULL);
+
+    g_free (description);
+    g_free (screen_str);
+    g_free (workspace_str);
   }
 #endif
-  g_free (description);
   g_free (wmclass);
-  g_free (screen_str);
-  g_free (workspace_str);

   return startup_id;
 }
sc0w commented 4 years ago

I see in the logs:

    X backend:      auto
    Wayland backend:    auto

what means auto?

we can see in the logs which version is compiled?

I mean something like auto (yes) / auto (no)

rbuj commented 4 years ago

I see in the logs:

  X backend:      auto
  Wayland backend:    auto

what means auto?

You can check the config.log file for further details:

<cut>
configure:13540: checking for GTK_LAYER_SHELL
configure:13547: $PKG_CONFIG --exists --print-errors "gtk-layer-shell-0"
configure:13550: $? = 0
configure:13564: $PKG_CONFIG --exists --print-errors "gtk-layer-shell-0"
configure:13567: $? = 0
configure:13613: result: yes
configure:13642: checking for X
configure:13649: $PKG_CONFIG --exists --print-errors "x11 xau"
configure:13652: $? = 0
configure:13666: $PKG_CONFIG --exists --print-errors "x11 xau"
configure:13669: $? = 0
configure:13715: result: yes
<cut>
#define HAVE_WAYLAND 1
#define HAVE_X11 1
<cut>

Test:

$ ./autogen.sh
<cut>
Configuration:

    Source code location:   .
    Compiler:       gcc
    Compiler flags:     -g -O2
    Warning flags:      -Wall -Wmissing-prototypes
    Spell Plugin enabled:   yes
    Gvfs metadata enabled:  yes
    GObject Introspection:  yes
    Tests enabled:      yes

    X11 backend:        auto
    Wayland backend:    auto

Now type `make' to compile pluma

we can see in the logs which version is compiled?

does it really matter?

mbkma commented 4 years ago

I hope indentation is fixed, still seems weird to me mixing tabs and spaces. Tab-size is 8, maybe this could be stated at the beginning of the file.

raveit65 commented 4 years ago

I hope indentation is fixed, still seems weird to me mixing tabs and spaces. Tab-size is 8, maybe this could be stated at the beginning of the file.

Our default is not use tabs in new code, see https://wiki.mate-desktop.org/#!pages/code_style.md If it isn't possible to respect old style in a file (tabs or spaces) and the result is to add a mixed usage again, than we should replace all tabs with spaces with a second commit, IHMO. But this can be done when the new PR is working and finished.

Use different compile parameters for X11 and Wayland backends? Is it possible to build only one binary can run on X11 and Wayland at the same time?

This is really a good question. Without any testing yet, from my point as fedora distro maintainer it should be necessary to compile a pluma RPM package for x11 and wayland. Because:

Compiling only for x11 or wayland isn't optimal for me as distro maintainer.

raveit65 commented 4 years ago
  X backend:      auto
  Wayland backend:    auto

what means auto? we can see in the logs which version is compiled? I mean something like auto (yes) / auto (no)

Why not simply yes or no ? Auto is complete meaningless for me because it doesn't matter if you use --enable-wayland when the dependencies for wayland aren't installed. The result is always No :wink:

rbuj commented 4 years ago

I hope indentation is fixed, still seems weird to me mixing tabs and spaces. Tab-size is 8, maybe this could be stated at the beginning of the file.

Yes, it's the most boring part ... you can usegit clang-format as a base.

 $ cat <<EOF >> .clang-format
---
BasedOnStyle: LLVM
IndentWidth: 8
PointerAlignment: Left
AllowShortCaseLabelsOnASingleLine: false
BinPackParameters: false
BinPackArguments: false
SpaceBeforeParens: Always
...
 $ git clang-format
rbuj commented 4 years ago

@sc0w, @raveit65, @yetist In such case, we must set the default value to yes if not given for both backends:

diff --git a/configure.ac b/configure.ac
index c42471b..1b627b6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -175,20 +175,20 @@ dnl **************************************************************************
 dnl X11/Wayland backends
 dnl **************************************************************************

-# $enable_x11 and $enable_wayland will be set to "yes", "no" or "auto"
+# $enable_x11 and $enable_wayland will be set to "yes" or "no"
 AC_ARG_ENABLE(x11,
               [AC_HELP_STRING([--enable-x11],
                               [Explicitly enable or disable X11 support
                               (default is to enable only if X development libraries are detected)])],
               [enable_x11=$enableval],
-              [enable_x11=auto])
+              [enable_x11=yes])

 AC_ARG_ENABLE(wayland,
               [AC_HELP_STRING([--enable-wayland],
                               [Explicitly enable or disable Wayland support
                               (default is to enable only if Wayland client development library is detected)])],
               [enable_wayland=$enableval],
-              [enable_wayland=auto])
+              [enable_wayland=yes])

 # Check if we have gtk-layer-shell installed, and thus should build with Wayland support
 have_wayland=no

PS: We will have to update this on mate-panel too.

raveit65 commented 4 years ago

Default yes for both is OK for me, as it is the goal to support both x11 and wayland.

yetist commented 4 years ago

@mbkma

The following 2 PRs can let eom and mate-terminal work in X11 and Wayland environment at the same time, hope to help you: https://github.com/mate-desktop/eom/pull/249 https://github.com/mate-desktop/mate-terminal/pull/342

wmww commented 4 years ago

I built and tested on Sway and Mir without issue

mbkma commented 4 years ago

As far as I see auto enables the corresponding option (x11 or wayland) only if the dependency is present. So running ./autogen.sh without arguments should build one binary for both, x11 and wayland (if their dependencies are installed), shouldn't it? Defaulting to yes breaks build if GTK Layer Shell library not installed.

rbuj commented 4 years ago

@mbkma I think I understand your concern. The error message may be improved.

$ ./autogen.sh --prefix=/usr
<cut>
checking for GTK_LAYER_SHELL... no
configure: error: Wayland enabled but GTK Layer Shell library not found.
*******************************************************************************************
* You can either install gtk-layer-shell or disable the wayland support --disable-wayland *
*******************************************************************************************

change:

-          AC_MSG_ERROR([Wayland enabled but GTK Layer Shell library not found])
+          AC_MSG_ERROR([Wayland enabled but GTK Layer Shell library not found.
+*******************************************************************************************
+* You can either install gtk-layer-shell or disable the wayland support --disable-wayland *
+*******************************************************************************************])
mbkma commented 4 years ago

If this last commit is ok, I can squash it into the previous. I now made it like @yetist with no special configuration parameters required.

raveit65 commented 4 years ago

I would say it is easier to review after a squash, because second commit revert changes from first commit.

raveit65 commented 4 years ago

Usually build requires for Archlinux are similar named as in Fedora, only without -devel. wayland-devel --> wayland. https://www.archlinux.org/packages/extra/x86_64/wayland/

mbkma commented 4 years ago

@raveit65 done :)

yetist commented 4 years ago

I think wayland(-devel) is not required when building, because gdk has depend it already.

mbkma commented 4 years ago

@lukefromdc warning should be fixed. If you want broadway, enable it with ./autogen.sh --enable-broadway

raveit65 commented 4 years ago

What is broadway?

mbkma commented 4 years ago

It is a backend which provides support for displaying GTK+ applications in a web browser, using HTML5 and web sockets (https://developer.gnome.org/gtk3/stable/gtk-broadway.html).

yetist commented 4 years ago

yes, I guess broadway backend works well, if you just add 'broadway' string, only need test. It not related this PR, so you can ignore what I said.

raveit65 commented 4 years ago

In a quick test it runs fine under weston or rootson wayland-compositor, and X11. I will do more testing for daily usage under x11 now.

mbkma commented 4 years ago

@yetist if I do that, I get the same Unable to init server: Could not connect: Connection refused warnings as @lukefromdc

raveit65 commented 4 years ago

@yetist Any concerns left or can this be merged?

raveit65 commented 4 years ago

@lukefromdc I did setup a wayfire session with mate-panel for testing which i can fire up from console. It works nice. But i noticed that plume sometimes opens with a larger window size than the wayfire desktop. Means pluma opens under mate-panel or wf-panel. Did you ever saw this?

lukefromdc commented 4 years ago

On wayfire (my version though is quite old) I have not seen anything (pluma or anything else) open larger than the desktop, but I have only tested with 1080p and 4K monitors-and a rather old version of wayfire. This test (a while ago) was surely on my 4K monitor, so lots of available space

raveit65 commented 4 years ago

@mbkma @lukefromdc I have removed the backend checks and i don't get a warning when starting pluma from terminal in wayfire session.

diff --git a/pluma/pluma.c b/pluma/pluma.c
index c8919f9..c396429 100644
--- a/pluma/pluma.c
+++ b/pluma/pluma.c
@@ -522,12 +522,6 @@ main (int argc, char *argv[])
    bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8");
    textdomain (GETTEXT_PACKAGE);

-#ifdef HAVE_BROADWAY
-   gdk_set_allowed_backends ("broadway,wayland,x11");
-#else
-   gdk_set_allowed_backends ("wayland,x11");
-#endif
-
    startup_timestamp = get_startup_timestamp();

    /* Setup command line options */

How do you trigger this warning?

Edit: Same with X11, no warning.

raveit65 commented 4 years ago

On wayfire (my version though is quite old) I have not seen anything (pluma or anything else) open larger than the desktop, but I have only tested with 1080p and 4K monitors-and a rather old version of wayfire. This test (a while ago) was surely on my 4K monitor, so lots of available space

It seems it was only a one shot, i never saw this again, so ignore it.

@lukefromdc Do you see other problems or do you like to approve it?

mbkma commented 4 years ago

@raveit65 I tested again with your suggestions and I don't get the warning anymore. Before that I could trigger it with ./autogen.sh --enable-broadway and launching pluma. If the warning is gone for everyone I could remove as well:

--- configure.ac    2020-06-12 13:38:59.898282936 +0200
+++ configure1.ac   2020-06-12 13:39:12.058522288 +0200
@@ -179,17 +179,6 @@
 AC_SUBST(PLUMA_CFLAGS)
 AC_SUBST(PLUMA_LIBS)

-AC_ARG_ENABLE(broadway,
-              [AC_HELP_STRING([--enable-broadway],
-                              [Explicitly enable or disable Broadway support
-                              (default is to disable)])],
-              [enable_broadway=$enableval],
-              [enable_broadway=no])
-
-if test "x$enable_broadway" = "xyes"; then
-  AC_DEFINE(HAVE_BROADWAY, 1, [Have Broadway support])
-fi
-
 PKG_CHECK_MODULES(EGG_SMCLIENT, [sm >= 1.0.0])

 AC_SUBST(EGG_SMCLIENT_CFLAGS)
@@ -296,7 +285,6 @@
    Gvfs metadata enabled:  $enable_gvfs_metadata
    GObject Introspection:  ${have_introspection}
    Tests enabled:      $enable_tests
-   Broadway enabled:   $enable_broadway
 "

 if expr ${PLUMA_MINOR_VERSION} % 2 > /dev/null; then
raveit65 commented 4 years ago

@mbkma Can you please update PR with last suggestions (removing backend checks and broadway config option)? Than we can merge it :)

raveit65 commented 4 years ago

My entire wayfire desktop does not support window scaling for whatever reason(and in fact I can't change it in GtkInspector either), but that's unrelated to this PR.

@lukefromdc I use those settings for HIDPI scaling in my wayfire.ini

[Virtual-1]
layout = 0,0
mode = 4096x2160@50.000000
scale = 2.000000
transform = normal

Virtual-1 is my monitor in qemu-kvm virtualisation.

mbkma commented 4 years ago

@raveit65 done :)

raveit65 commented 4 years ago

LGTM.