rikrd / geomerative

Geomerative is a library for Processing. It extends 2D geometry operations to facilitate generative geometry. Includes a TrueType font and an SVG interpreters. This library exposes the shapes (such as vector drawings or typographies) in a more approchable way. Geomerative makes it easy to access the contours, the control points and the curve points, making it easy to develop generative typography and geometry pieces in Processing.
http://www.ricardmarxer.com/geomerative
GNU General Public License v3.0
176 stars 29 forks source link

RShape.contains(RShape) is always false #7

Closed Angular-Angel closed 6 years ago

Angular-Angel commented 6 years ago

RShape.contains(RShape) always seems to return false, no matter what. Here's a sketch that accurately describes the problem - the small rectangle should turn red when inside the larger one, but it never does, despite intersecting with it, and then not intersecting with it after it goes inside.

import geomerative.*;

RShape shape;
RShape rect;

void setup(){
  size(600, 600, OPENGL);
  smooth();

  // VERY IMPORTANT: Allways initialize the library before using it
  RG.init(this);
  RG.setPolygonizer(RG.ADAPTATIVE);

  RG.ignoreStyles();

  shape = RG.getRect(100,100,100,50);
}

void draw(){
  rect = RShape.createRectangle(mouseX, mouseY, 10, 10);
  shape.draw(this);
  if (shape.contains(rect))
    rect.setFill(color(255, 0, 0));
  rect.draw(this);
  RPoint[] ps = shape.getIntersections(rect);
  if (ps != null) {
    for (int i=0; i<ps.length; i++) {
      ellipse(ps[i].x, ps[i].y, 10, 10);
    }
  }
}
monkstone commented 6 years ago

I believe this to be due to faulty logic in RGeomElem.java

  public boolean contains(RPoint[] ps) {
    boolean contains = false;
    if(ps != null){
      for(int i=0; i < ps.length; i++) {
        contains &= this.contains(ps[i]);
      }
    }
    return contains;
  } 

In my hands always returns false Simple crude fix:-

 public boolean contains(RPoint[] ps) {
        if (ps.length == 0) {
            return false;
        }
        for (RPoint p : ps) {
            if (!this.contains(p)) {
                return false;
            }
        }
        return true;
    } 
monkstone commented 6 years ago

Since java-8 possible to use lambda for a more elegant solution, and it makes sense not to overload contains.

    public boolean containsPoints(RPoint[] pts) {
        Stream<RPoint> outside = Arrays.stream(pts).filter(pt -> !contains(pt));
        return outside.count() == 0;
    }
Angular-Angel commented 6 years ago

Hmm. Is this library maintained? If not it might be necessary to fork it. A pain, but I could do it if I need to. :/

As for the function, the earlier fix you posted seems more efficient. It returns immediately upon discovering false, whereas this one performs a bunch of unnecessary computation. And yeah, this ones shorter, but it's also a lot denser and more complicated. Personally, I'd prefer the first, simpler option.

rikrd commented 6 years ago

Sorry for the delay fellows. I have been swamped with pressing deadlines and hadn't had a chance to look into this.

Angular-Angel commented 6 years ago

Do I need to do anything to update my library for this? I tried running Processing and it didn't show any updates, or say anything about updates. :/

rikrd commented 6 years ago

I have made a new release (42) and published it on the web: http://ricardmarxer.com/geomerative/geomerative.zip Processing didn't seem to pick up this update. I have asked the Processing library contact to see what's going on.

On Sat, Nov 10, 2018 at 4:25 AM Angular-Angel notifications@github.com wrote:

Do I need to do anything to update my library for this? I tried running Processing and it didn't show any updates, or say anything about updates. :/

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/rikrd/geomerative/issues/7#issuecomment-437554615, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA73JaZvJBdosrBCAG_P8dss0XZdibrks5utkcMgaJpZM4X9ODA .

-- ricard http://twitter.com/ricardmp http://www.ricardmarxer.com http://www.caligraft.com