search

Klortho / d3-flextree

Flexible tree layout algorithm that allows for variable node sizes
https://klortho.github.io/d3-flextree/
Do What The F*ck You Want To Public License
327 stars 45 forks source link

layout bug #1

Closed Klortho closed 6 years ago

Klortho commented 9 years ago

Discovered a bug in the layout algorithm -- see this issue. I added test trees grandkids1.json and grandkids2.json to illustrate it.

Klortho commented 9 years ago

See this patch from @lianyi. It seems to fix this bug, but causes test19 to fail.

I'm working in

Klortho commented 9 years ago

The differences for test19 were very small, and seemed, possibly, to be due to the same bug, but on a node deeper in the tree. I'm not 100% sure, but going cross-eyed looking at it. So, I pulled the change: https://github.com/Klortho/d3/pull/1.

Klortho commented 6 years ago

The bug is fixed for this tree:

[ 40, 40,
  [ 40, 40 ],
  [ 40, 40,
    [ 100, 40 ],
    [ 200, 40 ],
  ],
],

Here's before and after:

tree-a-before tree-a-after

But still exists for this tree:

[ 40, 40,
  [ 40, 40 ],
  [ 40, 40,
    [ 40, 40,
      [ 100, 40 ],
      [ 200, 40 ],
    ],
  ],
],
tree-b-before
jiakuan commented 6 years ago

I saw the same bug as well. The algorithm only moves subtree from the second index, that means the first child will never be moved if it doesn't have children.

As an improvement to the algorithm, perhaps we could add a special check for each first leaf child and move the first child instead?

Klortho commented 6 years ago

This is fixed in the just-released version (2.0.0)