template_list UI elements on Nodes don't resize correctly #37739

New Issue

Tom Edwards · 2013-12-08T18:09:13+01:00

Tom Edwards commented

2013-12-08 18:09:13 +01:00

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

Create a custom node with a template_list UI element
Resize the template_list

Demo: node_list_bugs.blend

**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** - Create a custom node with a template_list UI element - Resize the template_list Demo: [node_list_bugs.blend](https://archive.blender.org/developer/F37535/node_list_bugs.blend)

Tom Edwards commented

2013-12-08 18:09:13 +01:00

Changed status to: 'Open'

Lukas Tönne was assigned by Tom Edwards

2013-12-08 18:09:13 +01:00

Tom Edwards commented

2013-12-08 18:09:13 +01:00

Added subscriber: @artfunkel

Tom Edwards commented

2013-12-08 18:10:24 +01:00

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. :)

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. :)

Lukas Tönne commented

2013-12-09 12:31:23 +01:00

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")

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 Tönne commented

2013-12-09 12:35:36 +01:00

Added subscriber: @mont29

Lukas Tönne commented

2013-12-09 12:49:52 +01:00

Changed status from 'Open' to: 'Archived'

Lukas Tönne closed this issue

2013-12-09 12:49:52 +01:00

Lukas Tönne commented

2013-12-09 12:49:52 +01:00

Talked to @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 :)

Tom Edwards commented

2013-12-09 12:58:39 +01:00

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).

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).

Lukas Tönne commented

2013-12-09 13:09:40 +01:00

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 Tönne commented

2013-12-09 13:12:56 +01:00

Changed status from 'Archived' to: 'Open'

Lukas Tönne reopened this issue

2013-12-09 13:12:56 +01:00

Lukas Tönne commented

2013-12-09 13:12:56 +01:00

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

Lukas Tönne removed their assignment 2013-12-09 15:10:07 +01:00

Bastien Montagne was assigned by Lukas Tönne

2013-12-09 15:10:07 +01:00

Bastien Montagne commented

2013-12-09 16:45:17 +01:00

Added subscriber: @brecht

Bastien Montagne commented

2013-12-09 16:45:17 +01:00

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:

Make uiList check for being inside a panel or not, and have two different handling of mouse coords (ugly, but most "local" solution).
Remove that panel-specific hack from ui_block_to_window_fl & co. Will completely break panel layout code, though.
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.
is probably the easiest to implement, but it’s adding a hack over a hack, I don’t like that…
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, I really need your help here! :)

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… 3) 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, I really need your help here! :)

Brecht Van Lommel commented

2013-12-09 18:53:36 +01:00

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.

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.

Bastien Montagne commented

2013-12-09 19:02:55 +01:00

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.

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.

Bastien Montagne commented

2013-12-09 19:11:57 +01:00

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.

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.

Brecht Van Lommel commented

2013-12-09 20:12:35 +01:00

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.

list_resize_dragstart.patch

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. [list_resize_dragstart.patch](https://archive.blender.org/developer/F37944/list_resize_dragstart.patch)

Bastien Montagne commented

2013-12-10 16:23:07 +01:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2013-12-10 16:23:07 +01:00

Bastien Montagne commented

2013-12-10 16:23:07 +01:00

Closed by commit a1031d5dd5.

Closed by commit a1031d5dd5.

blender-admin commented

2013-12-10 16:23:16 +01:00

This issue was referenced by blender/blender-addons-contrib@a1031d5dd5

This issue was referenced by blender/blender-addons-contrib@a1031d5dd529885fea53b0908a912585d65c4567

blender-admin commented

2013-12-10 16:23:16 +01:00

This issue was referenced by a1031d5dd5

This issue was referenced by a1031d5dd529885fea53b0908a912585d65c4567

Bastien Montagne commented

2013-12-10 16:25:33 +01:00

@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…

@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…

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

template_list UI elements on Nodes don't resize correctly #37739

tuples are hashable, collect all the various identifiers to make a unique value