Re: partial window position/size patch

[Havoc Pennington <hp redhat com>]

Tim Janik <timj gtk org> writes:
> 
> i'm not sure which 2 possibilities you mean. granted, not all possibilities
> are usable on a day-to-day basis, but that doesn't mean we should get
> completely rid of them. that said, i don't really object to not recommend
> usage of this function (at least for normal dialogs) but i'm definitely
> opposed to deprecate it in the sense of "this function is scheduled for
> removal in a future version".
>

The basic meaning of "deprecated" is "do not use in new code," rather
than "scheduled for removal" - we would only remove things that
genuinely fall out of use I think.

So, the 2 possibilities I mean are:
 a) window is user-resizable, within geometry constraints. 
 b) window has a size fixed by the program (is locked to the
    requisition, basically, where set_usize() may be used to 
    change the requisition)

In case a):
  - auto shrink is evil, it fights the user
  - allow grow is required to make resizing possible
  - allow shrink is evil, the min size should be the requisition,
    allowing shrinking to 1x1 is just broken

In case b):
  - auto shrink locks the window to the requisition, which is correct
  - allow grow allows resizing, and is wrong
  - allow shrink is still evil

Right now, most uses of gtk_window_set_policy() are broken - it seems
that people pick the parameters to this function randomly. I can't
think of a single use for any combination of parameters other than the
above two combinations. So set_policy() should never have existed, it
just confuses people and makes them set one of the 6 broken policies.

What other policies do you think are useful?
 
> i'd go to the extend to fully remove this. for 1.2 we left it in place
> because we had no wm-spec back then, and ENLIGHTENMENT_DESKTOP was the
> only property that could indicate a virtual root window.
> with the new wm-spec at hand, that logic should move into
> gdk_window_get_root_origin() (unfortunately the new wm-spec doesn't
> specify a _NET_VIRTUAL_ROOT property on those windows, but it comes
> with a _NET_VIRTUAL_ROOTS list of virtual root window IDs that can be
> used instead).

There are two approaches possible to virtual roots:

 - pretend that the "root window" origin is at the virtual root
   origin; everywhere that we set a size or position, interpret
   those as relative to the virtual root instead of the real 
   root 

 - say that the root window is a distinct concept from the currently 
   visible desktop screen area, so that the visible screen 
   does not necessarily begin at 0,0

The property we really have to preserve for set_location(), set_size()
etc. is that get_location() and get_size() retrieve the same values
that set_location() and set_size() set. That is, in the same
coordinate system and referring to the same reference point on the
window.

The crappy thing about these virtual root window managers is that they
don't preserve this. Because they can redirect ConfigureRequest they
make the "setter" set different coordinates, but they can't redirect
the getter. i.e. if you XMoveWindow() is honored, and you then get the
window position, it isn't at the coordinates you requested.

GDK needs to hide this inconsistent/broken behavior.

The first solution mentioned above means making the change you
mentioned to gdk_window_get_root_origin().

The second solution is implemented as gdk_get_workspace_extents()
which gets the screen rectangle, and this should normally be used
instead of: 0, 0, gdk_screen_width (), gdk_screen_height ().

I think we had some reason to prefer the second solution, but I don't
remember it.

One issue is that we can't really hide the virtual roots entirely,
since gdk_root_window is actually the root window, and we need to
expose that, so it doesn't really make sense to say "OK, for this one
get_origin() function we really use the virtual root not the real
root," but then in many other cases we use the real root.

These virtual root window managers are broken and ICCCM-incompliant
anyhow. ;-)
 
> without evaluating _NET_VIRTUAL_ROOTS this will yield bogus
> results for window managers using virtual root windows.

Right.
 
> you shouldn't change the window size behind gtk_window_move_resize()'
> back, the default size and changes thereof are taken care of by that
> function, so just leave gtk_widget_queue_resize() here.

Discussed on IRC.

> 
> >  }
> >    
> >  static void
> > @@ -2324,10 +2341,10 @@
> >    /* From the default size and the allocation, figure out the size
> >     * the window should be.
> >     */
> > -  if (!default_size_changed ||
> > -      (!window->auto_shrink &&
> > -       new_width <= widget->allocation.width &&
> > -       new_height <= widget->allocation.height))
> > +  if (!window->auto_shrink &&
> > +      (!default_size_changed ||
> > +       (new_width <= widget->allocation.width &&
> > +        new_height <= widget->allocation.height)))
> >      {
> >        new_width = widget->allocation.width;
> >        new_height = widget->allocation.height;
> 
> what? this is completely bogus, with your code auto_shrink means
> to always keep the old size. you break auto_shrink with this.
> there's nothing wrong with the old version, so leave it.

The problem is that you need to allow set_default_size() to shrink the
window after mapping, which is broken at the moment.

The code looks like it handles that case, but it doesn't, because
default_size_changed doesn't mean that set_default_size() was called,
it means that the computed value for the default size (which may be
the requisition or the size from set_default_size) has changed.

So the original code means:
 if (requisition has not changed or 
     (window doesn't autoshrink and the new size is smaller))
     keep the current size

it should be I guess:

 if (requisition has not changed or 
     ((window doesn't autoshrink and the new size is smaller)
       and set_default_size() has not been called))
     keep the current size

So I think we need to add a way to tell whether set_default_size() has
been called.
 
> i don't think auto_shrink=TRUE is a good policy for non-resizable
> windows. 

It certainly isn't a good policy for resizable windows either, so
let's just nuke it if you feel that way. ;-)
 
> i don't think #ifndef GTK_DISABLE_DEPRECATED is apprpriate here
> at all.

Sure, we don't want two non-deprecated functions that do the same
thing. If we have set_size() then set_default_size() isn't required.
Deprecated simply means "do not use in new code." We can deprecate
widely-used functions if they are considered less good than some other
way to do the same thing.

Basically the claim is that set_default_size() should always simply
mean "emulate a user resize," both before and after mapping, so it
should simply be called set_size().

> also this doesn't account for our -2/-1/0 magic problems in
> set_uposition/set_usize/set_default_size, i'd first like to see
> a rationalized API intention discussion for all these before we
> actually touch the resizing code.

My suggestion is:
 
 a) -1 means "unset this value" (use requisition, don't specify a
    position)
 b) to avoid changing a value, just call get() then reset it to 
    the current value, or use object properties to set only one value
 c) 0 is not allowed for sizes

However, I don't think we should change existing functions, it would
break things, instead we should just use these semantics for new
functions.

Havoc