Re: GNOME CVS: gtk+ timj

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj@gtk.org> writes:

> On 12 Jan 1999, Owen Taylor wrote:
> 
> > 
> > Gnome CVS User <gnomecvs@redhat.com> writes:
> > 
> > [....]
> > 
> > > Log message:
> > > Tue Jan 12 13:47:07 1999  Tim Janik  <timj@gtk.org>
> > > 
> > > * reworked the redrawing heuristics somewhat, this fixed a bunch of
> > > existing redrawing problems and majorly reduces overall redrawing needs
> > > during normal operation. basically we now only queue redraws when
> > > neccessary and much rely on the draw_area coalescing code in gtkwidget.c
> > > to optimize the queued portions. widgets will now upon reallocation only
> > > get redrawed if their allocation has changed. upon hide/show only the
> > > area allocated by the child will be queued for the parent, this has the
> > > side effect that parents which change their appearance in dependance on
> > > the numer of visible children have to keep track of their children's
> > > visiblity and eventually fully redraw themselves. this is a minor
> > > constrain with great benefits in terms of redraw reduction, and only got
> > > triggered by the notebook widget.
> > > 
> > > * gtk/gtkwidget.c:
> > > (gtk_widget_queue_clear): don't bother if width and height == 0.
> > > (gtk_widget_queue_clear_child): new static function to queue a redraw of
> > > the area obscured by a child on a parent.
> > 
> > > (gtk_widget_queue_resize): queue_clear the widget if it is drawable.
> > > (gtk_widget_show): queue resize on the widget before showing.
> > > (gtk_widget_hide): queue resize on the widget after hiding.
> > > (gtk_widget_map): queue_draw the widget after mapping.
> > > (gtk_widget_unmap): queue_clear_child the widget.
> > > (gtk_widget_size_allocate): queue_clear_child and queue_draw if the
> > > widget's allocation changed.
> > > (gtk_widget_unparent): queue_clear_child so the parent redraws obscured
> > > portions.
> > 
> > Big problem here, you're missing a whole lot of
> > tests for NO_WINDOW widgets. So GTK will be doing a 
> > whole lot of extra work
> 
> can you please be more specific? all of the above cases require redraws
> on either the widgets or the parents, if areas get queued multiple times
> the'll later get coalesced.

There is one type of redraw that won't get coelesced - and
that is an expose from the X server. If I remove a WINDOW widget,
or move it, the X server will generate expose events for
the newly exposed areas. 

If I map a !NO_WINDOW widget, I don't have to queue a draw
on it, because the server will expose the new window,
etc, etc.

(And also, we shouldn't be relying too much on the draw
 combining code. Although it does a pretty good job, it
 isn't blazingly fast.)
 
> > > (gtk_widget_real_show):
> > > (gtk_widget_real_hide):
> > > (gtk_widget_real_map):
> > > (gtk_widget_real_unmap):
> > > (gtk_widget_real_size_allocate): don't bother with redraw queueing,
> > > descendants that override these functions don't do either and we handle
> > > all redrawing/resizing related stuff before or after the signal emission
> > > now.
> > > 
> > > * gtk/gtkcontainer.c:
> > > (gtk_container_resize_children): don't bother about redrawing anymore
> > > since gtk_widget_size_allocate handles that for us now.
> > 
> > Another big problem - haven't tested this yet, but it looks
> > like no redraw is going to be queued if the size didn't change.
> > 
> > Right now the guarantee is that if a widget queues a resize,
> > it will eventually get a redraw. I'm sure we are going
> > to trigger bugs somewhere by removing that guarantee.
> 
> that guarrantee is taken care off by gtk_widget_queue_resize doing a
> queue_clear on the widget.

Hmmm, there seems to be a problem - since we queue_clear() there,
we are already saying "redraw the current allocation of 
the widget", but in size_allocate we do:

  if (GTK_WIDGET_NO_WINDOW (widget))
    {
      if (widget->allocation.x != real_allocation.x ||
	  widget->allocation.y != real_allocation.y ||
	  widget->allocation.width != real_allocation.width ||
	  widget->allocation.height != real_allocation.height)
	{
	  gtk_widget_queue_clear_child (widget);
	  needs_draw = TRUE;
	}
    }

Which also redraws the old allocation of the widget.
This looks very much like it needs to be a queue_draw()
since otherwise we'll redraw the old allocation of the
widget twice, and the new allocation (if it moved)
never.

[ ... Thanks for the explanation of the togglebutton.c changes ]

                                        Owen