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

beginContour() and endContour doesnt work on shape #643

Closed
OverdriveNGain opened this issue Mar 23, 2017 · 15 comments
Closed

beginContour() and endContour doesnt work on shape #643

OverdriveNGain opened this issue Mar 23, 2017 · 15 comments
Labels
Help Wanted We have very little time and would like some help

Comments

@OverdriveNGain
Copy link

PShape s;
s = createShape();
translate(50, 50);
s.beginShape();

s.vertex(-40, -40);
s.vertex(40, -40);
s.vertex(40, 40);
s.vertex(-40, 40);

s.endShape(CLOSE);
s.beginShape();
s.beginContour();
s.vertex(-20, -20);
s.vertex(-20, 20);
s.vertex(20, 20);
s.vertex(20, -20);
s.endContour();

s.endShape();

shape(s);

this code came directly from the processing website (although shortened), yet it still doesnt work. https://processing.org/reference/PShape_beginContour_.html

Processing 3.3
Windows 10, 64-bit
untitled

@benfry
Copy link
Owner

benfry commented May 4, 2017

That's a typo in the documentation. It should be s.endShape(CLOSE) on the second-to-last line.

@kfrajer
Copy link

kfrajer commented May 14, 2017

@benfry I don't think the issue is resolved, or at least it is not clear how your answer solves the problem. I tried the provided example in the Foundation and it didn't work. The problem was observed in this post: https://forum.processing.org/two/discussion/comment/97902#Comment_97902

Below is an example from that post.

kfrajer

PShape s;

void setup()
{
  size(500, 300);

  s = createShape();

  s.beginShape();
  s.fill(255, 0, 0);
  s.vertex(100, 100);
  s.vertex(200, 100);
  s.vertex(200, 200);
  s.vertex(100, 200);
  s.vertex(100, 100);  //Added
  s.beginContour();
  s.vertex(125, 125);
  s.vertex(125, 175);
  s.vertex(175, 175);
  s.vertex(175, 125);
  s.vertex(125, 125);   //Added
  s.endContour();
  s.endShape(CLOSE);
}

void draw()
{
  shape(s, 200, 0);
}

@benfry
Copy link
Owner

benfry commented May 15, 2017

Using endShape(CLOSE) instead of endShape() ensures that the black line (the outer stroke) connects back to itself, which is the expected behavior.

I can't tell from your code what you're expecting it to do.

Also, those //Added lines in your last comment will cause problems when rendering. Never duplicate points, that's what endShape(CLOSE) is for.

@jeremydouglass
Copy link
Contributor

jeremydouglass commented May 16, 2017

The surprising thing here is that, given two otherwise identical code blocks with identical vertices in this form:

beginShape();
  // outer vertices
  beginContour();
    // inner vertices
  endContour();
endShape(CLOSE);

...the same code produces two different results:

  1. using functions, e.g. beginContour()
  2. using PShape methods, e.g. s.beginContour()

BugBeginContour

The left and center images show this surprising result. The "added vertices" example on the right was kfrajer's fix attempt.

Test code:

// beginContour() and endContour doesnt work on shape
// github.com/processing/processing/issues/4978
// forum.processing.org/two/discussion/comment/97902

PShape s;

void setup()
{
  frameRate(50);
  size(600, 200);
  fill(255, 0, 0);
  noLoop();
}

void draw()
{
  translate(50, 50);
  text("contour shape", 0, -10);
  beginShape();
  fill(255, 0, 0);
  vertex(  0, 0);
  vertex(100, 0);
  vertex(100, 100);
  vertex(  0, 100);
  beginContour();
  vertex( 25, 25);
  vertex( 25, 75);
  vertex( 75, 75);
  vertex( 75, 25);
  endContour();
  endShape(CLOSE);

  translate(200, 0);

  text("PShape, identical", 0, -10);
  s = createShape();
  s.beginShape();
  s.fill(255, 0, 0);
  s.vertex(  0, 0);
  s.vertex(100, 0);
  s.vertex(100, 100);
  s.vertex(  0, 100);
  s.beginContour();
  s.vertex( 25, 25);
  s.vertex( 25, 75);
  s.vertex( 75, 75);
  s.vertex( 75, 25);
  s.endContour();
  s.endShape(CLOSE);
  shape(s, 0, 0);

  translate(200, 0);

  text("PShape, two added", 0, -10);
  s = createShape();
  s.beginShape();
  s.fill(255, 0, 0);
  s.vertex(  0, 0);
  s.vertex(100, 0);
  s.vertex(100, 100);
  s.vertex(  0, 100);
  s.vertex(  0, 0); // added
  s.beginContour();
  s.vertex( 25, 25);
  s.vertex( 25, 75);
  s.vertex( 75, 75);
  s.vertex( 75, 25);
  s.vertex( 25, 25); // added
  s.endContour();
  s.endShape(CLOSE);
  shape(s, 0, 0);
}

@benfry
Copy link
Owner

benfry commented May 16, 2017

The middle image is the intended result. Is that the confusion here?

Again, never make the vertices come back to meet themselves. That'll cause problems.

@jeremydouglass
Copy link
Contributor

jeremydouglass commented May 16, 2017 via email

@jeremydouglass
Copy link
Contributor

Here is a second example of what I believe may be the same issue:

Two identical PShape sequences, except that one contains "beginContour / endContour" commands, and one does not. One should contain a contour -- but instead both render the same end shape.

PShape s;
void setup() {
  frameRate(50);
  size(400, 200);
  fill(255, 0, 0);
  noLoop();
}
void draw() {
  translate(50, 50);
  text("PShape w contour", 0, -10);
  s = createShape();
  s.beginShape();
  s.fill(255, 0, 0);
  s.vertex(  0, 0);
  s.vertex(100, 0);
  s.vertex(100, 100);
  s.vertex(  0, 100);
  s.beginContour(); // begin contour
  s.vertex( 25, 25);
  s.vertex( 25, 75);
  s.vertex( 75, 75);
  s.vertex( 75, 25);
  s.endContour(); // end contour
  s.endShape(CLOSE);
  shape(s, 0, 0);

  translate(200, 0);
  text("PShape w/out contour", 0, -10);
  s = createShape();
  s.beginShape();
  s.fill(255, 0, 0);
  s.vertex(  0, 0);
  s.vertex(100, 0);
  s.vertex(100, 100);
  s.vertex(  0, 100);
  s.vertex( 25, 25); // no begin
  s.vertex( 25, 75);
  s.vertex( 75, 75);
  s.vertex( 75, 25);
  s.endShape(CLOSE); // no end
  shape(s, 0, 0);
}

BugBeginContour2

@jeremydouglass
Copy link
Contributor

jeremydouglass commented May 16, 2017

Now let's do it again, but take those commands out of PShape methods and use the draw functions instead. Again, two identical sequences of vertices, with the only difference being that one uses "beginContour / endContour", and one does not.

PShape s;
void setup() {
  size(400, 200);
  fill(255, 0, 0);
  noLoop();
}
void draw() {
  translate(50, 50);
  text("shape w contour", 0, -10);
  beginShape();
  fill(255, 0, 0);
  vertex(  0, 0);
  vertex(100, 0);
  vertex(100, 100);
  vertex(  0, 100);
  beginContour(); // begin contour
  vertex( 25, 25);
  vertex( 25, 75);
  vertex( 75, 75);
  vertex( 75, 25);
  endContour(); // end begin contour
  endShape(CLOSE);

  translate(200, 0);
  text("shape w contour", 0, -10);
  beginShape();
  fill(255, 0, 0);
  vertex(  0, 0);
  vertex(100, 0);
  vertex(100, 100);
  vertex(  0, 100);
  vertex( 25, 25); // no begin
  vertex( 25, 75);
  vertex( 75, 75);
  vertex( 75, 25);
  endShape(CLOSE); // no end
} 

Here, we see two different shapes. That is expected -- one contains a contour, one does not. So within draw, beginContour() can be used to create contours in this way -- using it creates a closed shape.

BugBeginContour3

However, as we saw above, within PShape, PShape.beginContour() can not be used in this way. Using it does not create a closed shape -- it has no effect. Identical uses, but different results in the different draw() and PShape contexts.

BugBeginContour2

@benfry benfry reopened this May 16, 2017
@benfry
Copy link
Owner

benfry commented May 16, 2017

Sorry, misspoke—the correct version is the one marked “contour shape”

So, the documentation error is that it's also showing the wrong image, perpetuating the buggy behavior.

Ah, what a mess. Now, for someone to fix it…

@benfry benfry added the Help Wanted We have very little time and would like some help label May 16, 2017
@REAS
Copy link
Contributor

REAS commented May 18, 2017

@jeremydouglass Can you note this put in /processing/processing-docs/ so the documentation error is recorded?

@kfrajer
Copy link

kfrajer commented May 18, 2017

@REAS @jeremydouglass I believe benfry already posted this typo in docs: processing/processing-docs#551 and is assigned to your name.

kfrajer

@REAS
Copy link
Contributor

REAS commented May 18, 2017

Yes, my mistake. Thank you.

@REAS
Copy link
Contributor

REAS commented Nov 10, 2017

I walked through this whole thread and I've come back to the conclusion that there's an error.

Please check this example, I think the code is equivalent for both:

void setup() {
  size(400, 200);
  fill(255, 0, 0);

  translate(50, 50);
  
  beginShape();
  fill(255, 0, 0);
  vertex( 0, 0);
  vertex(100, 0);
  vertex(100, 100);
  vertex(0, 100);
  beginContour();
  vertex(25, 25);
  vertex(25, 75);
  vertex(75, 75);
  vertex(75, 25);
  endContour();
  endShape(CLOSE);

  translate(200, 0);

  PShape s = createShape();
  s.beginShape();
  s.fill(255, 0, 0);
  s.vertex(0, 0);
  s.vertex(100, 0);
  s.vertex(100, 100);
  s.vertex(0, 100);
  s.beginContour();
  s.vertex(25, 25);
  s.vertex(25, 75);
  s.vertex(75, 75);
  s.vertex(75, 25);
  s.endContour();
  s.endShape(CLOSE);
  shape(s, 0, 0);
}

screen shot 2017-11-10 at 1 09 03 pm

@jeremydouglass
Copy link
Contributor

jeremydouglass commented Nov 13, 2017

That's right. The two lines beginContour() / endContour() in the second shape are the bug.

Specifically, the contour is working correctly in the left example. If you remove the two Contour code lines, then the correct hollow box shape changes to a simple closed path.

The contour is not working in the right example. If you remove the two contour code lines, nothing changes. That's because the lines aren't doing anything in the first place -- although they should be.

shape() respects the beginContour(), but the PShape.shape() is rendering all the vertices as one closed shape and ignoring whether there is a contour defined or not.

@cluder
Copy link

cluder commented Mar 22, 2019

P2D and P3D also render correctly,
Only JAVA2d and FX2D render it wrongly.

In PGraphicsJava2D, drawGeometry() g.vertex() is called 8 times in a loop, inside g.vertex() there is a check, if a break is needed, but this 'break' variable is never true in this case.
So basically the break is recorded (in PShape.vertexCodes[]), but when drawing the shape, the break is not taken into account yet.

  protected void drawGeometry(PGraphics g) {
    // get cache object using g.
    g.beginShape(kind);
    if (style) {
      for (int i = 0; i < vertexCount; i++) {
        g.vertex(vertices[i]);  // we need a break for i == 4
      }

I made an example hack, which 'fixes' that problem for JAVA2D and FX2D, by adding an internal breakShape() method.
But pretty sure, that's not the right way to do it :) (i am new to processing)

diff --git a/core/src/processing/awt/PGraphicsJava2D.java b/core/src/processing/awt/PGraphicsJava2D.java
index c93ecad..b79f450 100644
--- a/core/src/processing/awt/PGraphicsJava2D.java
+++ b/core/src/processing/awt/PGraphicsJava2D.java
@@ -3007,7 +3007,10 @@
   }
 
 
-
+  @Override
+  protected void breakShape() {
+    breakShape = true;
+  }
   //////////////////////////////////////////////////////////////
 
   // BLEND
diff --git a/core/src/processing/core/PGraphics.java b/core/src/processing/core/PGraphics.java
index 65bcb28..dc0f33a 100644
--- a/core/src/processing/core/PGraphics.java
+++ b/core/src/processing/core/PGraphics.java
@@ -8574,4 +8574,8 @@
 
   }
 
+  protected void breakShape() {
+
+  }
+
 }
diff --git a/core/src/processing/core/PShape.java b/core/src/processing/core/PShape.java
index b587e3e..51e23d1 100644
--- a/core/src/processing/core/PShape.java
+++ b/core/src/processing/core/PShape.java
@@ -1717,6 +1717,11 @@
     g.beginShape(kind);
     if (style) {
       for (int i = 0; i < vertexCount; i++) {
+        if (vertexCodeCount > 0) {
+          if (vertexCodes[i] == BREAK) {
+            g.breakShape();
+          }
+        }
         g.vertex(vertices[i]);
       }
     } else {
diff --git a/core/src/processing/javafx/PGraphicsFX2D.java b/core/src/processing/javafx/PGraphicsFX2D.java
index 74758ce..163c5ed 100644
--- a/core/src/processing/javafx/PGraphicsFX2D.java
+++ b/core/src/processing/javafx/PGraphicsFX2D.java
@@ -2290,7 +2290,10 @@
 //  }
 
 
-
+  @Override
+  protected void breakShape() {
+    breakShape = true;
+  }
   //////////////////////////////////////////////////////////////
 
   // BLEND

image

@benfry benfry transferred this issue from processing/processing Jan 18, 2023
@benfry benfry closed this as completed in 4a967e3 Sep 15, 2023
benfry added a commit that referenced this issue Sep 15, 2023
fix #643 - PShape.beginContour() bug
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Help Wanted We have very little time and would like some help
Projects
None yet
Development

No branches or pull requests

6 participants