widget->state and styles



So, I sat down this morning to think some more about
GTK_STATE_INACTIVE, in particular the two objections:

 - There may need to be combinations of GTK_STATE_INACTIVE with, 
   say, GTK_STATE_SELECTED.

 - Adding a new state may introduce subtle, hard to
   locate breakage.

Since, this solution to the second is clearly to break lots
more stuff with decisiveness, in a blue-sky sort of way,
I tried to figure out how widget->state really _should_ work.

[ 
  Note: in this mail, I'm playing, not "crotchety old team
  leader trying to meet release schedule", but "bright young
  technical mind eager to tell everybody about his idea." 
]

Some problems that can be identified with the current way
that GtkStateType works and interacts with RC files and
colors are:

 * The current set of colors defined in an RC file 
   (20 colors - 5 states for each of fg,bg,base,text)
   is not at all intuitive for users: it's hard
   to map it into a simple model of "pick the color
   for the background of an entry."

   The fact that we still don't have a "Choose colors"
   capplet for the GNOME control center is good
   evidence for this.

 * The current model encourages and requires abuse of
   the form "lets use GTK_STATE_ACTIVE for the trough
   of a scrollbar."

   There is no good separation between the state
   of the widget (insensitive, prelit, etc.) and
   the type of color being used (selected, base, etc.)

 * Some colors that it makes sense to set are missing.
   For instance, there is no way to combine GTK_STATE_ACTIVE
   and GTK_STATE_INSENSITIVE to get the color for
   the trough of an insensitive scrollbar.

 * The gtk_paint_* functions have no representation
   of the type of color to use, so there is extensive
   use of magic detail strings to carry this information.

   A theme engine has to know that when it's told
   to paint a flat box with a detail of "entry_bg",
   it should use the base color, not the bg color.

 * We don't have an inactive state, and adding one
   would make all of the above problems worse.
   

My basic idea was that GtkStateType should be factored
into two components:

 mode: 
   GTK_MODE_NORMAL,
   GTK_MODE_SELECTED, 
   GTK_MODE_DEPRESSED, 
   GTK_MODE_BASE

 event mode: 
   GTK_EVENT_MODE_NORMAL,
   GTK_EVENT_MODE_PRELIGHT,
   GTK_EVENT_MODE_INACTIVE,
   GTK_EVENT_MODE_INSENSTIIVE

[ I'm not particularly happy with these names, but the
  alternatives I came up with were worse: "receptivity"? ]

So, the combination of these two gives a total of 16
states. But since we have included style->base[]
and style->text[] into this, the total number of colors
only goes from:

 (fg,bg,base,text) * 5 states      = 20 colors
 (fg,bg) * 4 modes * 4 event modes = 32 colors


GtkStateType would be retained, with the bits arranged
so that:
 
 state = mode | event_mode;


With this change, any code that does style->fg_gc[widget->state]
needs to be reevaluated. To enforce that, the 
fg, bg, fg_gc, bg_gc, etc members of the GtkStyle structure
would be moved to private fields, and public accessors added:

void      gtk_style_get_fg                   (GtkStyle     *style,
					      GtkMode       mode,
					      GtkEventMode  event_mode,
					      GdkColor     *color);
void      gtk_style_get_bg                   (GtkStyle     *style,
					      GtkMode       mode,
					      GtkEventMode  event_mode,
					      GdkColor     *color);
GdkGC *   gtk_style_get_fg_gc                (GtkStyle     *style,
					      GtkMode       mode,
					      GtkEventMode  event_mode);
GdkGC *   gtk_style_get_bg_gc                (GtkStyle     *style,
					      GtkMode       mode,
					      GtkEventMode  event_mode);

A number of additional special modes would be allowed for 
gtk_style_get_bg() - GTK_MODE_LIGHT, GTK_MODE_DARK, GTK_MODE_MID -
these correspond to the current style->light[state].

It turns out that a very common operation is something like:

 gc = gtk_style_get_bg_gc (widget->style,
                           GTK_MODE_BASE,
                           GTK_STATE_EVENT_MODE (widget->state));

So, there would be convenience functions:

void   gtk_widget_get_fg                (GtkWidget *widget,
					 GtkMode    mode,
					 GdkColor  *color);
void   gtk_widget_get_bg                (GtkWidget *widget,
					 GtkMode    mode,
					 GdkColor  *color);
GdkGC *gtk_widget_get_fg_gc             (GtkWidget *widget,
					 GtkMode    mode);
GdkGC *gtk_widget_get_bg_gc             (GtkWidget *widget,
					 GtkMode    mode);
void   gtk_widget_set_window_background (GtkWidget *widget,
					 GtkMode    mode);

Where @mode could have the special value GTK_MODE_DEFAULT
meaning "GTK_STATE_MODE (widget->state)".

gtk_widget_modify_fg(), gtk_widget_modify_bg() would be
changed to look like:

void gtk_widget_modify_fg     (GtkWidget    *widget,
			       GtkMode       mode,
			       GtkEventMode  event_mode,
			       GdkColor     *color);
void gtk_widget_modify_bg     (GtkWidget    *widget,
			       GtkMode       mode,
			       GtkEventMode  event_mode,
			       GdkColor     *color);
void gtk_widget_modify_all_fg (GtkWidget    *widget,
			       GtkMode       mode,
			       GdkColor     *color);
void gtk_widget_modify_all_bg (GtkWidget    *widget,
			       GtkMode       mode,
			       GdkColor     *color);

For modify_all_fg() and modify_all_bg(), only the color for
EVENT_MODE_NORMAL is provided, and the other colors are
calculated.


Another change is that gtk_widget_set_state() would be removed,
and it would be clarified that the state of a widget is
a computed value. To replace gtk_widget_set_state() would be:

void     gtk_widget_set_prelight (GtkWidget *widget,
				  gboolean   prelight);
gboolean gtk_widget_set_prelight (GtkWidget *widget);
void     gtk_widget_set_mode     (GtkWidget *widget,
				  GtkMode    mode);
GtkMode  gtk_widget_get_mode     (GtkWidget *widget);

(For gtk_widget_set_mode, we reuse the magic value
GTK_MODE_DEFAULT from above to mean "inherit from parent")


RC file syntax would obviously need changing as a result.

My proposal would be:

 # Set the entry background's for all event modes by calculation
 bg[BASE] = "blue"            

 # Override the inactive state with a specific color
 bg[BASE][INACTIVE] = "lightblue"

The current states NORMAL,SELECTED,ACTIVE,INSENSITIVE,PRELIGHT
could be handled reasonably OK with some aliasing:

 bg[ACTIVE] =    => bg[DEPRESSED]
 bg[PRELIGHT]    => bg[NORMAL][PRELIGHT]
 bg[INSENSITIVE] => bg[NORMAL][INSENSITIVE]
 

That's basically the API changes. So, what breaks in external
code?

 * Any code that accesses style->bg_gc[] directly will break
   at compile time.
  
   This is basically code that is using the GCs for drawing,
   and is usually trivial to fix up.

 * Code that is using gtk_widget_set_style() to modify
   appearance will most likely break at compile time,
   because of direct accesses to style->bg[].

   This is probably good, since such code should be using
   gtk_widget_modify_* instead.

 * Code that does tests on widget->state will most likely
   break; code I've seen that does this is usually pretty
   dubious to start with.

 * Code that is calling _paint() functions probably needs
   to be reevaluated to see what it _should_ be doing
   in the new framework.
  
   If we introduce #defines like:

     #define GTK_STATE_NORMAL (GTK_MODE_NORMAL | GTK_EVENT_MODE_NORMAL)

   We'll make getting such code compiling easier, but will
   probably make breakage of the previous type more obscure.

Theme engines and highly-tweaked custom widgets (e.g. gal)
will take the brunt of the breakage here. However, with the
changes we've already made in GTK+-2.0, theme engines
authors will already have to (and want to) make extensive
revisions.

To get a feel of what the above involves, I've attached a
(untested) diff to get GtkButton and GtkTreeView working
in this scheme. The widgets do fairly complex stuff with
drawing and widget->state, so most widgets will require
considerably less diffs. (GtkCList and GtkCTree look
rather nasty, however.)x


What would need doing in GTK+:

 * The scheme needs implementing in GtkStyle and GtkRCStyle.
   (1 long day?)

 * The above type of changes need to be made (about half a day?)

 * The new GtkIconFactory should probably be changed to separate
   out mode and event mode, and gtk_default_render_icon() will
   need work. (half a day?)

With debugging, unexpected things that turn up, and so
forth, we are probably talking about a week's worth of work
to get this solidly working.


So...

Pros:

 * Its a clean way to handle the inactive state.
 
 * We fix the other problems outlined at the start of the mail

 * If we don't fix this now, we can't fix it until GTK+-3.0.

Cons:

 * We break considerable amounts of external code. (The end 
   result is more functional and cleaner, but broken is broken.)

 * Putting another week of work into the schedule at this
   point is dangerous. It could basically be parallelized,
   but time to integrate parellized work is a short commodity
   these days.
   
Regards,
                                        Owen

Index: gtk/gtkbutton.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkbutton.c,v
retrieving revision 1.52
diff -u -r1.52 gtkbutton.c
--- gtk/gtkbutton.c	2001/02/28 19:07:44	1.52
+++ gtk/gtkbutton.c	2001/03/04 18:45:58
@@ -490,7 +490,7 @@
   gdk_window_set_user_data (widget->window, button);
 
   widget->style = gtk_style_attach (widget->style, widget->window);
-  gtk_style_set_background (widget->style, widget->window, GTK_STATE_NORMAL);
+  gtk_style_set_background (widget->style, widget->window, widget->state);
 }
 
 static void
@@ -610,7 +610,6 @@
 		  GdkRectangle *area)
 {
   GtkButton *button;
-  GtkShadowType shadow_type;
   gint width, height;
   gint x, y;
    
@@ -630,7 +629,7 @@
 	  GTK_BUTTON (widget)->relief == GTK_RELIEF_NORMAL)
 	{
 	  gtk_paint_box (widget->style, widget->window,
-			 GTK_STATE_NORMAL, GTK_SHADOW_IN,
+			 widget->state, GTK_SHADOW_IN,
 			 area, widget, "buttondefault",
 			 x, y, width, height);
 	}
@@ -653,17 +652,12 @@
 	  height -= 2;
 	}
 	
-      if (GTK_WIDGET_STATE (widget) == GTK_STATE_ACTIVE)
-	shadow_type = GTK_SHADOW_IN;
-      else
-	shadow_type = GTK_SHADOW_OUT;
-
       if ((button->relief != GTK_RELIEF_NONE) ||
-	  ((GTK_WIDGET_STATE(widget) != GTK_STATE_NORMAL) &&
-	   (GTK_WIDGET_STATE(widget) != GTK_STATE_INSENSITIVE)))
+	  button->in_button || button->down)
 	gtk_paint_box (widget->style, widget->window,
-		       GTK_WIDGET_STATE (widget),
-		       shadow_type, area, widget, "button",
+		       widget->state,
+		       button->down ? GTK_SHADOW_IN : GTK_SHADOW_OUT;
+		       area, widget, "button",
 		       x, y, width, height);
        
       if (GTK_WIDGET_HAS_FOCUS (widget))
@@ -826,30 +820,16 @@
 static void
 gtk_real_button_pressed (GtkButton *button)
 {
-  GtkStateType new_state;
-
-  g_return_if_fail (button != NULL);
-  g_return_if_fail (GTK_IS_BUTTON (button));
-
   button->button_down = TRUE;
 
-  new_state = (button->in_button ? GTK_STATE_ACTIVE : GTK_STATE_NORMAL);
-
-  if (GTK_WIDGET_STATE (button) != new_state)
-    {
-      gtk_widget_set_state (GTK_WIDGET (button), new_state);
-      gtk_widget_queue_draw (GTK_WIDGET (button));
-    }
+  gtk_widget_set_mode (GTK_WIDGET (button),
+		       button->in_button ? GTK_MODE_DEPRESSED : GTK_MODE_NORMAL);
+  gtk_widget_set_prelight (GTK_WIDGET (button), FALSE);
 }
 
 static void
 gtk_real_button_released (GtkButton *button)
 {
-  GtkStateType new_state;
-
-  g_return_if_fail (button != NULL);
-  g_return_if_fail (GTK_IS_BUTTON (button));
-
   if (button->button_down)
     {
       button->button_down = FALSE;
@@ -857,45 +837,21 @@
       if (button->in_button)
 	gtk_button_clicked (button);
 
-      new_state = (button->in_button ? GTK_STATE_PRELIGHT : GTK_STATE_NORMAL);
-
-      if (GTK_WIDGET_STATE (button) != new_state)
-	{
-	  gtk_widget_set_state (GTK_WIDGET (button), new_state);
-	  /* We _draw () instead of queue_draw so that if the operation
-	   * blocks, the label doesn't vanish.
-	   */
-	  gtk_widget_draw (GTK_WIDGET (button), NULL);
-	}
+      gtk_widget_set_mode (GTK_WIDGET (button), GTK_MODE_NORMAL);
+      gtk_widget_set_prelight (GTK_WIDGET (button), button->in_button);
     }
 }
 
 static void
 gtk_real_button_enter (GtkButton *button)
 {
-  GtkStateType new_state;
-
-  g_return_if_fail (button != NULL);
-  g_return_if_fail (GTK_IS_BUTTON (button));
-
-  new_state = (button->button_down ? GTK_STATE_ACTIVE : GTK_STATE_PRELIGHT);
-
-  if (GTK_WIDGET_STATE (button) != new_state)
-    {
-      gtk_widget_set_state (GTK_WIDGET (button), new_state);
-      gtk_widget_queue_draw (GTK_WIDGET (button));
-    }
+  if (!button->down)
+    gtk_widget_set_prelight (GTK_WIDGET (button), TRUE);
 }
 
 static void
 gtk_real_button_leave (GtkButton *button)
 {
-  g_return_if_fail (button != NULL);
-  g_return_if_fail (GTK_IS_BUTTON (button));
-
-  if (GTK_WIDGET_STATE (button) != GTK_STATE_NORMAL)
-    {
-      gtk_widget_set_state (GTK_WIDGET (button), GTK_STATE_NORMAL);
-      gtk_widget_queue_draw (GTK_WIDGET (button));
-    }
+  if (!button->down)
+    gtk_widget_set_prelight (GTK_WIDGET (button), FALSE);
 }
Index: gtk/gtktextdisplay.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktextdisplay.c,v
retrieving revision 1.21
diff -u -r1.21 gtktextdisplay.c
--- gtk/gtktextdisplay.c	2001/01/04 17:48:42	1.21
+++ gtk/gtktextdisplay.c	2001/03/04 18:46:14
@@ -250,7 +250,8 @@
       
       if (selected)
         {
-          fg_gc = render_state->widget->style->fg_gc[GTK_STATE_SELECTED];
+          fg_gc = gtk_widget_get_fg_gc (render_state->widget,
+					GTK_MODE_SELECTED);
         }
       else
         {
@@ -464,7 +465,11 @@
   PangoLayoutIter *iter;
   PangoRectangle layout_logical;
   int screen_width;
-  
+  GdkGC *selected_fg_gc = gtk_widget_get_fg_gc (render_state->widget,
+						GTK_MODE_SELECTED);
+  GdkGC *selected_bg_gc = gtk_widget_get_bg_gc (render_state->widget,
+						GTK_MODE_SELECTED);
+					    
   gboolean first = TRUE;
 
   iter = pango_layout_get_iter (layout);
@@ -517,7 +522,7 @@
           selection_end_index > line->length + byte_offset) /* All selected */
         {
           gdk_draw_rectangle (drawable,
-                              render_state->widget->style->bg_gc[GTK_STATE_SELECTED],
+                              selected_gc,
                               TRUE,
                               x + line_display->left_margin,
                               selection_y,
@@ -548,11 +553,11 @@
                                                           selection_height,
                                                           selection_start_index, selection_end_index);
 
-              gdk_gc_set_clip_region (render_state->widget->style->fg_gc [GTK_STATE_SELECTED], clip_region);
-              gdk_gc_set_clip_region (render_state->widget->style->bg_gc [GTK_STATE_SELECTED], clip_region);
+              gdk_gc_set_clip_region (selected_fg_gc, clip_region);
+              gdk_gc_set_clip_region (selected_bg_gc, clip_region);
 
               gdk_draw_rectangle (drawable,
-                                  render_state->widget->style->bg_gc[GTK_STATE_SELECTED],
+                                  selected_bg_gc,
                                   TRUE,
                                   x + PANGO_PIXELS (line_rect.x),
                                   selection_y,
@@ -564,8 +569,8 @@
                                   y + PANGO_PIXELS (baseline),
                                   TRUE);
 
-              gdk_gc_set_clip_region (render_state->widget->style->fg_gc [GTK_STATE_SELECTED], NULL);
-              gdk_gc_set_clip_region (render_state->widget->style->bg_gc [GTK_STATE_SELECTED], NULL);
+              gdk_gc_set_clip_region (selected_fg_gc, NULL);
+              gdk_gc_set_clip_region (selected_bg_gc, NULL);
 
               gdk_region_destroy (clip_region);
 
@@ -786,9 +791,10 @@
           GdkGC *gc;
 
           if (cursor->is_strong)
-            gc = widget->style->bg_gc[GTK_STATE_SELECTED];
+            gc = gtk_style_get_mode_bg_gc (render_state->widget, 
+					   GTK_MODE_SELECTED);
           else
-            gc = widget->style->fg_gc[GTK_STATE_NORMAL];
+            gc = gtk_widget_get_fg_gc (widget, GTK_MODE_DEFAULT);
 
           gdk_draw_line (drawable, gc,
                          line_display->x_offset + cursor->x - x_offset,
Index: gtk/gtktextview.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktextview.c,v
retrieving revision 1.70
diff -u -r1.70 gtktextview.c
--- gtk/gtktextview.c	2001/02/28 19:07:45	1.70
+++ gtk/gtktextview.c	2001/03/04 18:46:26
@@ -2674,8 +2674,7 @@
   /* must come before text_window_realize calls */
   widget->style = gtk_style_attach (widget->style, widget->window);
 
-  gdk_window_set_background (widget->window,
-                             &widget->style->bg[GTK_WIDGET_STATE (widget)]);
+  gtk_style_set_background (widget->window, widget->style, widget->state);
 
   text_window_realize (text_view->text_window, widget->window);
 
@@ -2751,27 +2750,33 @@
 
   if (GTK_WIDGET_REALIZED (widget))
     {
-      gdk_window_set_background (widget->window,
-                                 &widget->style->bg[GTK_WIDGET_STATE (widget)]);
-
-      gdk_window_set_background (text_view->text_window->bin_window,
-                                 &widget->style->base[GTK_WIDGET_STATE (widget)]);
+      gtk_widget_set_window_background (text_view,
+					widget->window,
+					GTK_MODE_DEFAULT);
+      
+      gtk_widget_set_window_background (text_view,
+					text_view->text_window->bin_window,
+					GTK_MODE_BASE);
 
       if (text_view->left_window)
-        gdk_window_set_background (text_view->left_window->bin_window,
-                                   &widget->style->bg[GTK_WIDGET_STATE (widget)]);
+	gtk_widget_set_window_background (text_view, 
+					  text_view->left_window->bin_window,
+					  GTK_MODE_DEFAULT);
       if (text_view->right_window)
-        gdk_window_set_background (text_view->right_window->bin_window,
-                                   &widget->style->bg[GTK_WIDGET_STATE (widget)]);
+	gtk_widget_set_window_background (text_view, 
+					  text_view->right_window->bin_window,
+					  GTK_MODE_DEFAULT);
 
       if (text_view->top_window)
-        gdk_window_set_background (text_view->top_window->bin_window,
-                                   &widget->style->bg[GTK_WIDGET_STATE (widget)]);
+	gtk_widget_set_window_background (text_view, 
+					  text_view->top_window->bin_window,
+					  GTK_MODE_DEFAULT);
 
       if (text_view->bottom_window)
-        gdk_window_set_background (text_view->bottom_window->bin_window,
-                                   &widget->style->bg[GTK_WIDGET_STATE (widget)]);
-
+	gtk_widget_set_window_background (text_view, 
+					  text_view->bottom_window->bin_window,
+					  GTK_MODE_DEFAULT);
+      
       gtk_text_view_set_attributes_from_style (text_view,
                                                text_view->layout->default_style,
                                                widget->style);
@@ -4063,12 +4068,18 @@
  */
 
 static void
-gtk_text_view_set_attributes_from_style (GtkTextView        *text_view,
+gtk_text_view_set_attributes_from_style (GtkTextView       *text_view,
                                          GtkTextAttributes *values,
-                                         GtkStyle           *style)
+                                         GtkStyle          *style)
 {
-  values->appearance.bg_color = style->base[GTK_STATE_NORMAL];
-  values->appearance.fg_color = style->fg[GTK_STATE_NORMAL];
+  gtk_style_get_bg (style,
+		    GTK_MODE_BASE,
+		    GTK_EVENT_MODE_NORMAL,
+		    &values->appearance.bg_color);
+  gtk_style_get_fg (style,
+		    GTK_MODE_BASE,
+		    GTK_EVENT_MODE_NORMAL,
+		    &values->appearance.fg_color);
 
   if (values->font.family_name)
     g_free (values->font.family_name);
@@ -5012,15 +5023,12 @@
 
       gtk_im_context_set_client_window (GTK_TEXT_VIEW (win->widget)->im_context,
                                         win->window);
-
 
-      gdk_window_set_background (win->bin_window,
-                                 &win->widget->style->base[GTK_WIDGET_STATE (win->widget)]);
+      gtk_widget_set_window_background (win->widget, win->bin_window, GTK_MODE_DEFAULT);
     }
   else
     {
-      gdk_window_set_background (win->bin_window,
-                                 &win->widget->style->bg[GTK_WIDGET_STATE (win->widget)]);
+      gtk_widget_set_window_background (win->widget, win->bin_window, GTK_MODE_DEFAULT);
     }
 
   g_object_set_qdata (G_OBJECT (win->window),


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]