Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Squarify algorithm unresponsive to changed aspect ratio. #45

Closed
jheer opened this issue Jun 2, 2016 · 8 comments
Closed

Squarify algorithm unresponsive to changed aspect ratio. #45

jheer opened this issue Jun 2, 2016 · 8 comments
Assignees

Comments

@jheer
Copy link

jheer commented Jun 2, 2016

When a squarified layout is first calculated, partition decisions are cached on the tree nodes. If one then updates treemap.tile to use a squarified layout with a different target aspect ratio, this new ratio is effectively ignored in favor of the cached data. While stable layout is valuable in many situations, it is not always desired. In the case of assigning a new tiling method, my expectation was that this assignment would (by default) override any cached results. Perhaps the API could include a mechanism to make this determination user controllable.

This issue affects changes to the treemap.size parameter as well. Sometimes one may want the treemap to re-partition on changes of width/height rather than stay stable.

For now, it appears that the resolution is to either generate a new hierarchy instance or walk the existing hierarchy and clear out the _squarify caches.

@mbostock
Copy link
Member

mbostock commented Jun 2, 2016

This is the expected behavior, but I agree it should at least be better documented. If you don’t want a stable update, you can use root.copy to create a fresh hierarchy and pass that to the treemap.

@mbostock
Copy link
Member

mbostock commented Jun 2, 2016

Some possible API improvements:

  1. Make the cached layout (_squarify) specific to the aspect ratio, such that if the aspect ratio changes, a new layout is computed from scratch. This seems reasonable, although a little magical.
  2. Make opting out of the stable update more obvious, say d3.treemapSquarify.stable(false). I don’t know if this is significantly better than just improving the API documentation, however.
  3. Make opting in to the stable update explicit, say by exposing d3.treemapResquarify as a tiling algorithm (that requires the input to have been previously computed using d3.treemapResquarify… or possibly d3.treemapSquarify?).
  4. Generalize stable updates to the treemap (Generalize stable treemaps. #36), i.e., like the previous approach, you might use d3.treemapSquarify to compute the initial layout, and then d3.treemapStable to update that layout with new values. The stable update wouldn’t rely on hidden fields (_squarify); it would automatically determine topology of the previous layout and preserve it on update.

@mbostock mbostock self-assigned this Jun 2, 2016
@jheer
Copy link
Author

jheer commented Jun 2, 2016

Generalized stable updates sounds quite attractive! In that case, might the top-level treemap method accept a stable parameter to opt-in or opt-out of stable updates? (Not unlike sticky in D3 v3, but all the more reasonable if shared across tiling methods.) From the perspective of higher-level tools built on top of D3, it would be expedient to be able to parameterize the treemap once and have it behave appropriately, rather than swapping tiling methods around for the sake of stable layout.

One additional benefit might be some memory savings for very large trees. When stable === false the layout need not generate and cache partition decisions. However, one must consider the case when toggling the stable value. If toggled from false to true, should the layout cache the current layout or only future layout runs? I find the latter is reasonable (and simpler to implement).

mbostock added a commit that referenced this issue Jun 2, 2016
@mbostock
Copy link
Member

mbostock commented Jun 2, 2016

I’ve implemented the third proposal in #46. Care to take a look?

For generalized stable update, I was imagining that you’d use d3.treemapStable to update an existing treemap’s layout, rather than needing to set tile.stable(true) on each tiling function. The problem with the latter approach is that it requires each tiling function to expose a stable parameter, and given that it’s easy for people to writing tiling functions outside the library, it’d be better to centralize the stable update logic.

I suppose another way to do it, though, would be to set treemap.stable(true), which could override the treemap’s tiling function (treemap.tile) and instead use treemapStable, which could then just be a private internal function. (Or it could still be public… it doesn’t matter.) Though then you’d need some additional logic to detect whether a layout was previously computed, but that could potentially be as simple as checking whether node.x0 is defined.

Anyhow, a generalized stable update is still theoretical at this point. I took a brief crack at it previously and wasn’t successful, though I still think it should be possible.

The proposed implementation in #46 avoids saving the partition decisions if you use d3.treemapSquarify rather than d3.treemapResquarify. (Though the objects are created internally and then discarded to simplify the implementation. I could optimize that, I suppose, but it’d make the logic a bit more complicated.)

@jheer
Copy link
Author

jheer commented Jun 2, 2016

Thanks! #46 is definitely an improvement, and a good working solution in the absence of generalized stable updates.

@mbostock
Copy link
Member

mbostock commented Jun 2, 2016

Thanks for the feedback. I’ll push a new patch release shortly.

@mbostock
Copy link
Member

mbostock commented Jun 2, 2016

Released in 0.2.4.

@jheer
Copy link
Author

jheer commented Jun 2, 2016

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants