Add linux alert #294

akinsho · 2019-01-31T09:04:01Z

Very early WIP implementation of gtk dialog alert to fix #278 (please ignore the commented out code etc. just trying to get stuff working)

Progress so far:

No longer chokes on #include <gtk/gtk.h> - currently need to tweak the borrowed snippet from #278 as an example to create an alert doesn't work as is, I think

Things to note/note to self:

I have to rework the discover.ml so it can use the pkg_config functionality on linux but behave as normal on other systems
Fix GTK crash on opening dialog - things compile but the function to open a dialog causes a crash, note this also happens in a separate project when I try and create a dialog with the same function (might be specific to my machine or more likely a bug in the code)

akinsho · 2019-01-31T09:07:07Z

src/Native/dialog.cpp

 #elif __APPLE__
-  #include "ReveryCocoa.h"
+#include "ReveryCocoa.h"


courtesy of clang-format 🤦‍♂️ (I'm hoping its formatting this how cpp code should be formatted if not can revert its changes)

Ah, interesting! We should have a code standard for our C/C++ code. I'm OK going with clang-format for this.

src/Native/dialog_gtk.c

akinsho · 2019-01-31T09:09:08Z

src/Native/dune

    (cxx_names dialog)
    (c_flags (:include c_flags.sexp))
+    (c_library_flags (:include c_library_flags.sexp))


separate config opt for c_library not equivalent to library_flags or any of the others
see here

Got it, thanks for adding this!

also run ocamlformat

akinsho · 2019-02-01T00:46:31Z

src/Native/config/discover.ml

-  | Linux 
-  | Unknown 
+module C = Configurator.V1
+


Split out the functionality to derive flags so each platform can specify a getter, also ran ocamlformat on the file as I was having difficulty following its layout especially being new to ocaml syntax

akinsho · 2019-02-02T14:50:50Z

@bryphe @OhadRau if either of you have a linux machine I'd really appreciate some help testing this. Locally I'm getting a segfault with the following error

(process:8099): Gtk-CRITICAL **: 14:33:21.922: _gtk_style_provider_private_get_settings: assertion 'GTK_IS_STYLE_PROVIDER_PRIVATE (provider)' failed
[1]    8099 segmentation fault (core dumped)

The only reference I can find anywhere to this is this PR from 2012

I'm trying to figure out whether this error is general or relates to some config option or messing about I've done on my setup, don't think I've messed with my gnome setting too much but I might have done.

You should be able to build this locally and run the dialog example just to see what happens. As I mentioned above the crash happens in a completely separate project just trying to draw a dialog on my machine, other stuff like creating a window with buttons etc works fine.

OhadRau · 2019-02-02T17:37:55Z

don't currently have a native Linux machine but I can test in WSL if that helps.

OhadRau · 2019-02-02T18:02:07Z

Having trouble getting this to compile. Still fails when trying to include gtk.h -- not sure if I'm doing something wrong, but I have pkg-config + libgtk2.0-dev + libgtk-3-dev installed so maybe I need to clear some build artifacts?

akinsho · 2019-02-03T09:14:38Z

thanks for trying it out @OhadRau, if you run
pkg-config --libs gtk+-3.0 or pkg-config --cflags gtk+-3.0 do they work?

not really familiar with WSL not sure how this would work on it tbh

OhadRau · 2019-02-03T18:07:27Z

Yeah, turns out it was just an issue with a cached build so I just deleted _esy and it compiled. I'm getting the segfault as well though :/

akinsho · 2019-02-03T21:42:16Z

@OhadRau that's at least consistent behavior, I think it might be something undocumented/not clearly documented in the API. I think that the example snippets probably miss some important bit of setup I need to do as I've tried some other more complete examples that involve opening a full window and that works. Maybe the style context (gtk's terminology) is gotten from the parent window somehow 🤷‍♂️

akinsho · 2019-02-03T22:17:28Z

Ah think I've got it now, the widgets etc. all have to called inside of a gtk_application, I've had to create a gtk window, to attach the dialog to in order to get it to run, wondering if I can somehow associate the dialog with revery's window 🤔

akinsho · 2019-02-03T23:01:28Z

Some progress here, this now correctly creates a dialog a few things to note,

The issues I was having seemed to be because the dialog wasn't associated with an application +/- window (previously tried creating a window, so think this might be more related to the dialog not being associated with an application but Im really not sure)

I v. briefly tried seeing if I could use the window handle that's passed in from glfw but not entirely sure how to convert it to a GTKWindow found this but it didn't exactly work but I only tried very briefly

Outstanding

Closing the dialog seems to stall the revery window not sure why

src/Native/dialog_gtk.c

Update todo comments

akinsho · 2019-02-04T21:38:11Z

@bryphe I've got this working locally and would appreciate you having a look over, two things to note which I've left as TODO comments is the conversion of the window handle to a GTKWindow not essential for now as this runs without it,

The second thing is that I narrowed down why this wasn't initially working to having to create a gtk_application without it seems like some context that was required to run the dialog was missing.

I'm not sure how glfw creates the windows but it appears much like other native windows on system which makes me think there's probably a gtk application created when a revery window is opened which could be re-used as I'm not sure what the consequences are of creating a new application.

Re. adding lib-gtk as far as I can tell we use an ubuntu vm for our linux pipeline on CI which I think should have gtk installed by default. Can't tell if it will need to be explicitly added (I think maybe not as the CI seems to be hitting the recurrent issue we've been getting)

bryphe · 2019-02-05T01:29:48Z

Thanks for all the work on this @Akin909 ! Will try it on my Linux machine shortly 👍

I noticed the CI build is failing with these errors:

2019-02-05T01:26:26.3450382Z dialog_gtk.c:1:24: fatal error: glib/gi18n.h: No such file or directory
2019-02-05T01:26:26.3451229Z  #include <glib/gi18n.h>

On Windows / Mac - should it be including that header file? Or should that be Linux only?

On Linux - perhaps there is an additional include directory we need?

bryphe · 2019-02-05T01:31:57Z

src/Native/dialog_gtk.c

@@ -0,0 +1,32 @@
+#include <glib/gi18n.h>


Ah, I think we'll need to wrap this in something like:

#ifndef WIN32 #ifndef __APPLE ... #endif #endif

Since we don't want this code to be picked up for Windows / Mac builds. This is probably why we're getting that glib/gi18n.h header file build failure on those environments.

bryphe · 2019-02-05T01:33:02Z

src/Native/config/discover.ml

+  | Some pc -> (
+    match C.Pkg_config.query pc ~package:"gtk+-3.0" with
+    | None -> default
+    | Some conf -> {libs= conf.libs; cflags= conf.cflags; flags= []} )


Does this return an include directory to, for the location of the header files? We might need that to fix the issue finding glib/gi18n.h on Linux in CI.

Do we need to include libgtk-3-dev as a dependency in CI?

We have our set of includes here:

revery/azure-pipelines.yml

Line 21 in be1ca7b

- script: sudo apt-get install -y libxrandr-dev libxinerama-dev libxcursor-dev libxi-dev libegl1-mesa-dev mesa-utils mesa-utils-extra ragel

and I don't believe we get GTK yet.

@bryphe glib/gi18n might eventually be necessary but isn't actually needed in this current iteration so I will remove it. re. the function it does return the header locations for the package specified in this case "gtk+-3".

I'll add gtk to the CI I saw that but thought it might be bundled by default with the distro even in CI

Side note we can continuously call pkg_config.query if we need to derive the paths for other includes (not that this is always necessary)

bryphe · 2019-02-05T01:34:42Z

src/Native/dialog_gtk.c

+   * to a GTK window, see (for inspiration):
+   * https://gist.github.com/mmozeiko/2401933b1fa89e5d5bd238b33eab0465
+   *
+   * 2. Get reference to revery application, is there an existing


It looks like if we can figure out issue 1, we might also be able to get the GtkApplication associated with the window with the gtk_window_get_application method: https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-get-application

I suspect there would already be a Gtk application created. We can track this in a separate issue, though

bryphe

Great progress, @Akin909 ! I ran it on my Manjaro box and saw that native dialog pop-up 👍

Here's the remaining set of work I see that we need:

Fix CI build error on Windows / Mac machines. This should be fixed by using #ifndef to conditionally include the body of dialog_gtk.c - we shouldn't be hitting those includes on Windows / Mac.
Fix CI build error on Linux. I think this may be resolved by adding libgtk-3-dev as a dependency, but we might also need some help from pkg-config to make sure the right include paths for glib and gtk are provided to the C compiler.

We also should track the work in an issue that was documented in dialog_gtk.c - figuring out how to convert the X11 window to a GTKWindow, and from there, getting a GTKApplication, instead of creating a new one.

bryphe · 2019-02-05T15:30:40Z

Sorry about the CI builds not triggering - following up here: https://twitter.com/bryphe/status/1092806881406406656

Kicked off a manual build w/ the latest to see how it works 🤞

bryphe · 2019-02-05T17:14:14Z

CI is green now! 🎉 Awesome work @Akin909 - we have the foundation in-place now for native linux integration!

Very early WIP implementation of gtk dialog alert to fix revery-ui#278 (please ignore the commented out code etc. just trying to get stuff working) Progress so far: No longer chokes on `#include <gtk/gtk.h>` - currently need to tweak the borrowed snippet from revery-ui#278 as an example to create an alert doesn't work as is, I think Things to note/note to self: - [x] I have to rework the `discover.ml` so it can use the pkg_config functionality on linux but behave as normal on other systems - [x] Fix GTK crash on opening dialog - things compile but the function to open a dialog causes a crash, note this also happens in a separate project when I try and create a dialog with the same function (might be specific to my machine or more likely a bug in the code)

Add revery gtk header and alert implementation

6477442

akinsho commented Jan 31, 2019

View reviewed changes

src/Native/dialog_gtk.c Outdated Show resolved Hide resolved

akinsho commented Jan 31, 2019

View reviewed changes

akinsho added 3 commits January 31, 2019 19:55

Initialise dialogue var

16ba9c5

Refactor discover.ml to allow for multiple strategies

02190f0

also run ocamlformat

Rename main function

8cb4f8e

akinsho commented Feb 1, 2019

View reviewed changes

akinsho added 2 commits February 1, 2019 16:58

pass message to dialog function

c82ed47

remove not implemented warning

0e409b6

add a cmake lists for LSP usage

7325f8e

akinsho added 3 commits February 3, 2019 22:33

create application window for dialog widget

ec8c790

rename dialog function

640edb6

rename function to activate as that is what it must be called

099f5de

akinsho commented Feb 3, 2019

View reviewed changes

src/Native/dialog_gtk.c Show resolved Hide resolved

akinsho added 3 commits February 4, 2019 09:06

Use WINDOW_TOPLEVEL to create new window

ecaa1bd

Remove created window

70b8a17

Tweak dialog to render the text passed in

ab27fb1

Update todo comments

Simplify discover by having get_linux return a config type

31bd16a

bryphe reviewed Feb 5, 2019

View reviewed changes

bryphe requested changes Feb 5, 2019

View reviewed changes

akinsho added 3 commits February 5, 2019 08:48

Remove unused dependency and wrap in OS check

f9544f9

Re-add checks for alternate OSes

05473be

Add gtk package to azure CI

bf8e1ef

akinsho changed the title ~~[WIP] Add revery gtk header and alert implementation~~ Add revery gtk header and alert implementation Feb 5, 2019

akinsho changed the title ~~Add revery gtk header and alert implementation~~ Add linux alert implementation Feb 5, 2019

akinsho changed the title ~~Add linux alert implementation~~ Add linux alert Feb 5, 2019

akinsho mentioned this pull request Feb 5, 2019

Use Revery GTK application on Linux for widgets #308

Open

bryphe approved these changes Feb 5, 2019

View reviewed changes

akinsho merged commit ed83266 into revery-ui:master Feb 5, 2019

akinsho deleted the feature/add-linux-alert branch February 5, 2019 19:02

akinsho mentioned this pull request Feb 9, 2019

esy build fails on Ubuntu 18.04.1 LTS #316

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add linux alert #294

Add linux alert #294

akinsho commented Jan 31, 2019 •

edited

Loading

akinsho Jan 31, 2019

bryphe Feb 5, 2019

akinsho Jan 31, 2019

bryphe Feb 5, 2019

akinsho Feb 1, 2019 •

edited

Loading

akinsho commented Feb 2, 2019

OhadRau commented Feb 2, 2019

OhadRau commented Feb 2, 2019

akinsho commented Feb 3, 2019

OhadRau commented Feb 3, 2019

akinsho commented Feb 3, 2019

akinsho commented Feb 3, 2019

akinsho commented Feb 3, 2019 •

edited

Loading

akinsho commented Feb 4, 2019

bryphe commented Feb 5, 2019 •

edited

Loading

bryphe Feb 5, 2019 •

edited

Loading

bryphe Feb 5, 2019

bryphe Feb 5, 2019

akinsho Feb 5, 2019 •

edited

Loading

akinsho Feb 5, 2019

bryphe Feb 5, 2019

bryphe left a comment

bryphe commented Feb 5, 2019

bryphe commented Feb 5, 2019

Add linux alert #294

Add linux alert #294

Conversation

akinsho commented Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akinsho Feb 1, 2019 • edited Loading

Choose a reason for hiding this comment

akinsho commented Feb 2, 2019

OhadRau commented Feb 2, 2019

OhadRau commented Feb 2, 2019

akinsho commented Feb 3, 2019

OhadRau commented Feb 3, 2019

akinsho commented Feb 3, 2019

akinsho commented Feb 3, 2019

akinsho commented Feb 3, 2019 • edited Loading

akinsho commented Feb 4, 2019

bryphe commented Feb 5, 2019 • edited Loading

bryphe Feb 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akinsho Feb 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryphe left a comment

Choose a reason for hiding this comment

bryphe commented Feb 5, 2019

bryphe commented Feb 5, 2019

akinsho commented Jan 31, 2019 •

edited

Loading

akinsho Feb 1, 2019 •

edited

Loading

akinsho commented Feb 3, 2019 •

edited

Loading

bryphe commented Feb 5, 2019 •

edited

Loading

bryphe Feb 5, 2019 •

edited

Loading

akinsho Feb 5, 2019 •

edited

Loading