GUI backends phase 1 #664

mmuman · 2018-10-07T01:41:11Z

This is my first attempt at trying to sort out the X11-specific code from the rest, and allow implementing other display backends, to help with Haiku port, Wayland (#56), or Metal (OSX)…

It's mostly working but not really finished. The last commit kept the original code around with changed ifdefs for easier comparison until it's done, and I'll keep amending it for now.

I'm not used much to templating in C++ so maybe the classes look a bit naive, but I believe it's a reasonable implementation (maybe using templates would avoid RTTI and speed things up, I don't know, but I'm not confident enough to try it).
I'm not used to non CamelCase class names, but oh well.

I'm open to comments and suggestions.

Some details:

the BUILD_GUI define is declared when either BUILD_X11 or any later added GUI backend is enabled.
Each backend has a priority (0 for text ones, 1 for ncurses, 2 for graphical ones), and they are sorted to try the more interesting ones first (hmm, maybe I should just use a sorted vector right away?).
The display accessors added to conky.cc are a bit uncertain. First I wanted to just loop over each of the displays, but then part of the code actually assume graphical output, or not. So it's still a mixture of for loop on current_display_outputs and calls to display_output(). In draw_stuff() I call [un]set_display_output() to make sure the display we're drawing to is the graphical one. Although in theory it's the highest priority, so the first on the list, so always the "current" one. So this part can probably be simplified.
I'm not entirely sure how to handle settings reloading, currently it breaks. I noticed the X11 window is actually created in the settings class, and not really closed between reloads. I'll have to think about this. On platforms where we would support more than one GUI displays (like, X11, SDL, Wayland…) having two enabled in the settings would cause trouble as even though only the first found would be initialize()d, the other one would still have the settings on and so the window created…
In theory we could display to many outputs at once, but I believe some parts of the code just isn't designed for this. For multiple text outputs it's ok, not for others. For example X11+ncurses didn't work already. Is it desirable? I don't think it's worth it.
the interface mimics the few X11 calls that are used in the drawing code, I believe they shouldn't be too hard to implement on other platforms (except maybe the stipple patterns on Haiku but well).
I believe most of the X11 settings are perfectly valid for other GUIs, except maybe -w.
I added messages about backends disabled at compile-time, but they are probably useless (and X11 on Haiku wouldn't make much sense anyway). Probably I should use the logging interface instead of cerr?
the lua scripting interface is concerning: it exposes X11 stuff for no particular reason, I believe it should rather provide a GUI-agnostic cairo_surface_create() call. Else it means having to fix all scripts for other platforms. We'll likely have to provide fake calls on other platforms to mimic the X11 API for compatibility anyway.
~~I think most backend methods should just return void actually, the bool is useless.~~ DONE
I noticed I used the American "initialize" instead of British, easy to change.
the fonts.clear() call when not using X11 is a bit weird, and not easy to do properly from a backend that's not been initialised. Why initialize fonts if we won't use them…

mmuman · 2018-10-07T04:11:47Z

Hmm, the font stuff also needs work it seems.

brndnmtthws · 2018-10-11T12:42:15Z

Wow, this is an epic PR. Thank you for the work. It sounds like this is still a work in progress, so I will let you continue, but please let me know when you think it's time for me to take a closer look.

brndnmtthws · 2018-10-11T12:44:28Z

src/display-console.cc

+ *
+ * Please see COPYING for details
+ *
+ * Copyright (C) 2010 Pavel Labath et al.


Ok I'll include it in my next round.

mmuman · 2018-10-18T23:46:14Z

Second round: Not much more, mostly cleanup of history and code.
Fixed copyright on new files.

lasers · 2018-10-21T12:36:22Z

@mmuman Can you fix https://www.codefactor.io/repository/github/brndnmtthws/conky/pull/672?

npyl · 2018-10-21T12:45:01Z

@mmuman For macOS if you include <vector> in display-output.cc the build may pass (It was required when i tried to compile your first round of patches)
Btw, cheers for this amazing PR, it will really help conky. 🍺

mmuman · 2018-10-22T16:46:45Z

@mmuman For macOS if you include <vector> in display-output.cc the build may pass (It was required when i tried to compile your first round of patches)

Ok.

@mmuman Can you fix https://www.codefactor.io/repository/github/brndnmtthws/conky/pull/672?

Thanks, I'll have a look.

mmuman · 2018-10-30T03:54:09Z

Another round: autosquashed all the fixup commits. Also moved the accessors around as I'll probably need them in font code.

lasers · 2018-11-04T10:03:48Z

Where do we stand now on this? Ready for a review?

mmuman · 2018-11-05T01:04:11Z

I was quite busy with work so I still didn't touch the font stuff, I'll try this week.

mmuman · 2018-11-11T00:19:35Z

I had another look at the font code, and I'm not sure how best change it:

keep the current font_list struct and add a pointer to display-specific stuff inside,
make fonts an std::vector<font_list *> and subclass font_list directly…
some other way ?

Also, it seems like xftalpha is applied only to the first font, is it an oversight or intended?

mmuman · 2018-11-11T03:01:22Z

Ok, I managed to fix the only thing left that was holding the cleanup up (crash on config reload).
I think I'll deal with the font code separately, the changes are already large enough.
Please comment.

mmuman · 2018-11-11T03:25:22Z

Looks like including < vector > in display-output.cc isn't enough, clang complains…

mmuman · 2018-11-11T03:39:58Z

and we need #include algorithm for sort()…
(bleh, <*> drops in markdown :p)

Hopefully this will lead the way to adding support for things like Wayland and Haiku graphics, cf. brndnmtthws#56. We define a display_output_base class that display backends can derive from to implement display-specific calls.

We already do this like with HTTP+stdout.

or text-only.

Some leftovers still, but it still works.

we want to be able to access either all outputs, or the currently selected one (if any, else return the top one, which ought to be the GUI one if we have one).

Still some things to sort out, but seems to work. A lot of variables and calls had to be made non-static.

Backported from 308f2cb

unused paramaters and (un)signed comparison mostly.

Since we rely on a global object ctor to add a display output to the list, it is not referenced from anywhere else, so does not get linked in when building tests since most objects are pulled from a static library. Another option would be to use --whole-archive to link to it.

mmuman · 2022-09-10T18:28:50Z

good idea to bump the version before such a big change btw.

brndnmtthws · 2022-09-10T18:29:34Z

Oh cool, let's see then… Should I squash or keep the history?

I'll leave that up to you. Squashing will keep the history a bit cleaner, but if you don't squash then you'll get credited for 23 commits in one merge.

mmuman · 2022-09-10T19:10:53Z

Since we still have things to fix I though it'd be useful to have the full history, but I'll take the git blame regardless :-D

npyl · 2022-09-11T10:22:42Z

Thanks so much for this PR @mmuman and @brndnmtthws
I have been waiting for it since 2018 haha.

mmuman · 2022-09-12T16:31:27Z

Getting there. Anyone experienced with profiling who'd want to compare before/after?

mmuman force-pushed the gui-backends branch 3 times, most recently from d0e216b to 233a875 Compare October 7, 2018 03:59

npyl mentioned this pull request Oct 7, 2018

Port conky to Metal Conky-for-macOS/conky-for-macOS#35

Open

brndnmtthws reviewed Oct 11, 2018

View reviewed changes

mmuman force-pushed the gui-backends branch 2 times, most recently from 215cb8e to 413639b Compare October 18, 2018 23:43

mmuman force-pushed the gui-backends branch 5 times, most recently from 95993aa to 2120049 Compare October 19, 2018 01:32

lasers mentioned this pull request Oct 21, 2018

WIP: RFC: GUI backends #672

Closed

mmuman force-pushed the gui-backends branch from 354d765 to 612d6af Compare October 30, 2018 03:52

mmuman force-pushed the gui-backends branch from 612d6af to 783f53d Compare November 11, 2018 02:59

mmuman force-pushed the gui-backends branch from 783f53d to 9b212a5 Compare November 11, 2018 03:10

mmuman force-pushed the gui-backends branch from 9b212a5 to f9126c0 Compare November 11, 2018 03:27

mmuman added 22 commits September 10, 2022 20:28

Make out_to_stderr setting non-static

062f0be

Initial stubbing of display backends

8d96c11

Hopefully this will lead the way to adding support for things like Wayland and Haiku graphics, cf. brndnmtthws#56. We define a display_output_base class that display backends can derive from to implement display-specific calls.

Move HTTP output code from conky.cc to display-http

5752b21

Actually allow more than one display-output at once

59adf0f

We already do this like with HTTP+stdout.

Make sure the console output isn't enabled along with ncurses

79bc896

Add a display-output flag to tell if it's graphical-based

fea1ac3

or text-only.

Initial ncurses display-output

07bc3b0

Some leftovers still, but it still works.

http display: use lower priority than graphical displays

94fbb94

Replace BUILD_X11 ifdef by BUILD_GUI when it makes sense

087c98f

display-*: Fix copyright

263bc96

display-*: return void for most calls as return-value is useless

a8849c5

proper console display (move code out of conky.cc)

7171d55

add display accessors

c7eab23

we want to be able to access either all outputs, or the currently selected one (if any, else return the top one, which ought to be the GUI one if we have one).

move file output to a separate display-file

850dbe8

move X11 stuff from conky.cc to display-x11

c7cb9a0

Still some things to sort out, but seems to work. A lot of variables and calls had to be made non-static.

Move X11 font stuff to display-x11

a2f22a8

display: fix logging

656a582

Update copyright, authors.

06fff6c

Backported from 308f2cb

Fix warnings

72e2a29

unused paramaters and (un)signed comparison mostly.

Fix crash when no display output is enabled

c4b9e00

Add TODO for the gui-backends branch

fdd478e

mmuman force-pushed the gui-backends branch from fcba98e to fdd478e Compare September 10, 2022 18:28

mmuman merged commit 15f6f27 into brndnmtthws:main Sep 10, 2022

mmuman deleted the gui-backends branch September 10, 2022 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GUI backends phase 1 #664

GUI backends phase 1 #664

mmuman commented Oct 7, 2018 •

edited

Loading

mmuman commented Oct 7, 2018

brndnmtthws commented Oct 11, 2018

brndnmtthws Oct 11, 2018

mmuman Oct 12, 2018

mmuman commented Oct 18, 2018

lasers commented Oct 21, 2018

npyl commented Oct 21, 2018

mmuman commented Oct 22, 2018

mmuman commented Oct 30, 2018

lasers commented Nov 4, 2018

mmuman commented Nov 5, 2018

mmuman commented Nov 11, 2018

mmuman commented Nov 11, 2018

mmuman commented Nov 11, 2018 •

edited

Loading

mmuman commented Nov 11, 2018 •

edited

Loading

mmuman commented Sep 10, 2022

brndnmtthws commented Sep 10, 2022

mmuman commented Sep 10, 2022

npyl commented Sep 11, 2022

mmuman commented Sep 12, 2022

GUI backends phase 1 #664

GUI backends phase 1 #664

Conversation

mmuman commented Oct 7, 2018 • edited Loading

mmuman commented Oct 7, 2018

brndnmtthws commented Oct 11, 2018

brndnmtthws Oct 11, 2018

Choose a reason for hiding this comment

mmuman Oct 12, 2018

Choose a reason for hiding this comment

mmuman commented Oct 18, 2018

lasers commented Oct 21, 2018

npyl commented Oct 21, 2018

mmuman commented Oct 22, 2018

mmuman commented Oct 30, 2018

lasers commented Nov 4, 2018

mmuman commented Nov 5, 2018

mmuman commented Nov 11, 2018

mmuman commented Nov 11, 2018

mmuman commented Nov 11, 2018 • edited Loading

mmuman commented Nov 11, 2018 • edited Loading

mmuman commented Sep 10, 2022

brndnmtthws commented Sep 10, 2022

mmuman commented Sep 10, 2022

npyl commented Sep 11, 2022

mmuman commented Sep 12, 2022

mmuman commented Oct 7, 2018 •

edited

Loading

mmuman commented Nov 11, 2018 •

edited

Loading

mmuman commented Nov 11, 2018 •

edited

Loading