Page MenuHome

template_list UI elements on Nodes don't resize correctly
Closed, ResolvedPublic

Description

System Information
Win7 x64

Blender Version
Broken: Blender 2.69 r60995
Worked: unknown

When a template_list is in a node, it greatly over-reacts to the user resizing it with the drag bar.

Exact steps for others to reproduce the error

  1. Create a custom node with a template_list UI element
  2. Resize the template_list

Demo:

Event Timeline

Tom Edwards (artfunkel) raised the priority of this task from to 90.
Tom Edwards (artfunkel) updated the task description. (Show Details)
Tom Edwards (artfunkel) edited a custom field.

I was going to report a bug about the list heights being shared between nodes, but that can fixed by making using the node's name as the template_list list_id value. :)

Yes, afaict the persistent data of the UILists is stored per area, meaning that all lists in the node editor main area with the same identifier will share their size. What happens then is that not only do all nodes show the same height, but when you change it in one instance this will be repeatedly applied to all the lists, leading to the observed over-reacting.

Will see if the redundant height increments can be avoided by flagging list data and applying only once, but ultimately you really want to set a unique identifier for each node showing a list. A simple way to do this would be by including the node name in the identifier:

layout.template_list("UI_UL_list", "foo_%s" % self.name, bpy.data, "objects", self, "active")

This could still be a problem when using the same node name in different trees shown at the same time though (happens by default with standard names), so to make it really unique it would need to include the node tree name as well. Even then the node tree name may not be unique either, if you have a more complex setup with node trees linked from multiple files, in which case you'd need to include the library path as well. The resulting string can be really long, so ultimately it may become necessary to turn the whole thing into a hash value, like so:

ntree = self.id_data
node = self

# tuples are hashable, collect all the various identifiers to make a unique value
node_id = str(hash( (ntree.library.filepath if ntree.library else "", ntree.name, node.name) ))
list_id = "foo_%s" % node_id
layout.template_list("UI_UL_list", list_id, bpy.data, "objects", self, "active")
Lukas Toenne (lukastoenne) lowered the priority of this task from 90 to 50.Dec 9 2013, 12:32 PM
Lukas Toenne (lukastoenne) changed the task status from Unknown Status to Invalid.Dec 9 2013, 12:49 PM

Talked to @Bastien Montagne (mont29) about it: using a unique identifier is really the only way to fix this. So this is not really a bug, more of a limitation with a nice way to avoid it :)

Makes sense, but I've found that the overreaction happens to exactly the same extent even when there is only one node in the whole blend. If this is about the resize operation being re-applied for each node then the bug should only appear when there is more than one node, right? I'm also seeing the bug in my current node tree even though each list ID includes the node's name.

Also, I don't understand the need to include tree and library names. Is there a way to show nodes from multiple trees in the same area? I can display the same node tree in two side-by-side areas and have different list heights (again using just node names in the list ID).

Hmm, that's a good point, i contradicted myself there. Using just the node name should actually be fine. Only exception to this case i can think of is if you allowed node groups containing themselves and thus can have the same tree open in one editor multiple times - but that's really exotic and not possible with most node designs, so wouldn't worry about that.

Lukas Toenne (lukastoenne) changed the task status from Invalid to Unknown Status.Dec 9 2013, 1:12 PM

And i confirm the issue with overreacting, might be caused by complicated DPI conversion in nodes or so ... reopening

Ok, so issue goes deep inside blender's UI roots…

Dev notes:
After talk with Lukas, and careful reading of code, here is the current situation, as far as I understand it:

  • uiBlocks do not have any positioning data in themself, only a width/height, and some transform matrix (generated by wm_subwindow_getmatrix()) that just seems to transform from window to sub-window (did not went deep here, but point is, all uiBlocks of a same region seem to have same winmat).
  • Consequently, ui_window_to_block_fl() (and its counter part ui_block_to_window_fl()) have misleading names, because they do not actually convert coords into a block local coordinate system (output coordinates remain relative to the subwindow, so that when you e.g. resize a node, this does not affect the mouse coordinates in the block).
  • But there is one exception: panels! For panels, those funcs use the panel's ofsx/ofsy values, and hence produce coordinates which are relative to the (bottom of the) panel, no more to the related sub-window.

This inconsistency causes this bug, as uiList needs to compare evolution of mouse in "block's coordinates", and the process to compute them differs for blocks inside panels and other blocks, it can't work in both cases (not unless using again ugly hacks like checking whether it is inside a panel and have special handling in this case).

So solutions I see so far:

  1. Make uiList check for being inside a panel or not, and have two different handling of mouse coords (ugly, but most "local" solution).
  2. Remove that panel-specific hack from ui_block_to_window_fl & co. Will completely break panel layout code, though.
  3. Move position data from panels/menus/nodes/etc. into uiBlocks, so that ui_block_to_window_fl & co actually always return coords in real block space.
  1. is probably the easiest to implement, but it’s adding a hack over a hack, I don’t like that…
  2. is the most appealing solution on a general view (I do not understand why a block would not store its position, this is generic data, should not need custom storage in higher-level "classes" like nodes, panels, whatever, imho). After all, I guess panel pointer was added to uiBlock precisely because of that? But this is most certainly not a simple change!

Anyway, I’m still far from being an expert in this complex UI code, so @Brecht Van Lommel (brecht), I really need your help here! :)

Bastien Montagne (mont29) lowered the priority of this task from 50 to Normal.Dec 9 2013, 4:45 PM

I don't think it's quite so complicated. For resizing of the UI list box only relative mouse coordinates matter, the difference between the start and end coordinates. So the panel (or node) offset should have no influence on this, it's all about the zoom level which should be included in the transform?

What I would do is look at ui_numedit_but_CURVE. The curve widget needs to do a similar thing, and what it seems to do is apply ui_window_to_block to data->draglastx and data->draglasty, and it stores the untransformed mouse coordinates in them.

There is one key difference between Curve widget and uiList: Curve editing never resizes its block, hence its container size and/or position remains the same during the whole drag. On the other hand, uiList dragging *does* resize its block, which in turns resize its panel or node.

Issue is a resized node does not change at all the coords returned by ui_window_to_block_fl(), while a resized panel does… I can hack around this in ui_do_but_LISTBOX, but Imho, this inconsistency should be removed one way or the other.

This just gave me another test idea: If I use an uiList in a panel it cannot resize (e.g. because a sibling column is already very long), what happens?

just add:

col = row.column()
for c in "ABCDEFGHIJKLMNOP":
    col.label("c")

right after e.g. row.template_list("MESH_UL_vgroups", "", ob, "vertex_groups", ob.vertex_groups, "active_index", rows=rows) (properties_data_mesh.py:199)… And we get again that "list growing out of control" issue.

I coded those lists for panels, so tested them here only… But in fact, this is a serious issue. I would consider ui_window_to_block_fl() to be broken, as it returns results that vary depending on what uses the block.

Ok, I'm pretty confused by this, the way this list resizing code works now is strange to me, rather than computing the delta from the starting point it keeps applying it every time? So then the resizing can get out of control quickly. Here's a quick patch that changes that behavior, which seems to solve this particular issue, though not sure it handles autoresize properly.

Bastien Montagne (mont29) changed the task status from Unknown Status to Resolved.Dec 10 2013, 4:23 PM

Closed by commit rBa1031d5dd529.

@Brecht Van Lommel (brecht), thanks a lot for the patch, it’s much better than what I had in thought for 1)…

Still, I find the behavior of ui_window_to_block() & co rather confusing, but I guess we have more serious issues to fix in UI…