Skip to content

fix point() origin bug and stale per-vertex color leak#8712

Merged
ksen0 merged 2 commits intoprocessing:mainfrom
perminder-17:point-position
Apr 8, 2026
Merged

fix point() origin bug and stale per-vertex color leak#8712
ksen0 merged 2 commits intoprocessing:mainfrom
perminder-17:point-position

Conversation

@perminder-17
Copy link
Copy Markdown
Collaborator

@perminder-17 perminder-17 commented Apr 8, 2026

Resolves #8701

In _drawPoints, we now sync incoming vertices into this.immediateMode.geometry.vertices before _prepareBuffer runs. This ensures aPosition gets real point coordinates instead of falling back to (0,0,0).

Color leak fix:
For direct point() calls, we clear stale vertexStrokeColors (left over from beginShape(POINTS)). This makes point() use current stroke() color while beginShape(POINTS) still uses per-vertex colors correctly.

PR Checklist

@perminder-17
Copy link
Copy Markdown
Collaborator Author

https://editor.p5js.org/aman12345/sketches/FzSrg-6pj

here's the build.

@perminder-17 perminder-17 requested a review from davepagurek April 8, 2026 11:28
@ksen0
Copy link
Copy Markdown
Member

ksen0 commented Apr 8, 2026

I tried it out, also with some push/pops on color. It seems to function for me; the PR fixes the regression, but does not add parity for the weight effect, which is also mentioned in the issue. I checked earlier 1.x versions, and the behavior is indeed different between 1.x and 2.x - @davepagurek note the strokeWeight(8); which is definitely doing something in both cases, but a different thing. However as this is not a regression, I do not think it should be touched in this PR, though documenting why there's a difference would be good if it's intentional.

@davepagurek
Copy link
Copy Markdown
Contributor

davepagurek commented Apr 8, 2026

In 2.x points have the same width as strokes, whereas in 1.x I believe they are using half the width? e.g. if I draw a line in the background, in the build above it looks like this:

function setup() {
  createCanvas(600, 600, WEBGL);
// }

  // strokeWeight(80)
// function draw() {
  background(0);
//  camera(0, 0, 300, 0, 0, 0, 0, 1, 0);

  strokeWeight(80);
  
  stroke('white')
  line(-width/2, -height/2, width/2, height/2)

  // Per-vertex stroke colors using beginShape(POINTS)
  beginShape(POINTS);

  stroke(255, 0, 0);     // Red
  vertex(-60, -40, 0);

  stroke(0, 255, 0);     // Green
  vertex(-20, -40, 0);

  stroke(0, 0, 255);     // Blue
  vertex(20, -40, 0);

  stroke(255, 255, 0);   // Yellow
  vertex(60, -40, 0);

  endShape();

  // Single point() calls for comparison
  stroke(255, 0, 255);   // Magenta
  point(-40, 40, 0);
// push()
// translate(-40, 0, 0);
  stroke(0, 255, 255);   // Cyan
  point(0, 30, 0);
  // pop()


  stroke(255, 0, 255);           // White
  point(40, 40, 0);
}
This PR2.x
image image

Note that I also commented out the camera call. Further complicating things is that in 1.x the point radius did not scale with the camera, so if you uncomment the camera call, you get:

This PR2.x
image image

So basically 2.x handles points a lot more consistently with strokes (by actually removing the separate render path for points and turning them into strokes behind the scenes.)

@davepagurek
Copy link
Copy Markdown
Contributor

Agreed that it's not worth refactoring to fully match 2.x -- it's a pretty big/risky change to do that in 1.x at this point, and I wouldn't be surprised if some 1.x sketches depend on this behaviour and would break if we change it, even though the new behaviour is more consistent overall.

@ksen0
Copy link
Copy Markdown
Member

ksen0 commented Apr 8, 2026

Agreed that it's not worth refactoring to fully match 2.x -- it's a pretty big/risky change to do that in 1.x at this point, and I wouldn't be surprised if some 1.x sketches depend on this behaviour and would break if we change it, even though the new behaviour is more consistent overall.

Agreed. And thanks for the illustrations, @perminder-17 I think it'd be great to add those illustrations to the compatibility repo

@perminder-17
Copy link
Copy Markdown
Collaborator Author

Agreed. And thanks for the illustrations, @perminder-17 I think it'd be great to add those illustrations to the compatibility repo

Sure, Thanks @davepagurek for the examples.

@davepagurek
Copy link
Copy Markdown
Contributor

Thanks for working on a fix @perminder-17!

@ksen0 ksen0 merged commit ad56112 into processing:main Apr 8, 2026
2 checks passed
@perminder-17
Copy link
Copy Markdown
Collaborator Author

perminder-17 commented Apr 8, 2026

Thanks for working on a fix @perminder-17!

Yeah, happy to help!

@perminder-17 perminder-17 deleted the point-position branch April 8, 2026 12:32
@ksen0 ksen0 mentioned this pull request Apr 8, 2026
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Points are always rendered in the center

3 participants