Re: window state stuff

[Havoc Pennington <hp redhat com>]

Tim Janik <timj gtk org> writes:  
> > struct _GdkEventWindowState
> > {
> >   GdkEventType type;
> >   GdkWindow *window;
> >   gint8 send_event;
> >   GdkWindowState changed;
> 
> is this what the state should be changed to, or has the state
> already changed to
>

It's the bits that have changed. i.e.:
  temp_event.changed = (unset_flags | set_flags) ^ old;
  
Could just have the old bits, but I think most code would end up 
writing something like:

 "if ((event->old_state ^ event->new_state) & GTK_WINDOW_STATE_ICONIFIED)"

and I think the xor isn't very intuitive, so the changed bits are
maybe more convenient than having the old state there.

> make this function public, progs like the tasklist prolly want to use it.
> also it should be named _gdk_wmspec_supports() since you actively use it
> for repetitive checks throughout the code.

By public I assume you mean gdkx.h, not gdk.h, since it is X-specific.
 
> you need to gdk_sync() before popping error traps, otherwise they don't
> help a single bit.

<pedantic>
Not for the GetWindowProperty() since it's a round-trip call anyhow,
at least other examples in GDK would so indicate. For XSelectInput()
yeah.
</pedantic>
 
> most importantly, you also need to set the hints so they are in place
> if the user fires up a _NET compliant wm during runtime.

Indeed. (Though it might be odd if you click the "maximize" button
with an old WM running, then upgrade to a new WM and suddenly your
apps maximize - I'm not sure what window managers should do with
preexisting state hints.)

> 
> > @@ -737,14 +809,36 @@
> >    g_return_if_fail (window != NULL);
> >  
> >    private = (GdkWindowObject*) window;
> > +
> > +  /* You can't simply unmap toplevel windows. */
> > +  switch (private->window_type)
> > +    {
> > +    case GDK_WINDOW_TOPLEVEL:
> > +    case GDK_WINDOW_DIALOG:
> > +    case GDK_WINDOW_TEMP: /* ? */
> > +      gdk_window_withdraw (window);
> 
> so here you withdraw...
> 
> > +      return;

withdraw and return. i.e. gdk_window_hide() on a toplevel just calls
gdk_window_withdraw() instead. It isn't correct to simply unmap a
toplevel, you can only withdraw it or request iconification.

> > +      if (GDK_WINDOW_IS_MAPPED (window))
> > +        gdk_synthesize_window_state (window,
> > +                                     0,
> > +                                     GDK_WINDOW_STATE_WITHDRAWN);
> 
> and send out wm state.

Only for child windows, we bailed out and went to
gdk_window_withdraw() for toplevels.

Somewhat oddly, the GDK_WINDOW_STATE_WITHDRAWN flag is different for
child windows and toplevels. For toplevels it indicates
withdrawn/notwithdrawn; for child windows it indicates
mapped/unmapped. The preexisting GdkWindow::mapped flag also had these
two different meanings, so I didn't introduce that, but it might be
somewhat confusing to people.

Note this code/comment in gdk_synthesize_window_state():

  /* We only really send the event to toplevels, since
   * all the window states don't apply to non-toplevels.
   * Non-toplevels do use the GDK_WINDOW_STATE_WITHDRAWN flag
   * internally so we needed to update window->state.
   */
  switch (((GdkWindowObject*)window)->window_type)
    {
    case GDK_WINDOW_TOPLEVEL:
    case GDK_WINDOW_DIALOG:
    case GDK_WINDOW_TEMP: /* ? */
      gdk_event_put ((GdkEvent*)&temp_event);
      break;
      
    case GDK_WINDOW_FOREIGN:
    case GDK_WINDOW_ROOT:
    case GDK_WINDOW_CHILD:
      break;
    }

So WITHDRAWN is really an internal-use-only flag when applied to child
windows.

> > +  /* This is a bad hack to set the window background. */
> >    gtk_window_paint (widget, NULL);
> 
> could you elaborate on _why_ this is a bad hack?

Because you are calling _paint() for the unexpected side effect of
setting the window background. According to Owen it's some weird
rastercode thing. Anyway, any code that involves calling a _paint()
method on an invisible window is a bad hack in my book. ;-)
 
The point of the comment though is "this sets the window background,"
rather than "this is a bad hack," since otherwise one might be tempted
to remove the function call there.

> this doesn't seem right, you should first gtk_widget_show() the widget,
> then, do the above gdk_window_ ops.

If the widget isn't showing already, then there's no need to do the
other stuff, since initially-shown windows will naturally get the
focus and so on.
 
> i think having present_initially : 1; wouldn't be a too bad idea.
> 

The theory is that when initially mapped, a window is "naturally"
presented, that is, most WMs will initially map a window focused, on
the current desktop, etc., or at least if they don't it's because the
user requested otherwise. By default most of them do. So if you have
an unmapped window gtk_window_present() is equivalent to
gtk_window_show(), and there's no need to present() it.

I suppose we could actually write gtk_window_present() like this and
it would be clearer:

void
gtk_window_present (GtkWindow *window)
{
  GtkWidget *widget;

  g_return_if_fail (GTK_IS_WINDOW (window));

  widget = GTK_WIDGET (window);

  if (GTK_WIDGET_VISIBLE (widget))
    {
      gdk_window_show (widget->window);
      gdk_window_focus (widget->window,
                        gtk_get_current_event_time ());


      /* FIXME add desktop-moving stuff from WM spec */
    }
  else
    {
      gtk_widget_show (widget);
    }
}

Havoc