NotifyWindow
Note: The current results of this discussion are documented at NotifydEngine
Note: this has turned into a discussion page, see below.
There is a general agreement that notification-daemon needs a common base class for themes to extend. I'm calling it NotifyWindow, which seems to be a sensible name. This page describes in broad terms the NotifyWindow class interface with some comments on implementation in places where they seem to be appropriate. Comments and improvements definitely useful.
After looking at the current interface between the theme and the daemon, there doesn't seem to be anything particularly wrong with it. All we need to do is switch it over to being a more convenient object-oriented system, where a theme just implements a subclass of NotifyWindow. NotifyWindow itself would be derived from GtkWindow?, which is what is currently used to implement both themes, and provides the necessary functionality.
NotifyWindow's proposed interface is based quite closely on the current interface used to communicate with themes, which I believe to be largely sufficient. Needless to say, most of these methods will be virtual so that the theme classes can override them. Many of them will be no-ops in NotifyWindow (or pure virtuals, but I don't like the idea of those so much in a language where the compiler can't enforce their implementation, as it leads to dereferencing null pointers too easily).
NotifyWindow *notify_window_new(NotifyUrlCb? cb) This is just the constructor. It takes the callback function for clicking anywhere on the notification.
void notify_window_show(NotifyWindow *nw) A wrapper around gtk_window_show(). Virtual of course, so that the themes can do any other fancy work they want to do here.
void notify_window_hide(NotifyWindow *nw) Similar to show()
void notify_window_move(NotifyWindow *nw, int x, int y) Since any theme which can do arrows is going to need to recompute them on move, themes are going to have to override this one.
void notify_window_set_hints(NotifyWindow *nw, GHashTable *hints) We can't really provide a good implementation of this one, as themes are going to need to do their own thing when the need for an arrow is detected, or when the urgency changes, and that all comes in through here.
Perhaps we could do better than that. Arrows and urgency are fundamental concepts, so perhaps we can have void notify_window_set_urgency void notify_window_set_arrow(NotifyWindow *nw, int x, int y) as well. But it would also be nice to make arrow handling easier for the theme drawing code if we can. What are those x and y, after all? relative to the given top left of the notification? screen coordinates as in the current system? How can the core ease the burden on theme programmers here?
void notify_window_set_text(NotifyWindow *nw, const gchar *summary, const gchar *body) void notify_window_set_icon(NotifyWindow *nw, GdkPixbuf? *pixbuf) void notify_window_set_arrow(NotifyWindow *nw, int x, int y) void notify_window_add_action(NotifyWindow *nw, const gchar *label, const gchar *key, ActionInvokedCb? cb) Definitely need to leave this lot up to the themes. add_action() bothers me though, because the name implies, in a GTK world, that it should take a GtkAction?. Now, it *could* take a GtkAction? if we wanted it to, but I'm not sure that would be a wise move for all themes. It would open us up to the ability to add stock action buttons to notifications, and for apps to request them, but that presumably requires a notification spec change which might not be appropriate. It would be nice if we had another name for the current form of that method though, I think so at least.
void notify_window_set_timeout(NotifyWindow *nw, guint timeout) void notify_window_tick(NotifyWindow *nw, glong remaining)
The two animation functions can be no-ops for themes without anything to tick. I suggest that in order to avoid needlessly calling them, NotifyWindow also has a Boolean property "tickable" which indicates if a tick function will actually do anything. If it's false at show() time, the daemon won't call set_timeout() or tick(). This is roughly the same as the way the daemon picks between calling them for standard, and not calling them for bubbles, which doesn't have them.
As regards the interface to the daemon core, using virtual methods the only time the core should need to know what class the notification actually is would be at construction time, so the theme's just going to have to provide a construction function to call, since we unfortunately don't have the language capacity to pass type names around for this sort of thing. This is, however, a great deal better than passing ALL the functions around. It'll still be doing a pointer dereference to call the functions, but it'll make the core and the theme code cleaner I think.
Improvements welcome!
Notes by M.S.
If we work on defining a clean interface for theme engines to implement I would propose to check if we can satisfy a few more requirements and just do it right.
Aims:
- Cleaner theme code with primary focus on drawing
Questions:
- Further abstraction? Example: ...set_arrow() denotes a graphical concept, whereas a task concept would be better (such as set_point_at()). Additionally, my theme might not support/need drawing arrows at all or draws pink beavers instead of arrows.
- Are themes bound to a daemon session or a notification? It might be useful to create a specific notification with a different theme. Example: A regular notification uses the standard theme, another one an "album/music" theme with composited playing information and a different one gives feedback on changes of the volume level (when issued by multimedia keys).
- Positioning logic and how much of that in the theme engine? Currently the daemon calculates stack popup positions BUT also the theme calculates it's position if it is using an arrow... Something certainly can be optimized here instead of both parts calculating/positioning and twiddling with multi-head? Also a theme might want to create stacks in the middle of the screen or use the full display width and align to the top of the desktop...
The API seems to be split into those categories:
- Create/Destroy
- Visibility (show,hide,tick)
- Content Data (text, icon, action/buttons)
- Context Data (urgency, hints, timeout)
Responses by MattW
Okay, let's do it better!
set_point_at() is also a bit graphical if we're going down that route (which is actually a good route to go down). set_source(), set_origin() (bad move perhaps, origin's a bit overloaded in graphics). What's the name of the concept of there being a point of the screen from which the notification should be seen to come?
Using a different theme for certain notifications: it's certainly possible. Growl does it on the Mac. I personally am inclined to see it as unnecessary. Volume adjustments behave differently to the usual model we see with libnotify, and do people really want everything coming up looking different? That basically returns us to everything looking like each app developer wrote their own notification method (even though they didn't). I'm making assumptions here, I know, but it's not a feature I'm interested in.
It does raise the point of what to do with themes which behave differently in terms of where they draw. The daemon needs to know if they behave in a standard stacking sort of way, or if they do something else like hugging a screen edge (stackably, sequentially or like a stock ticker?)
Some way of getting the daemon to do the positioning calculations even when using an arrow would require the theme to be able to tell the daemon how to calculate the right position for it - arrow sizes, offsets perhaps, but some themes are always going to want to go off and do their own thing, so I guess we need an interface where the theme can say 'don't calculate me any positions, I'll do it all the hard way myself'. Still probably needs to know which head to be on.
Hmm. Doing this properly is harder than I thought. I'm inclined to take the simplest options where we can. Support a standard model of small stacking notifications, some sort of way of calculating positions for notifications which have arrows, and then have a back door for themes with really esoteric requirements. We shouldn't be trying to support every possible idea.
MattW: Next iteration of design: use an interface
To provide support for really weird/fancy themes without adding an unreasonable amount of complexity, specify that themes must implement an interface. Also provide a GtkWindow? subclass which implements that interface to allow easy implementation of standard-behaviour stacking themes. Then anybody who wants to do anything else will be able to just implement the interface on top of any other class they feel like.
So the critical thing is, what does the interface need to support? As I see it, it needs to have methods to:
create, show, hide and destroy notifications set the notification's content (text, icon and actions) set the notification's urgency inform the core if the notification participates in the standard positioning system or not if it does: methods for telling the notification where to be if it doesn't: just let it do whatever it wants a method for telling the notification what to point at (arrows): themes which don't support arrows can no-op this. The core doesn't need to know, because arrowed notifications don't participate in the positioning system anyway inform the core if the notification requires tick events. If so, the set_timeout and tick methods need to be implemented in an appropriate manner. We could just say no-op them if they aren't needed, but it seems a bit silly for the core to call them for notifications which don't have any animation.
I also feel we need to stop calling actions in notifications actions, because it suggests a clash with GtkAction?, and we don't really do that. Responses?
I also haven't mentioned hints here... what do we use hints for other than the x and y for arrows? Do we need hints? Should we stick to implementing arrows via hints? I'm not sure.
Responses by Christian Hammond
I like the idea of having a generic interface that isn't necessarily GtkWindow-based. Your design for this looks good and I think it would work well to start.
I'm not very concerned with the clash between actions and GtkAction?. Largely because we're not using GtkAction? at all and probably won't have a real need to. The spec indicates actions, which is perfectly fine since it's not tied to GTK+ or anything, and I think we should be consistent with it.
I don't believe we're currently using hints for anything in the theme layer aside from X, Y positions (which could and probably should be its own specialized data). I think it's fine to have hint support in the themes still, as it may be useful in the future even if it's not now. For example, imagine if some device like the N800 had a specialized theme that respected some device-specific hint and programs that set that hint. We shouldn't prevent that just because we don't have an immediate use-case for it.
