#24 RFE: setSize of GraphLayout move vertices to filling space

open
nobody
None
5
2012-08-01
2012-08-01
Anonymous
No

At the moment, a call to setSize() to resize any GraphLayout will result in the current layout being moved to the centre of the new window (see bug tracker for when this fails with AggregateLayout).

It would be really useful if the vertices were scaled to fill the new window space instead.

For example, start with a simple 100x100px window with vertices on each corner. If the window is resized to be 200x200px (the relevant setSize() methods are called on window movement - also listed in the bug tracker), then the 4 vertices will be move to be centred in the middle of the new window but will still have the 100x100px square shape. In this RFE, the vertices would be moved to be positioned at the corners of the 200x200px square.

Discussion

  • Comment has been marked as spam. 
    Undo

    You can see all pending comments posted by this user  here

    Anonymous - 2012-08-14

    This is a monster workaround, but it also address issues #3553281 and #3553275.

    I think this bug exemplifies and typifies issue #3542000 !!!! i.e. the getters/setters/constructors in the Layout code (probably everywhere in JUNG, if this is to be extrapolated) are doing *way* more than they should be and it makes the code very hard to understand and customise. I feel a refactor to follow the boring Javabeans pattern would make these classes a lot cleaner - any precomputation of values should be done *lazily* at the point they are needed. If setters do *anything* other than set their value, they should simply mark a class as having been updated, as a hint for such pre-computation routines. A Lombok powered @BoundSetter would enable a class to listen to its own property changes and flag itself as "dirty". That's about 5 lines of code to do what is currently taking hundreds.

    Some further notes/criticisms ;-) in the comments, and again, sorry about the formatting - sourceforge just has a mission to make everything painful and ugly.

    This works for FRLayout, I suspect it will work with small modifications for others.

    Enjoy!

    @Override
    public void setSize(Dimension size) {
    // WORKAROUND: setSize doesn't touch the initialiser - ID: 3553281
    // https://sourceforge.net/tracker/?func=detail&aid=3553281&group_id=73840&atid=539119
    //
    // passing up will result in:
    //
    // FRLayout.setSize: an initialisation check (which will fail)
    // AbstractLayout.setSize: this.size = size (which is all we need to do the call for)
    // AbstractLayout.setSize: initialize()
    // -> doInit() and resetting of parameters
    // AbstractLayout.setSize: adjustLocations() (private, no @Override)
    // -> ConcurrentModificationException issues!
    // FRLayout.setSize: set max_dimension (private, no @Override)
    //
    // Instead, we manually set the 'size' here and reimplement expected logic

    Dimension oldSize = this.size;
    this.size = Preconditions.checkNotNull(size);
    Map<V, Point2D> adjustedLocations = Maps.newHashMap();
    for (V vertex : locations.keySet()) {
    if (!getGraph().getVertices().contains(vertex)) {
    // partially addressing "Layouts keeping Vertices alive - ID: 3553275"
    // http://sourceforge.net/tracker/?func=detail&aid=3553275&group_id=73840&atid=539119
    continue;
    }
    Point2D location = locations.get(vertex);
    if (location != null) {
    adjustedLocations.put(vertex, rescaleLocation(location, oldSize, size));
    }
    }
    // setInitializer() will just reset the locations of everything, so we repeat the logic
    // also, why does AbstractLayout clone the vertex? Seems a pointless endeavour...
    RandomLocationTransformer<V> initialiser = new RandomLocationTransformer<V>(size);
    Map<V, Point2D> newLocations = LazyMap.decorate(adjustedLocations, initialiser);
    // synchronized is much better than looping/catching/ignoring ConcurrentModificationException
    this.locations = Collections.synchronizedMap(newLocations);

    try {
    // it seems a bit of a premature optimisation to ever calculate max_dimension,
    // since it is only ever used in done(). Here, we have to use reflection.
    Field max_dim_field = getClass().getDeclaredField("max_dimension");
    double max = Math.max(size.width, size.height);
    max_dim_field.set(this, max);
    } catch (Exception ex) {
    log.log(Level.SEVERE, "Reflection FAIL", ex);
    }
    }

    protected Point2D rescaleLocation(Point2D location, Dimension oldSize, Dimension newSize) {
    if (oldSize == null) {
    return location;
    }

    double x_percentage = location.getX() / oldSize.getWidth();
    double y_percentage = location.getY() / oldSize.getHeight();
    return new Point2D.Double(x_percentage * newSize.width, y_percentage * newSize.height);
    }

     
  • Comment has been marked as spam. 
    Undo

    You can see all pending comments posted by this user  here

    Anonymous - 2012-08-14

    sorry, mistake in reflection code, should be

    Field max_dim_field = FRLayout.class.getDeclaredField("max_dimension");
    max_dim_field.setAccessible(true);

     

Log in to post a comment.