[Bf-committers] uiList enhancements

[Bastien Montagne]

Sending again without the attachment (else it gets stuck in ML 
validation process :/ ).

-------- Original Message --------
Subject: 	Re: [Bf-committers] uiList enhancements
Date: 	Fri, 23 Aug 2013 15:56:38 +0200
From: 	Bastien Montagne <montagne29 at wanadoo.fr>
To: 	bf-blender developers <bf-committers at blender.org>



Hi Brecht,

So here are "final" new versions of the patches, addressing
issues/comments/suggestions from first review:

RNA patch: http://www.pasteall.org/45034/diff

Optional parameters for py callbacks: http://www.pasteall.org/45035/diff

New drag icon: http://www.pasteall.org/44832/diff

New dragresize code (with restored maxrows feature):
http://www.pasteall.org/45087/diff

New UIList filtering code (with enhanced UI):
http://www.pasteall.org/45088/diff
Note I kept +/- icons to show/hide filtering options for now (imho, it’s
also consistent with other parts of Blender, e.g. the show/hide of menus
in headers...).

Also attached all those guys in archive to this mail!

Kind regards,
Bastien

On 21/08/2013 22:36, Bastien Montagne wrote:
>  Hi Brecht,
>
>  Thanks for this first review, and sorry for late reply, but when I
>  re-applied the patches here I got a nasty segfault... Turned out to be a
>  stupid mistake in rna_UIList_filter_arrays_get_length(), which actually
>  got a FunctionRNA as PointerRNA, not the object (i.e. expected
>  PointerRNA of uiList here) - anyway, I did not need this func at all
>  (dynamic props are really confusing, esp. when used as func parameters
>  :/ ). Why this worked smooth a week ago will remain black magic to me,
>  esp. when using gcc's sanitizer...
>
>  So, here is an updated RNA-fixes patch, taking into account your remarks
>  as well as making new fixes for issues I found this evening:
>  http://www.pasteall.org/45034/diff
>
>  Also, here is a patch adding support for "optional" parameters in Python
>  callbacks (the idea is that all params after the first one using
>  PROP_PYFUNC_OPTIONAL flag are considered optional, and only passed to py
>  func if it actually "has room" for them in its parameter list). This
>  should allow much smoother evolution when making non-crucial changes in
>  the RNA API: http://www.pasteall.org/45035/diff
>
>  (Pacthes adding dragsize and dragicon remain the same for now:
>  http://www.pasteall.org/44831/diff and http://www.pasteall.org/44832/diff ).
>
>  And the corrected patch adding filtering to uiLists:
>  http://www.pasteall.org/45036/diff
>
>  And now the answers:
>
>  On 21/08/2013 17:14, Brecht Van Lommel wrote:
>>  Quick UI review, still need to dig into patches:
>>
>>  * The vertex groups panel seems to have two lists, doesn't make sense
>>  to me, was that for testing?
>  Yes (actually there are the three layouts, DEFAULT, GRID and COMPACT),
>  will obviously be removed before commit. ;)
>
>>  * Adding a vertex group gives a memory leak "Error: Not freed memory
>>  blocks: 2" "rna_ui.c:371 len: 4 0x7fc990c102c8"
>  Do not have this, but may be related to the bug I fixed in new patch?
>
>>  * The filter options don't look very polished to me. Some suggestions
>>  ** Replace (+) icon by a small magnifying glass? I also would expect
>>  in the right-bottom.
>  Yes, why not… but I'll have to add yet another icon.
>
>>  ** Filter by Name is cut off, I think if there's a magnifying glass
>>  that label can be left out.
>>  ** Put everything on a single line, make sorting options into an enum.
>>  ** I'm not sure if the invert options are needed, for sorting seems
>>  unnecessary, and for filter advantage is also unclear to me.
>>  ** For vertex groups I don't think Filter by Empty and Filter by Name
>>  should be mutually exclusive, think it could always show textbox and
>>  have option to show empty.
>  All this is actually defined in py code, so rather easy to tweak (in the
>  limits of UI API). I would keep invert options though (but perhaps as
>  toggle icons too, would take much less room), sometimes it's much easier
>  to say "I don't want this" rather than "I want that and that and...",
>  the same goes with filtering, imho.
>
>>  * There is too much padding between last item and gripper, like the
>>  gripper is a full row but doesn't need to be?
>  Yes, I noted that too, still don't know why, I would expect it to be the
>  height I give the two buttons here, UI_UNIT_Y * 0.4... Looks like a row
>  layout from a box one is always at least UI_UNIT_Y height?
>
>  As for maxrows, indeed I overlooked materials and textures (at least).
>  :/ Will add this back asap.
>
>>  On Thu, Aug 15, 2013 at 12:43 AM, Bastien Montagne
>>  <montagne29 at wanadoo.fr>    wrote:
>>>  So, first one fixes some bugs with PROP_DYNAMIC RNA parameters (for RNA
>>>  functions) - unless I completely misunderstood that piece of code, that
>>>  kind of props was not handled nicely!
>>>
>>>         http://www.pasteall.org/44830/diff
>>>  int size = 0
>>  Change this to size_t.
>>
>>>  /* Only for PROP_DYNAMIC properties! */
>>  Can you rename the functions to contain the word 'dynamic' to indicate this?
>>
>>>  * I decided to drop the "maxrows" option of list template, afaict it's
>>>  not used anywhere, and it added quite some unneeded complication to
>>>  template code (especially as we now support the same kind of features
>>>  for GRID layout as well).
>>  It's definitely used, it automatically makes lists expand a few rows
>>  when you add items. Keeps the lists compact when empty but not
>>  unreasonably small when it has items. That doesn't seem to be working
>>  anymore, I thought this was a nice feature.
>>
>>>  I put all the "API maintenance" edits to scripts in a separate patch:
>>>
>>>         http://www.pasteall.org/44835/diff
>>  As discussed on IRC, the extra draw_item parameter breaks
>>  compatibility, would be nice if that was made optional somehow.
>>  _______________________________________________
>>  Bf-committers mailing list
>>  Bf-committers at blender.org
>>  http://lists.blender.org/mailman/listinfo/bf-committers
>>
>  _______________________________________________
>  Bf-committers mailing list
>  Bf-committers at blender.org
>  http://lists.blender.org/mailman/listinfo/bf-committers