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

Build Electron with GTK+ 3 #10

Open
tensor5 opened this issue May 29, 2016 · 35 comments
Open

Build Electron with GTK+ 3 #10

tensor5 opened this issue May 29, 2016 · 35 comments

Comments

@tensor5
Copy link
Owner

tensor5 commented May 29, 2016

This is done in the electron_gtk3 branch.

Atom and Electron's default app crash on keypress. No problem arises during the build of libchromiumcontent, but there are some warnings from Electron:

[...]
[946/1080] CXX obj/atom/browser/ui/electron_lib.file_dialog_gtk.o
../../atom/browser/ui/file_dialog_gtk.cc:45:32: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
    const char* confirm_text = GTK_STOCK_OK;
                               ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:756:38: note: expanded from macro 'GTK_STOCK_OK'
#define GTK_STOCK_OK               ((GtkStock)"gtk-ok")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
../../atom/browser/ui/file_dialog_gtk.cc:50:22: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
      confirm_text = GTK_STOCK_SAVE;
                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:964:38: note: expanded from macro 'GTK_STOCK_SAVE'
#define GTK_STOCK_SAVE             ((GtkStock)"gtk-save")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
../../atom/browser/ui/file_dialog_gtk.cc:52:22: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
      confirm_text = GTK_STOCK_OPEN;
                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:765:38: note: expanded from macro 'GTK_STOCK_OPEN'
#define GTK_STOCK_OPEN             ((GtkStock)"gtk-open")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
../../atom/browser/ui/file_dialog_gtk.cc:58:9: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
        GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
        ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:159:38: note: expanded from macro 'GTK_STOCK_CANCEL'
#define GTK_STOCK_CANCEL           ((GtkStock)"gtk-cancel")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
4 warnings generated.
[947/1080] CXX obj/atom/browser/ui/electron_lib.tray_icon.o
[948/1080] CXX obj/atom/browser/ui/electron_lib.message_box_gtk.o
../../atom/browser/ui/message_box_gtk.cc:56:35: warning: 'gtk_icon_source_new' is deprecated [-Wdeprecated-declarations]
      GtkIconSource* iconsource = gtk_icon_source_new();
                                  ^
/usr/include/gtk-3.0/gtk/deprecated/gtkiconfactory.h:170:16: note: 'gtk_icon_source_new' has been explicitly marked deprecated here
GtkIconSource* gtk_icon_source_new                      (void);
               ^
../../atom/browser/ui/message_box_gtk.cc:57:29: warning: 'gtk_icon_set_new' is deprecated [-Wdeprecated-declarations]
      GtkIconSet* iconset = gtk_icon_set_new();
                            ^
/usr/include/gtk-3.0/gtk/deprecated/gtkiconfactory.h:138:13: note: 'gtk_icon_set_new' has been explicitly marked deprecated here
GtkIconSet* gtk_icon_set_new             (void);
            ^
../../atom/browser/ui/message_box_gtk.cc:58:7: warning: 'gtk_icon_source_set_pixbuf' is deprecated [-Wdeprecated-declarations]
      gtk_icon_source_set_pixbuf(iconsource, pixbuf);
      ^
/usr/include/gtk-3.0/gtk/deprecated/gtkiconfactory.h:183:16: note: 'gtk_icon_source_set_pixbuf' has been explicitly marked deprecated here
void           gtk_icon_source_set_pixbuf               (GtkIconSource       *source,
               ^
../../atom/browser/ui/message_box_gtk.cc:59:7: warning: 'gtk_icon_set_add_source' is deprecated [-Wdeprecated-declarations]
      gtk_icon_set_add_source(iconset, iconsource);
      ^
/usr/include/gtk-3.0/gtk/deprecated/gtkiconfactory.h:159:16: note: 'gtk_icon_set_add_source' has been explicitly marked deprecated here
void           gtk_icon_set_add_source   (GtkIconSet          *icon_set,
               ^
../../atom/browser/ui/message_box_gtk.cc:60:26: warning: 'gtk_image_new_from_icon_set' is deprecated [-Wdeprecated-declarations]
      GtkWidget* image = gtk_image_new_from_icon_set(iconset,
                         ^
/usr/include/gtk-3.0/gtk/gtkimage.h:125:12: note: 'gtk_image_new_from_icon_set' has been explicitly marked deprecated here
GtkWidget* gtk_image_new_from_icon_set  (GtkIconSet      *icon_set,
           ^
../../atom/browser/ui/message_box_gtk.cc:62:7: warning: 'gtk_message_dialog_set_image' is deprecated [-Wdeprecated-declarations]
      gtk_message_dialog_set_image(GTK_MESSAGE_DIALOG(dialog_), image);
      ^
/usr/include/gtk-3.0/gtk/gtkmessagedialog.h:115:12: note: 'gtk_message_dialog_set_image' has been explicitly marked deprecated here
void       gtk_message_dialog_set_image    (GtkMessageDialog *dialog,
           ^
../../atom/browser/ui/message_box_gtk.cc:64:7: warning: 'gtk_icon_source_free' is deprecated [-Wdeprecated-declarations]
      gtk_icon_source_free(iconsource);
      ^
/usr/include/gtk-3.0/gtk/deprecated/gtkiconfactory.h:174:16: note: 'gtk_icon_source_free' has been explicitly marked deprecated here
void           gtk_icon_source_free                     (GtkIconSource       *source);
               ^
../../atom/browser/ui/message_box_gtk.cc:65:7: warning: 'gtk_icon_set_unref' is deprecated [-Wdeprecated-declarations]
      gtk_icon_set_unref(iconset);
      ^
/usr/include/gtk-3.0/gtk/deprecated/gtkiconfactory.h:145:13: note: 'gtk_icon_set_unref' has been explicitly marked deprecated here
void        gtk_icon_set_unref           (GtkIconSet      *icon_set);
            ^
../../atom/browser/ui/message_box_gtk.cc:106:14: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
      return GTK_STOCK_CANCEL;
             ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:159:38: note: expanded from macro 'GTK_STOCK_CANCEL'
#define GTK_STOCK_CANCEL           ((GtkStock)"gtk-cancel")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
../../atom/browser/ui/message_box_gtk.cc:108:14: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
      return GTK_STOCK_NO;
             ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:747:38: note: expanded from macro 'GTK_STOCK_NO'
#define GTK_STOCK_NO               ((GtkStock)"gtk-no")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
../../atom/browser/ui/message_box_gtk.cc:110:14: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
      return GTK_STOCK_OK;
             ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:756:38: note: expanded from macro 'GTK_STOCK_OK'
#define GTK_STOCK_OK               ((GtkStock)"gtk-ok")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
../../atom/browser/ui/message_box_gtk.cc:112:14: warning: 'GtkStock' is deprecated [-Wdeprecated-declarations]
      return GTK_STOCK_YES;
             ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:1094:38: note: expanded from macro 'GTK_STOCK_YES'
#define GTK_STOCK_YES              ((GtkStock)"gtk-yes")
                                     ^
/usr/include/gtk-3.0/gtk/deprecated/gtkstock.h:108:16: note: 'GtkStock' has been explicitly marked deprecated here
typedef char * GtkStock;
               ^
12 warnings generated.
[949/1080] CXX obj/atom/browser/ui/views/electron_lib.frameless_view.o
[950/1080] CXX obj/atom/browser/ui/views/electron_lib.global_menu_bar_x11.o
[951/1080] CXX obj/atom/browser/ui/views/electron_lib.menu_layout.o
[952/1080] CXX obj/atom/browser/ui/views/electron_lib.menu_delegate.o
[953/1080] CXX obj/atom/browser/ui/views/electron_lib.native_frame_view.o
[954/1080] CXX obj/atom/browser/ui/views/electron_lib.menu_bar.o
../../atom/browser/ui/views/menu_bar.cc:37:21: warning: 'gtk_rc_get_style' is deprecated [-Wdeprecated-declarations]
  GtkStyle* style = gtk_rc_get_style(menu_bar);
                    ^
/usr/include/gtk-3.0/gtk/deprecated/gtkrc.h:175:11: note: 'gtk_rc_get_style' has been explicitly marked deprecated here
GtkStyle* gtk_rc_get_style              (GtkWidget   *widget);
          ^
1 warning generated.
[...]

Upstream issue here.

tensor5 added a commit that referenced this issue Jun 23, 2016
@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

Thanks to the brilliant solution of @nikolowry, as of v1.2.4 our build of Electron uses GTK+ 3.

I'll leave this issue open until we are sure that the build with the new option is completely stable.

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

Text in the menu bar is white.

screenshot from 2016-06-23 09-58-50

@repsac-by
Copy link

On hidpi display font is too small. Do not scaled as it should. Before gtk3 all was well.

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

@repsac-by Can you attach a screenshot?

@repsac-by
Copy link

2016-06-23 10-27-51

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

Can you manually increase the font size in Atom, or does it look weird?

I don't have a hidpi display, but I noticed that gnome-tweak-tool has a setting Fonts -> Scaling Factor. Have you tried that one?

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

I also found this https://wiki.archlinux.org/index.php/HiDPI#GDK_3_.28GTK.2B_3.29
Please try and let me know.

@repsac-by
Copy link

Setting the font size in the atom only affects the source code editor.

Yes it is in the gnome-tweak-tool has HiDPI factor 2. Before gtk3 electron scaled, but now no longer. Other applications on the electron look the same, not only Atom.

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

I'll revert to gtk2 in the next build.

@repsac-by
Copy link

GDK_SCALE=2 it changes the scale

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

Strange, I tried on mine and didn't do anything.

@repsac-by
Copy link

Even if you set GDK_SCALE=1 the scale remains correct. It turns out it is important the very existence of environment variable.

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

If your problem is completely solved, we will stay with gtk3 for now and see if other issues come up.

@repsac-by
Copy link

Well, not completely, but I am willing to suffer for a while. 👍

@repsac-by
Copy link

if I can help someone, I myself did launcher /usr/local/bin/electron

#!/bin/sh

export GDK_SCALE=1

exec /usr/lib/electron/electron --ignore-gpu-blacklist "$@"

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

Can you test it in both Wayland and X? I may include the export in the electron launcher script.

@repsac-by
Copy link

repsac-by commented Jun 23, 2016

For wayland completely different behavior. GDK_SCALE=1 not scale, only GDK_SCALE=2.
And I had to change launcher /usr/lib/electron/electron to /usr/bin/electron.

#!/bin/sh

export GDK_SCALE=2

exec /usr/bin/electron --ignore-gpu-blacklist "$@"

@IkeLutra
Copy link

I also experienced this issue. Adding GDK_SCALE=1 fixes it in X for me. Will try in Wayland later if I have time.

Thanks for the solution btw

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

I noticed that GDK_SCALE=2 doesn't change anything for me, and my resolution is 1366x768. I wonder if there is a minimum resolution above which GDK_SCALE has effect. If so the following /usr/bin/electron should work for everybody:

#!/usr/bin/bash

export GDK_BACKEND=x11
export GDK_SCALE=2

exec /usr/lib/electron/electron "$@"

We should test this setting across different displays.

BTW @repsac-by is --ignore-gpu-blacklist relevant here?

@repsac-by
Copy link

Sorry --ignore-gpu-blacklist is not true to scale, I just activate the accelerated graphics.

@tensor5
Copy link
Owner Author

tensor5 commented Jun 23, 2016

I just noticed that although GDK_SCALE=2 doesn't scale my atom window, it does scale the Open File and Open Folder windows.

@repsac-by @willotter I guess you will have the same problem GDK_SCALE=1: a small file chooser window.

@IkeLutra
Copy link

@tensor5 With X and GDK_SCALE=1 I get a correct sized file chooser on my HiDPi screen. With GDK_SCALE=2 atom is all the same size but the file chooser is huge.

Also btw @repsac-by I used to use --ignore-gpu-blacklist but it all seems to work without it now so might be worth trying.

tensor5 added a commit that referenced this issue Jun 24, 2016
@tensor5
Copy link
Owner Author

tensor5 commented Jun 24, 2016

Electron should now work out of the box on HiDPI displays, at least on X.

@tensor5
Copy link
Owner Author

tensor5 commented Jul 20, 2016

Commit fe61a99 fixes the issue of white text in the menu bar (see #18).

@juvevood
Copy link

juvevood commented Sep 8, 2016

black theme was wrong
black

@tensor5
Copy link
Owner Author

tensor5 commented Sep 8, 2016

@juvevood, what theme are you using?

@juvevood
Copy link

juvevood commented Sep 8, 2016

@tensor5
Copy link
Owner Author

tensor5 commented Sep 11, 2016

@juvevood, I fixed the menu bar text in 31dc838. However, there is still the issue with the menu item becoming black when hovered.

@juvevood
Copy link

@tensor5 ,Thank you!I also feedback the problem to the author of the theme, I tried other themes there is no problem

@tensor5
Copy link
Owner Author

tensor5 commented Sep 18, 2016

@juvevood I think I can fix it, just need some time.

Once I've done with this and a few other details, I'll PR a comprehensive gtk3 patch to Electron.

@tensor5
Copy link
Owner Author

tensor5 commented Sep 28, 2016

@repsac-by and @willotter, I changed the way Electron handles HiDPI in 5e21d0d, can you tell me if the new settings work correctly on your screens? (I don't have a HiDPI screen).

@IkeLutra
Copy link

@tensor5 Just updated to electron-1.4.1-2 (is that the correct version?) and seems to be working fine for me.

@tensor5
Copy link
Owner Author

tensor5 commented Sep 28, 2016

@willotter your version is correct, thank you.

@tista500
Copy link

tista500 commented Jan 8, 2017

@tensor5,

Hi I'm an author of Adapta-gtk-theme.
I now have 3 questions about your patch to employ Gtk+ 3.x backends for GtkMenuBar and GtkMenuItem.

  1. https://github.com/tensor5/arch-atom/blob/master/electron/gtk3-menu-bar.patch#L46
    That pointer seems to be shared to fill the background-area in both menubar node and menubar > menuitem node, right?

  2. In Gtk+ 2.x, we usually set styling for GtkMenuItem like this (gtkrc):

widget_class "*<GtkMenuItem>*"                              style "menu_item"
widget_class "*<GtkMenuBar>.<GtkMenuItem>*"      style "menubar_item"

Indeed, the styling of GtkMenuItem is placed at root because that styling is used in GtkMenu GtkMenuItem ( = menu-items in the context menu). But in Gtk+ 3.x, we do not.
Currently we set menuitem node more precisely like this (CSS):

menubar > menuitem { }
menu menuitem { }

But unfortunately I can't find any cascading structures in your patch. It means a single menuitem node was shared in both menubar > menuitem and menu menuitem? And I can't understand where that menuitem styling came from (In Adapta, there's no @at-root menuitem styling).

  1. Pitch black hovered menuitem node in context-menu was shown in Adapta, is this related to alpha values in RGBA of background-color property?

I did not read a whole codes of atom/browser/ui/views/menu_bar.cc yet though, but if that code did not prepare for the alpha blending by filling the parent GtkMenu with proper solid background, Adapta might fail to composite that alpha value of background-color when menu menuitem was hovered (In Gtk+ 3.x, we usually set background-color only for GtkMenu instead of GtkMenuItem itself).

Finally,
If Q.2 and Q.3 were completely separated, I can set solid background-color to menu menuitem:hover node with alpha = 1.0. But if not, solid background affects to menubar > menuitem:hover node as well... Means it willintroduce a new issue in GtkMenuBar. :(

So let me know if you had some ideas...
Regards.

EDIT

I've committed a workaround for GtkPopupWindow without ClientSideDecoration: adapta-project/adapta-gtk-theme@87fc030
That commit overrides menu menuitem of .popup style-class by using fully opaque colour definitions for non-composited elements in context-menus. So pitch black hovered menuitem node issue should be fixed with this ugly workaround...

@tensor5
Copy link
Owner Author

tensor5 commented Jan 10, 2017

Hi @tista500,

Thank you for looking into this issue.

1 and 2. The original GTK2 code of Electron only had one background color, picked with:

```
GtkWidget* menu_bar = gtk_menu_bar_new();
GtkStyle* style = gtk_rc_get_style(menu_bar);
*background = libgtk2ui::GdkColorToSkColor(style->bg[GTK_STATE_NORMAL]);
```

(see https://github.com/electron/electron/blob/master/atom/browser/ui/views/menu_bar.cc#L35)

I tried to replicate that in GTK3, however sometimes the background color is not defined in the css node of menubar, so I use if (background_color_ptr->alpha == 0.0) to check that and in that case I pick the background color of the top level window.

The problem with this patch is that I'm using gtk_style_context_get_background_color (although I naively rewrote it in terms gtk_style_context_get) which is deprecated in GTK3. I may write a better patch when I have time that uses gtk_render_background() instead, but it requires more work.

  1. I don't really understand this question, I should try Adapta first and see it myself :)

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

No branches or pull requests

5 participants