Re: API Change request: gtk_paint_focus and GtkStyleClass->draw_focus

[Bill Haneman <bill haneman sun com>]

Owen Taylor wrote:
> 
> Bill Haneman <bill haneman sun com> writes:
> 
> > Hi All:
> >
> > I mentioned an API issue embedded in my outstanding theme patch for
> > GTK+ yesterday.  Here are the details.  [...] It requires the
> > following API change to gtk_paint_focus, and to the draw_focus
> > method of GtkStyleClass:
> >
> >
> > void
> > gtk_paint_focus (GtkStyle      *style,
> >                  GdkWindow     *window,
> >                  GdkRectangle  *area,
> >                  GtkWidget     *widget,
> > +              GtkStateType   state_type,
> >                  const gchar   *detail,
> >                  gint           x,
> >                  gint           y,
> >                  gint           width,
> >                  gint           height)
> 
> OK, thought about this some now. I see 2 ways of avoiding an API
> change here:
> 
>  * Just always use widget->state. The results really aren't bad;
>    we only have problems:
> 
>     - If we area drawing a "subwidget" that has a state
>       independent of the widget
>     - If the foreground in the state of the subwidget is
>       significantly different from foreground of the
>       state of the widget.
>     - If focus indication is necessary for the subwidget.
> 
>   Examples do exist - an example might be a black-and-white high-contrast
>   theme where selected rows of are drawn in black on white and other
>   rows white on black - but they certainly aren't common.

This is exactly the reason why we can't use widget->state, as detailed
in my original report.  The cases you mention above are very important
cases, not edge cases.  They are in fact predominant for high-contrast
themes.

The "uncommon" case you mention applies to all lists and treeviews in
GTK+-2.0 using high-contrast themes.


>  * A gross hack -- callers that need to paint with something
>    other than widget->state can do:
> 
>     GtkStateType saved_state = widget->state;
>     widget->state = subwidget_state;
>     gtk_paint_focus (style, window, area, widget, ...);
>     widget->state = saved_state;
> 
> We could also do this as a compatible API change:
> 
>  *  We add gtk_paint_focus_extended() and a paint_focus_extended()
>     virtual function to GtkStyle. (Default implementation of
>     paint_focus() calls paint_focus_extended().)
> 
>     [ This would be the route we'd take if we were making the same change
>     for GTK+-2.2. ]
> 
> > gtk_paint_focus is called 17 times in gtk+.  The only use I have found
> > outside gtk+ in gnome libraries is in gal/e-text, where it appears
> > twice.  It does not seem to appear in eel, gnomeui, or other UI libs.
> 
> Are you familiar with cvs.gnome.org/lxr/. The searching there is
> sometimes a little wonky, but in this case, the identifier search turns
> up uses in:
> 
>  ggv, gnomeicu, granite, gnome-applets, rp3, galeon, bonobo-config,
>  bonobo-conf, metacity

Hmm, OK, I haven't tried an lxr search before, thanks.

> In addition to gal and GTK+.
> 
> Of these only gnome-applets, bonobo-config, and metacity, are
> currently targetting GTK+-2.0. The relevant code in bonobo-config and
> gnome-applets is not currently compiled, so of things in CVS, only
> metacity actually breaks currently. (And on the vtable end,
> pixbuf-engine.) But it's certainly a real API change causing real
> incompatibilities.
> 
> Still, I can't really claim that any of the three solutions listed
> above actually benefits GTK+ users more than just going ahead and
> making the change, fixing up uses on CVS, and documenting it in the
> release announcement for 1.3.12. (Though they might be more gentle
> to my pride in being able to stick to an API freeze.)

It seems to me that we do need a change here, the API change to
gtk_paint_focus would be the most straightforward, and the only
alternative that strikes me as clean and reasonable would be to add API
for the subwidget case (maybe gtk_paint_subwidget_focus) and deprecate
the passing of a NULL widget to gtk_paint_focus.  Possibly this is worse
that the API change since developers are accustomed to the NULL widget
usage and might be surprised if it started throwing warnings.

-Bill
 
> So, unless people have convincing arguments otherwise, I'll go ahead
> and make the change.
> 
> Regards,
>                                         Owen
> 
> _______________________________________________
> gnome-2-0-list mailing list
> gnome-2-0-list gnome org
> http://mail.gnome.org/mailman/listinfo/gnome-2-0-list