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

ellipseMode(CORNERS) broken in some cases #7289

Open
1 of 17 tasks
martinleopold opened this issue Sep 26, 2024 · 0 comments · May be fixed by #7290
Open
1 of 17 tasks

ellipseMode(CORNERS) broken in some cases #7289

martinleopold opened this issue Sep 26, 2024 · 0 comments · May be fixed by #7290

Comments

@martinleopold
Copy link

martinleopold commented Sep 26, 2024

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

1.10.0

Web browser and version

Safari 18.0

Operating system

maxOS 14.7

Steps to reproduce this

Basically ellipseMode(CORNERS) only seems to work, if the first point given is top left, and the coordinates are positive.

I've made the sketch below to test out different configurations of first point (P1) and second point (P2), as well as positive and negative coordinates. This is the result:

Screen Shot 2024-09-26 at 12 35 41

7 of the 16 test cases crash p5 (IndexError), 8 produce a wrong ellipse rendering, only 1 works as intended.

This also affects arc(), since it uses the same rendering code.

Here is a PR to address this issue: #7290

Steps:

  1. Run this sketch: https://editor.p5js.org/groedl/sketches/pBpiKShRE (or see snippet below)

Snippet:

function setup() {
  createCanvas(600, 900);
  noFill();
  noLoop();
}

function ellipseCorners(x1, y1, x2, y2) {
  // Bounding box (light gray)
  rectMode(CORNERS);
  stroke(200);
  noFill();
  strokeWeight(1);
  rect(x1, y1, x2, y2);
  
  // P1, P2 dots
  stroke(150);
  strokeWeight(4);
  point(x1, y1);
  point(x2, y2);
  
  // P1, P2 labels
  fill(150);
  noStroke();
  text('P1', x1 + 5, y1);
  text('P2', x2 + 5, y2);
  
  // Ellipse
  noFill();
  stroke(0);
  strokeWeight(1);
  ellipseMode(CORNERS);
  
  try {
    ellipse(x1, y1, x2, y2);
    console.log(`Drawing (${x1}, ${y1}, ${x2}, ${y2})`);
  } catch (e) {
    console.warn(`Error drawing (${x1}, ${y1}, ${x2}, ${y2})`);
    console.error(e);
  }
}


function draw() {
  background(220);
  translate(width/2, height/2);
  
  // Coordinate System
  stroke(200);
  line(-width/2, 0, width/2, 0);
  line(0, -height/2, 0, height/2);
  stroke(0);
  
  
  // Quadrant I (Bottom Right)
  console.log("(I) BOTTOM RIGHT");
  ellipseCorners(100, 50, 200, 100);  // OK
  ellipseCorners(100, 200, 200, 150); // Error
  ellipseCorners(200, 300, 100, 250); // Error
  ellipseCorners(200, 350, 100, 400); // Error
  
  
  // Quadrant II (Bottom Left)
  console.log();
  console.log("(II) BOTTOM LEFT");
  ellipseCorners(-200, 50, -100, 100);  // Wrong ellipse
  ellipseCorners(-200, 200, -100, 150); // Error
  ellipseCorners(-100, 300, -200, 250); // Error
  ellipseCorners(-100, 350, -200, 400); // Wrong ellipse
  
  
  // Quadrant III (Top Left)
  console.log();
  console.log("(III) TOP LEFT");
  ellipseCorners(-200, -400, -100, -350); // Wrong ellipse
  ellipseCorners(-200, -250, -100, -300); // Wrong ellipse
  ellipseCorners(-100, -150, -200, -200); // Wrong ellipse
  ellipseCorners(-100, -100, -200, -50);  // Wrong ellipse
  
  
  // Quadrant IV (Top Rigth)
  console.log();
  console.log("(IV) TOP RIGHT");
  ellipseCorners(100, -400, 200, -350); // Wrong ellipse
  ellipseCorners(100, -250, 200, -300); // Wrong ellipse
  ellipseCorners(200, -150, 100, -200); // Error
  ellipseCorners(200, -100, 100, -50);  // Error
}
@martinleopold martinleopold linked a pull request Sep 26, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant