This page discusses the rationales for omitting or rejecting upstream submissions mentioned on Maemo/Gtk26Contributions.
The change in question is:
-#define GDK_SCRATCH_IMAGE_WIDTH 256 -#define GDK_SCRATCH_IMAGE_HEIGHT 64 +#define GDK_SCRATCH_IMAGE_WIDTH 128 +#define GDK_SCRATCH_IMAGE_HEIGHT 32
This change doesn't make sense upstream because the reduced scratch image sizes only tend to reduce rendering speed, while causing only negligible memory savings, due to the temporary utilization of scratch images.
TommiKomulainen: But scratch images are per-process and never freed so the cumulative effect in the system is relatively big. Also the change above showed no significant performance hit in our case.
R02: gtk/gtkcalendar.h gtk/gtkcalendar.c
Many of the changes applied don't have a place in upstream, and the maemo changes are very large, both in terms of functionality and code size. The change set is so massive, that a widget fork seems in order. That is, a maemo specific calendar should be developed and maintained outside of the maemo-gtk library. Whether derivation from GtkCalendar would be a suitable development model for such a split should be decided when it occours.
luc: couldn't GtkCalendar be designed as a plugged-in widget? I think all input widgets (Calendar, File, Color, Font, more?) should have a public interface + a default implementation (in GTK+) + a plugin system. So anyone could create a new widget implementing the interface, and load it either system wide, either per user.
R03: gtk/gtkcheckbutton.c Hildon Caption Control
The "Hildon Caption Control" patch tries to reduce the clickable area of a check button to the space really occupied by the button child and the check button indicator. It does this by implementing a ::button-press method (gtk_check_button_button_press) in the check button implementation. This is unneccessary for multiple reasons. First, signal handlers can simply be connected to widgets by custom user code without patching gtk+ itself (see g_signal_connect()), and secondly, reducing the event sensitive area of a check button can simply be achieved by reducing it's allocation. In the case at hand, packing the check button into a GtkAlignment is recommended.
- Xanlopez: Agreed, but we need to think about the best way of doing this kinds of fix-ups: hildon_gtk_check_button_new ()? hildonize_check_button () ? Force the applications to do it themselves? Change the patch to address your comments but keep it in GTK+ (ugh)? Very probably there won't be a unique solution for absolutely every case and we'll need to use them all depending on the context.
The introduction of a new ::close signal to GtkMenuShell seems to offer no functionality that couldn't be achieved through the ::deactivate, ::selection-done or ::cancel signals already. The ::close signal should therefore be removed again, and any possible current use of it should be fixed to use the before mentioned signals instead.
XanLopez: Agreed, we should hunt for particular examples and put some migration examples in the guide though.
The newly added GtkButton::keyboard-button seems to just reproduce GtkButton::label functionality, albeit a bit buggy. And it comes at the cost of increasing the memory requirements of *all* buttons on the platform by 8 bytes. It also appears to not be used anywhere, so the new property should simply be backed out again.
XanLopez: Already done
The newly added GtkButton::listboxheader property causes buttons to abort their paint functions prematurely, breaking things like DEFAULT and FOCUS state drawing. Also, it attempts at drawing a text string (normally used to create a child widget from) directly. If this code was actively used somewhere in the platform at all, it is highly suspectible of causing wierd drawing artefacts, and it would impose a _large_ wastage, because using button->label_text as just string storage (which is the way this patch abuses the field) still always causes a child label widget to be created. It is not perfectly clear what exactly this patch means to achieve, it could as well just be the remainings of an intermediate patch state. We recommend this patch to be backed out of the gtkbutton.c code. And in case any work is needed to alter button label or seperator layout in list headers, we are happy to provide advice or help to achieve clearly defined layout goals in more maintainable ways.
TommiKomulainen: It was indeed unused in IT2005 but that changed in IT2006 as did the implementation. The idea is to have a visual separator between the header buttons and the treeview contents, because due to the overall theming it is desirable the header looks like labels rather than buttons. You can see this for example in Control panel/Certificate manager/Authorities
R07: GtkCellRendererText: Render prelight state cell as normal state
Usually GtkCellRendererText renders the layout with state = GTK_STATE_PRELIGHT if the cell's and widget's state is PRELIGHT. In maemo-gtk, the state is set to GTK_STATE_NORMAL instead of GTK_STATE_PRELIGHT. This change does not really belong in GtkCellRendererText, as it changes the drawing style for all themes. It would be better to introduce this change in the theme engine. GtkCellRendererText renders it's layout using gtk_paint_layout() with detail == "cellrenderertext". The theme engine could special case this by looking at the detail and state arguments and eventually changing the state.
R08: Change of default values of properties
At some places the default values of properties are changed. Such changes cannot be done upstream, as they break the API. Properties can be set manually by the application or by an auxillary library. In some cases it might be useful to derive the object type and set the property to a different value in the instance _init function.
XanLopez: see comment for R03.
R09: Disallowing moving the cursor to insensitive rows
In stock GTK+ the focus cursor (the dotted rectangle around one or more cells) is used for navigating the entire tree and can move everywhere, also to insensitive rows and even separators. If this wasn't possible, you would be unable to get to the bottom of the tree with the keyboard if all lower lines are insensitive. Also, when the focus cursor cannot move to an insensitive row, that row cannot be expanded/collapsed using the keyboard. Note that insensitive and separator rows are not selected when the cursor is on them.
In maemo-gtk insensitive rows are skipped, the focus cursor and selection are moved to the first sensitive row after the insensitive one, provided there is one. It is recommended to back these changes out.
R10: Disallowing expanding and collapsing insensitive rows
Stock Gtk+ allows the selection/deselection of children of insensitive parent rows by allowing insensitive rows to be expanded and collapsed. The Combo Box demo in gtk-demo shows an example of this. Maemo-gtk currently explicitly disables this in the treeview code. This restriction should be backed out of the maemo-gtk code to restore the basic functionality provided by stock Gtk+. In case the current behaviour indeed needs to be preserved, it is recommended to use the ::test_expand_row and ::test_collapase_row signals on GtkTreeView to control which rows can be expanded/collapsed.
R11: Keeping the focus cursor on the selected row
When the single selection mode is enabled, maemo-gtk keeps the focus cursor on the selected row. We can't do that in upstream since the focus cursor and selection are really independent, the focus cursor is there to manipulate the selection. Though, usually the focus cursor and selected row are in sync.
In multiple selection mode the focus cursor is removed if the focus cursor row is no longer selected. Without a focus cursor we cannot edit the selection with the keyboard. Stock GTK+ does unselect the row, but the focus cursor stays on that row.
It is recommended to back out both changes since both are unsuitable for keyboard navigation.
R12: Empty tree views cannot gain focus
Stock GtkTreeViews can always gain focus, in case the view is empty a "view spanning focus rectangle" is drawn. This way one can possibly bring up a context menu on the empty view using the keyboard. When an empty view cannot gain focus, this would not be possible and thus keyboard navigation would be insufficient. We'd suggest to take this out.
R13: Not possible to focus a single cell
GtkTreeView has a thought out system for focussing and activating cells. The focus cursor plays an important role in this, dependent on the configuration of columns one of the following cursors is displayed:
- A row spanning rectangle; displayed when there are no editable or activatable cells in the columns,
- A column spanning rectangle when a column has a single editable or activatable cell,
- A rectangle around a single cell if a column has multiple editable or activatable cells.
This behaviour has been changed in maemo-gtk, the whole idea of the focus cursor has been removed.
In stock GTK+ the left/right keys can move the focus cursor to other cells on a row. This has been disabled in maemo-gtk. When a row has multiple activatable cells, focussing and activating a single cell with the keyboard is impossible when this behaviour is not there. It does not affect mouse/touchscreen behaviour in any way. It is recommended to back these changes to restore proper keyboard functionality.
R14: back out gtk_check_button_clicked and gtk_check_button_update_state
Maemo-gtk introduces gtk_check_button_clicked and gtk_check_button_update_state in gtkcheckbutton.c. These two functions reimplement precisely the funcitonality that GtkCheckButton already inherits from GtkToggleButton. Without the slightest alteration in functionality. There is simply no point in adding those implementations to gtkcheckbutton.c, so the functions should simply be backed out.
R15: When a tree view is not in a scrolled window with hscroll, columns are truncated
There is a patch in maemo-gtk which truncates tree view columns (sets width to 1) if there is not enough space. This is only done if the tree view is not in a scrolled window with horizontal scroll bars. It would be much better if this was controlled by application programmers. They can either put the tree view in a scrolled window with horizontal scrolling enabled, or set meaningful maximum column widths on the tree view columns such that the tree view will never become wider than possible or wished. As this will always provide a better solution than just truncating the columns, this patch should be backed out.
R16: GtkWidgetPrivate and signal connections in gtkwidget.c
A GtkWidgetPrivate structure is introduced for GtkWidget in gtkwidget.c from maemo-gtk, which increases the memory requirements for every widget of the platform by 48 bytes (5 pointers, 7 integers). Most of the contents of this structure are rarely used though, because it's used just for implementing tap-and-hold behaviour and animations (only a fairly small subset of all widget types can make use of this functionality). This results in a large amount of wasted memory.
For an illustration of the claimed wastage, assuming 10% as a generous estimate on the number of total widget types that actually make use of the GtkWidgetPrivate structure, the overhead/utility ratio is around 10/1. That means ca. 1/2 KB is allocated according to the estimate, just to keep the state of a user interface element which implements tap-and-hold or animation functionality, 9/10th of which are always unused. Further, in order to implement the described behaviour, signal handlers are connected to the button and enter/leave signals of the widgets offering tap-and-hold functionality. For object/widget implementations however, overriding the default handler of a widget's class event signal is actually the proper way to implement customized behaviour (and saves the overhead of signal connection maintenance). It is therefore strongly recommended to refactor the code so that tap-and-hold state memory is allocated on demand only, and so that handlers setup via signal connections are turned into proper object class default handlers instead.
A patch to achieve the described effect has been sent to Nokia developers.
TommiKomulainen: The patch was applied to gtk+2.0 (2:2.6.10-1.osso17) which is included in IT2006
R17: GtkEntry: don't select all on grab_focus
maemo-gtk removes the code that selects all text when the entry grabs focus, however there is the "gtk-entry-select-on-focus" settings property which just needs to be set to FALSE to achieve exactly the same. Therefore this change should be backed out.
R18: GtkTreeView: passive focus
This rationale is somewhat related with R07. Maemo-gtk adds a "passive focus" style property, turned on by default. The only difference with stock GTK+ here is there are different backgrounds chosen depending on the selection mode of the tree view (multiple selection mode has a different background compared to the rest). The distinction can be made in the theme engine, since a pointer to the widget is passed in, together with a tree view specific detail string.
Another problem is that the maemo theme pixmaps consist of both the row background and the cursor rectangle. In the gtk_paint_focus() function with the maemo theme enabled also the background is drawn, drawing on top of the contents of the cell renderers in case of the tree view. For this to work correctly the focus rectangle drawing code has been moved to a place before the contents of the cell renderers are drawn in maemo-gtk. It would be better to have separate pixmaps for this, the background and focus rectangles are usually drawn separately.
All issues can and should be solved in the theme engine, so it is recommended to back this change out.
R19: GtkFrame: cosmetic changes
The changes to gtkframe.c by maemo-gtk involve changing the default border width of a GtkFrame in the convenience function gtk_frame_new(), and drawing the frame with gtk_paint_box() instead of gtk_paint_shadow() in correspondance to a "hildonlike" style property. The changes to gtk_frame_new() can easily be moved into a GtkFrame construction function in libhildon, and any cosmetic changes achieved by the switch to gtk_paint_box() in the expose logic should be moved into the corresponding theme engine. Because of the drawing detail "frame" that is passed in to gtk_paint_shadow() by stock Gtk+, a theme engine can easily detect the particular function call and adjust it's drawing operations accordingly.
When the suggested chanegs are applied, the maemo-gtk changes to gtkframe.c can be backed out.
R20: GtkMenu: change window type to TOPLEVEL
This change requires changes to the window manager to work properly and is thus not suited for upstream. It should probaby be backed out from maemo-gtk because if the window manager needs to be hacked anyway in order support this, it could simply be hacked to do the same special stuff to windows with the MENU type hint.
TommiKomulainen: The change was made in order to allow window manager include also menus when managing the (rather elaborate) window stacking order. GDK_WINDOW_TEMP maps to an override redirect window which essentially is a flag for the window manager to ignore it completely. Modifying the window manager to track override redirect windows is bound to have (performance) issues and Matthew has consistently been against the whole idea.
R21: gtkscrolledwindow.c scroll to focus child
The change in question warps the scrolled window scroll position upon resizes to contain the currently focussed child if any. The upstream consensus on this was that it's undesirable behaviour and can feel like fighting the user to some extend, so it won't be included in upstream, details are described in 335247. Unless a satisfactory rationale can be provided for keeping this change in maemo-gtk at all, it should be backed out. In case such a rationale covers aspects not addressed in the upstream bug report, it can be reopened for reconsideration.
R22: GtkWindow, GtkWidget: prev_focus_widget
This change affects focus behavior when the user clicks a widget, and then moves the stylus in and out. In stock GTK+, the focus is set to the clicked widget. Maemo-GTK moves the focus back to the previously focussed widget when the stylus leaves the widget. The purpose of this change is somewhat unclear, it is even a regression because click+drag_out is the only way to give focus to a widget using the mouse without activating it. This change can't go upstream, and it should be backed out from maemo-gtk because it kills a useful functionality that many users know and use.
Apart from the questionable functionality it adds, the feature also doesn't work reliably. It never works on first click in a newly opened window, and it has been seen to randomly not work at all.
TommiKomulainen: This has been backed out in gtk+2.0 (2:2.6.10-1.osso12) which is included in IT2006
R23: Checkbox mode/Multiple selection using tickmarks
In the tree view a so-called checkbox mode was introduced. The most important changes in tree view for this features are to allow "checked" rows to be drawn with a different background. All other parts should be possible to implement in "application space". Upstream cannot take over the full checkbox mode, but wants to accept changes to improve the themability of row background drawing. Two upstream bugs are pending:
https://bugzilla.gnome.org/show_bug.cgi?id=333760 - A patch which allows for more extensible row background theming.
With those features in stock GTK+, it should be possible to implement the checkbox mode fully in application space and back the changes out of maemo-gtk.
R24: gtk/gtkrange.c: stop scrolling on focus_out
Maemo-gtk contains a change in gtkrange.c to stop auto-scrolling when focus is lost (usually happens when a new dialog pops up). This change has been submitted upstream as 334446 and was rejected after some discussion, mainly because even the use case isn't entirely clear, since most GtkRange derived widgets are not grabbing focus anyway. Unless a reasonable usage case can be provided, this change should be backed out of maemo-gtk as well.
R25: gtk/gtkcellview.[ch]: make gtk_cell_view_set_cell_data() public
The gtk_cell_view_set_cell_data() was made public in maemo-gtk. Grepping all binaries and libraries on the latest available 770 and scratchbox images/rootstraps didn't find a single user of this function, and the use case where calling it would be needed is unclear. Unless a use case exists for having this function as public API, it should be private again and the change backed out of maemo-gtk.
R26: using GDK_INTERP_NEAREST instead of GDK_INTERP_BILINEAR
Some code portions change icon interpolation mode from GDK_INTERP_BILINEAR to GDK_INTERP_NEAREST in the interest of preserving CPU performance without affecting look-and-feel. It should be noted that this will only work for icons which are displayed at their original sizes, i.e. unscaled icons. As soon as icons are either up or downscaled, NEAREST interpolation produces unpleasant visible artefacts. Not all icons for all applications and themes will always be provided at the displayed size, so switching back to BILINEAR interpolation which provides good display results for most scaling ranges is strongly recommended. Regarding performance, BILINEAR interpolation can be roughly twice as slow as NEAREST interpolation, however it doesn't have major impacts like e.g. GDK_INTERP_HYPER would. Also possible slow downs should be fairly limited, because usually only a small amount of the screen requires icons and interpolation.
R27: Writing HildonButtonContainer
A dedicated HildonButtonContainer should be written that implements all the various Button sizing, Button theming (regarding drawing details) and maybe also ellipsization requirements needed for the Maemo platform. Such a container can replace current workarounds like OssoGtkButtonAttachFlags, GTK_BUTTONBOX_HETEROGENOUS and implement missing features like 'homogeneous button boxes'. It can also be used to replace other container types used for button layout in stock Gtk+ dialogs. Allthough customization of button containers for stock Gtk+ dialogs is not currently possibly, it is functionality that is reasonably easy to add and could arguably also go upstream (e.g. by adding a Gtk+ function to specify the GType to be used for container construction in dialogs).
R28: gtktextview.c: scroll to cursor on size_allocate
This patch was comitted upstream and reverted a few days after because it broke applications (see 344874), it's thus unmergable. However it's very easy to get the desired behavior by simply deriving from GtkTextView and overriding size_allocate(), or even simpler by g_signal_connect_after() to size-allocate from application code, or from a convenience constructor in hildon libs.
R29: gtkscrolledwindow.c scrollbar-dislocation style property
This boolean property, when set to TRUE, causes the scrollbars to be rendered outside the container's border. If this change was folded upstream, GtkScrolledWindow would be the only container that draws something in the area that's supposed to be an empty area that's always outside the widget. The change is unmergable because it breaks this assumption about a container's border. The same visual appearance can be acheived by setting the scrolled window's "scrollbar-spacing" style property and packing the widget so it has different borders on different sides. If style-controlled individual borders around widgets are absolutely needed, a new container class should be created in hildon libs that allows to specify the borders on all four sides separately via style properties, and the scrolled window should be packed in such a container.
R30: GtkButton::child-offset-y style property
This property, which enables modifying the button's child allocation, seems to be an obsolete workaround for earlier font rendering problems. In the source, the resp. code has #ifdef OSSO_FONT_HACK. While this hack is enabled, the style property is used nowhere in any existing theme. Also font rendering in buttons looks absolutely fine. Since it appears to be a workaround for a problem that has long been fixed in the proper place, this modification should be backed out of maemo-gtk without any replacement.
R31: gtkentry.c and gtktextview.c cursor placement on focus_in()
Both GtkEntry and GtkTextView have some commented-out code in their GtkWidget::focus_in() implementations that sets the cursor position. In GtkTextView there is additional commented-out code in GtkWidget::focus_out() which fiddles with the text view's selection. A comment in the code indicates that there is some design decision pending that might affect these code portions. Since the commented out code is there for quite a while now, chances are that either the decision was never taken or that the commented-out code was simply forgotten to remove. In either case the dead code should be removed, since such changes can easily be done by connecting to the resp. widgets' "focus-in-event" and "focus-out-event" signals, or by subclassing them.
R32: gtkmenu.c: popdown menus on visibility_notify_event
Popping menus down in response to visibility-notify events was introduced in the maemo platform as a workaround to cope with coercive desktop changes (such as from the Home or Power keys) when popups are present. A better way (listening to the key events) has been found meanwhile, so this change is not needed upstream and doesn't make sense beyond the mentioned workaround scenario.
TommiKomulainen: This is not entirely accurate. The menus should close whenever a dialog or window, for whatever reason (maybe an app is slow to start, maybe there's a critical notification from the system without user interaction, etc.) appears. Listening to key events is only a partial solution.
R33: gtkmenu.c: modify widget name/style for MenuBar children
The maemo platform has menu changes that adjust a menu's widget name according to whether it is a child of a menu bar, a submenu, or a context popup. These name changes are apparently in place to support different rcstyles for these menus. This change is not suitable for upstream because upstream in general doesn't assign widget names at all. Also, no actuall use of the fetures outlined could be found throughout the meamo plattform and its themes, menubars in particular are never used. It is therefore suggested, to simply back the mentioned changes out of the maemo Gtk+ version.
R34: Arrow sensitivity adjustment in GtkCombBox
Maemo adds a property called "autodimmed_button", which automatically makes the combo box insensitive if the tree model attached to it is empty. We think this should be controlled by the application instead, since there are multiple valid use cases where you would want to pop up the combo box when the list is (still) empty; for example with a history list which is still empty, or the list will only be filled after the user has pressed the button (database accesses for example).
This modification should move to hildon-libs, or even better client applications should handle this themselves.
R35: unused menu_popped boolean in GtkEntry
Maemo-gtk adds "gboolean menu_popped" to the GtkEntryPrivate struct. This boolean is only set and never used (nothing depends on it), therefore it can be safely removed without replacement.
R36: GtkIconSize: newly added HILDON icon sizes
New icon sizes can be registered with Gtk+ during runtime, thus there is no need to patch maemo-gtk to register icon sizes beyond the stock icon sizes. The recommended change here is to add a hildon_init() function to hildon libs which wraps gtk_init() and can be used to customize Gtk+ according to maemo platform needs. Amongst other things this function should then register newly required icon sizes (alternatively, a Gtk+ module could be written that automates customizations upon gtk_init() time).