Re: Yet another scrolling menus patch.

[Owen Taylor <otaylor redhat com>]

Alexander Larsson <alla lysator liu se> writes:

> >  - The blanked-out area is less odd-looking for option menus, but
> >    seems a little odd to me. Having the text move over the background
> >    without any bevelling doesn't give a very natural sense of
> >    scrolling.
> > 
> >    One idea I had was to do something like the mockup in:
> >   
> >     http://people.redhat.com/otaylor/tmp/menu-scroll.png
> > 
> >    The idea here is to make it a little more obvious that the
> >    blank area is off-the-edge of the menu, while still leaving
> >    the outer frame in the same place to provide visual stability
> >    and a big target for the mouse to hit. 
> 
> Yes, that looks good. I didn't understand what it depicted at first, but
> now I see.

Well, experiment with it - it was just an idea I had.

[...] 

> > > There is some flashing redraw going on when you pop down a menu and there
> > > is a torn of menu visible. I don't know how to fix this. The old way where
> > > you call gtk_menu_reparent with unrealize==true is broken. If there are
> > > torn off submenus of the torn off menu they're unrealized when their  
> > > parent menu-item is unrealized as part of the gtk_menu_reparent, and they
> > > never get realized again.
> > 
> > I don't quite understand the point here; unrealize = TRUE was
> > only ever used for moving the menu from the tearoff to the 
> > real menu, and unrealize = FALSE was always used for the other
> > way.
> 
> Yes. I had to remove the usage of unrealize = TRUE for moving the tearoff
> from the tearoff to the real menu beacuse of this chain:
> 
> gtk_menu_reparent() ->
> gtk_container_remove (GTK_CONTAINER (widget->parent), widget) -> 
>  (widget here is the GtkMenu, and parent is the GtkWindow)
> gtk_widget_unparent (widget) ->
> gtk_widget_unrealize (widget) ->
> gtk_widget_real_unrealize (widget) ->
> gtk_container_forall (GTK_CONTAINER (widget),
> 		  (GtkCallback) gtk_widget_unrealize,
> 		  NULL) ->
> gtk_menu_shell_forall (widget, TRUE, gtk_widget_unrealize) ->
> gtk_widget_real_unrealize (menuitem) ->
> gtk_container_forall (GTK_CONTAINER (menuitem),
> 		  (GtkCallback) gtk_widget_unrealize,
> 		  NULL) ->
> gtk_menu_item_forall (menuitem, TRUE, 
>                       (GtkCallback) gtk_widget_unrealize,
>                       NULL) ->
>   if (include_internals && menu_item->submenu)
>     (* callback) (menu_item->submenu, callback_data) ->

This is just wrong; gtk_widget_forall() iterates over the
widget heirarchy.

I think this was a misguided attempt to make gtk_widget_show_all()
automatically propagate from window to attached menus.

But gtk_widget_show_all() is _already_ virtualized, and implemented
to do that in gtk_menu_item_show_all(), so this is unecessary.

Its also possible reason that this might have a been an attempt was to
be able to iterate over _all_ widgets starting from only the GtkWindow
widgets - but since GtkMenu is now contained in a GtkWindow, so that
certainly isn't necessary.

Could you try removing it and see if anything breaks?

> gtk_widget_unrealize(submenu)
> 
> This means that any torn of submenu of the reparented menu is unrealized
> while it is shown, which is pretty nasty. This bug probably exists in Gtk+
> 1.2 too.

Yes, probably; it's not like you normally have tearoff submenus...
 
> The thing was that removing the usage of unrealize == TRUE while moving
> from the tear off to the normal window did not reintroduce any
> flickering! All flickering i saw was when moving in the other director.

Well, double buffering lowers flicker a lot, though I can't
identify exactly what flicker I was trying to avoid. My best guess
at this point was that an unecessary call to gtk_widget_queue_draw()
was being done so that the menu was being drawn twice once 
it was reparented into the new position.

Either double-buffering or the vastly improved expose/draw queuing
code has most likely fixed that problem.
  
If you want to fool around with GTK+-1.2 you could probably find
what the problem was, but that doesn't sound too exciting.

> > I believe the flicker you are seeing here is exactly the same 
> > flicker that GtkMenu always had, but just more obvious because
> > you are testing it with bigger menus and more complicated geometries.

> I don't claim to understand why this affects flickering, but using
> unrealize == TRUE while moving from the normal menu to the tearoff did
> remove the redraw flicker you normaly get on that operation. 

I don't want to admit it, but it took me a good hour to figure
out why it _doesn't_ flicker. (Not that I'm complaining)

Its basically a coincidence...

What happens is that when the menu is unrealized and then re-realized,
the allocations are as follows:

 parent - 1x1 at (0,0) 
 child1 - 100x20 at (0,0)
 child2 - 100x20 at (0,20)
 child3 - 100x20 at (0,40)

That is, the parent is small but the children are full sized. Then,
when the queued_resize gets processed, the parent gets resized
to full size. 

But in order to eliminate flicker when scrolling, gdkgeometry-x11.c
contains the following logic:

 - if a move or resize operation on a window would change the clip 
   region on the children, then before the window is resized
   the background for children is temporarily set to None, the
   move/resize done, and the background for the children restored.

So, at the point where the parent is resized to final size, the
background for the children is temporarily None, and thus they
are not cleared to the background color and the previous background
(the image of the menu) is left in place.

Then the redraw happens, but everything except for the upper left
hand pixel is already perfectly fine, so no flicker is seen.

>From this explanation, I believe if you stare really hard at the upper
left hand pixel at just the right moment, you might see a flicker, but
this is probably completely unobservable in practice.

With this handy coincidence in mind, I think that the right thing to
do is to fix gtk_menu_shell_forall(), eliminate the unrealize parent
to gtk_menu_reparent and always unrealize.

> It had other
> strange effects that i didn't explore further though, as i suppose they
> are the same as the above chain.

I believe so.
                                        Owen