Re: Height for width treeviews

[Tristan Van Berkom <tvb gnome org>]

On Fri, Jul 2, 2010 at 12:32 PM, Matthias Clasen
<matthias clasen gmail com> wrote:
> On Wed, Jun 30, 2010 at 12:16 PM, Tristan Van Berkom <tvb gnome org> wrote:
>> Greetings,
>>   Last couple weeks I've been working on the height-for-width
>> cell renderer api and now I have an intelligible branch for review:
>
> Some initial feedback, from playing with that branch for a few minutes:
>
> * I don't think your scrolledwindow cycle detection hack will work in
> general. What if the treeview is not a direct child of the
> scrolledwindow ? Eg there could be a vbox in between.
>
> * Do we need a similar hack for other 'async allocating widgets, like
> the text view ? Would it therefore not make more sense to have the
> hack in the scrolledwindow code ?

Yes we need to handle textview, and it would be better to handle in
scrolled window but: from every angle I look at it... handling it from scrolled
window is impossible with the current scrolled-window/child api.

  - The scrolled window queues a resize when the scrollbars toggle
    because of adjustment changes
  - The child sets the adjustment due to either allocation changes
    or content changes, sometimes synchronously and sometimes
    asynchronously
  - The scrolled window has no way to determine the origin of
    the adjustment changes

Because of all of this lack of context in scrolled-window, there is
no way to differentiate feedback loops from asynchronous appending
of large data pushing the vscrollbar... watching the adjustment
values as they come in and taking into account the previous
toggles of scrollbar visibility would also be unreliable.

And the last case evil hack to say... count to 1000 itterations
and break it from scrolled window is not going to work either,
it will :
 a.) introduce rendering glitches
 b.) have the same problem: it is valid to have 1000 sequential
      resizes queued for a treeview, while rows are being appended.

Surely though, an API with greater visibility and context will probably
fix the problem, for instance something could probably be done with
this:

Say the child has one signal to notify about both adjustment changes:
   void GtkScrollableIface::resized(GtkScrollable *child, enum
SizeChangeReason);

Where SizeChangeReason can be ContentChanged, AllocationChanged...

And a customized allocator method with more context, maybe:

  void GtkScrollableIface->allocate (GtkScrollable *scrollable,
GtkAllocation *full_allocation, GtkAllocation *canvas_allocation)

That would give the scrollable child context of where the canvas fits
in the scrolled window after the scrollbars are placed,
or maybe something like

  void GtkScrollableIface->allocate (GtkScrollable *scrollable,
GtkAllocation *full_allocation, enum AllocationReason)

Where the scrollable child can tell if it was allocated
"BecauseVScrollBarToggle" "BecauseHScrollBarToggle",
or "JustBecause"...

Whatever the solution is going to be, I seriously doubt there will be
any solution without creatively
thinking up a better api for the long term.. I did not take
time/consideration to invent such an
interface yet, but I suppose this patch will be blocking on that.

>
> * Toying with your 'wrapping treeview' demo, and making it as narrow
> as possible, I notice 'demonstrate' overlaps with the icon next cell.
> Shouldn't that line have determined the minimum width of that cell,
> preventing it from shrinking any further ?

I'll look further into this, its probable that I introduced
a minor bug in spacing/padding calculations.

Some refactoring around those allocations was needed here because the
column->width used to be padding/spacing inclusive (which does not make sense if
the column needs to use its width to position its renderers).

> * Looking at the list store example in gtk-demo, and sorting by
> clicking on the headers, there is an off-by-a-few-pixels delta
> between the header and the sort column background. The delta grows as
> you sort by columns further right, so I think this may be some missing
> per-column padding, or something.

Probably also related to afore mentioned refactor... I'll look into
that (these spacing bugs should at least be easy to fix compared
to scrolled window).

>
> * Open the file chooser in the pickers example for the first time, I
> notice the first 30 or so files getting a bigger height than the rest.
> Changing the sorting mixes these different-height rows into the others
> but doesn't fix their size. Changing directories back and forth does.
> I also notice that the name column gets too much width, and refuses to
> let me make it any smaller.
>
> * Using tests/testtreecolumns, and adding 2 columns to one of the
> windows, the column resizing is wonky. If I start dragging the
> separator, I have to go almost all the way to the right, then the
> column jumps. Once it jumped, it follows the pointer further to the
> right, but if I move too far left, it jumps back.
>

Ok I had not viewed those effects, fixing that will be a matter
of tracing the ever-so-stateful treeview structures during drag
resize operations under different configurations, I'll get on that
next chance I get.

Overall I guess the treeview stuff is not ready because of
   - scrolled window async children issue
   - some padding/spacing miscalculations
   - broken high-level treeview state in come cases that need more attention
     (I'll have to reproduce those last bugs you mentioned but I'm quite
     sure its a messed up treeview state that I have to trace).

Thanks for the feedback, I'll let you know as soon as I address the
more minor issues... in the meantime we should probably consider
what's going to be the best interface for scrollable children.

Cheers,
            -Tristan