syntagmatic / parallel-coordinates

A d3-based parallel coordinates plot in canvas. This library is no longer actively developed.
http://syntagmatic.github.com/parallel-coordinates/
Other
511 stars 211 forks source link

Brushes are removed after calling resize #260

Open melmeseery opened 8 years ago

melmeseery commented 8 years ago

Hi,

I was updating the dimension and doing a resize to the parallel coordinates when I noticed that my brush stopped working. Digging deeper I saw that the brush are reset and destroyed inside resize, but are not added to the axes again. So if I resize the PC do I have to loose the brushes currently installed for each axes. I have no problem with resetting the brushes but why remove all the brushes and do not add them again. Anyway adding the brush again did solve my problem.

So here is my solution to the problem

pc.resize = function() {
  // selection size
  pc.selection.select("svg")
    .attr("width", __.width)
    .attr("height", __.height)
  pc.svg.attr("transform", "translate(" + __.margin.left + "," + __.margin.top + ")");

  // FIXME: the current brush state should pass through
  if (flags.brushable) pc.brushReset();

  // scales
  pc.autoscale();

  // axes, destroys old brushes.
  if (g) pc.createAxes();
  if (flags.brushable) pc.brushable();
  if (flags.reorderable) pc.reorderable();

  // adding the brushes again after destroying the old one. 
  if (pc.brushMode() !== "None") {
        var mode = pc.brushMode();
        pc.brushMode("None");
        pc.brushMode(mode);
      }

  events.resize.call(this, {width: __.width, height: __.height, margin: __.margin});
  return this;
};
bbroeksema commented 8 years ago

Yes, this is a known issue. The problem is that the brushes need to be recalculated based on the new size. I've done some refactoring on brushes a while ago, but never get to the point yet to also fix this issue. I don't have time in the short term to get to this, but I could guide someone who's willing to pick this up.

varvaratou commented 8 years ago

@bbroeksema Do you refer to the work done for commit: https://github.com/bbroeksema/parallel-coordinates/commit/a453198a2fd881f991de6edfa3915629c4a43c29

Could you provide some guidance on what more needs to be done to fix this issue? Thank you!

bbroeksema commented 8 years ago

Likely yes, but I don't remember exactly. Generally, the point is that when you resize, all scales (on which the brushes are based) have become invalid. The simple 'workaround' is to just remove brushes. The proper fix is to re-render brushes using the new scales.

If you want guidance, you'll need to come with concrete questions. I.e. pick a brush (e.g. 1D-brushes) play a bit with placing brushes and resizing the parcoords to understand the current and desired behavior. Next, try to figure out in the code what is causing the current behavior. If you're at that point I can likely give you some pointers on what kind of changes you'll need to do to make it work.

Unfortunately I'm currently not in a situation to spent significant time on this myself.

tdudgeon commented 8 years ago

Just picking up on this - is this resize() function supposed to redraw the PCP at a different size e.g. when the size of the enclosing div changes? If so then that's what I'm wanting, but I can't get it to work. I posted on the google group about this: https://groups.google.com/forum/#!topic/d3-js/ULAFL8yEikE Currently I'm completely regenerating the chart (and rebinding all the data), but I want to avoid this (and keep the settings like order or dimensions and brush settings.

KevinDuringWork commented 8 years ago

I have a fairly ugly fix for this involving by extracting the brush state (extents) and redrawing on resize. LMK if anyone is interested in a more detailed writeup.

tdudgeon commented 8 years ago

Any info would be very useful (so yes please), but also a discussion on whether the resize() function is supposed to function as described?

syntagmatic commented 8 years ago

It is indeed desirable if resizing left the rest of the chart state intact, but in the initial implementation I believe it simply reset the brushes.

KevinDuringWork commented 8 years ago

Actually I think my changes are very similar if not the same as AutodeskFractal/parallel-coordinates. Sorry is there a particular reason why it's not merged into the current master?

syntagmatic commented 8 years ago

I didn't see a pull request aside from https://github.com/syntagmatic/parallel-coordinates/issues/260#ref-pullrequest-156001128 which was merged

I'm not actively coordinating development of the library (I don't think anyone is). If I get around to it, I'd like to do a complete re-write for d3 version 4, but leave this library as it is because of all the great features contributors have submitted.