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

Shape point() does not work with scale > 1, strokeWeight < 1 #4707

Closed
1 task done
ednl opened this issue Jul 23, 2020 · 5 comments · Fixed by #4756
Closed
1 task done

Shape point() does not work with scale > 1, strokeWeight < 1 #4707

ednl opened this issue Jul 23, 2020 · 5 comments · Fixed by #4756
Labels

Comments

@ednl
Copy link

ednl commented Jul 23, 2020

Most appropriate sub-area of p5.js

  • Core/Environment/Rendering

Details about the bug:

  • p5.js version: 1.1.9 July 22, 2020
  • Web browser and version: Safari 13.1.2 (15609.3.5.1.3)
  • Operating System: MacOS 10.15.6
  • Steps to reproduce this: I have a square canvas and want to use it as a coordinate system(-1..+1, -1..+1). So I translate to (width/2, width/2) and scale by (width/2,-width/2). Then I set the strokeWeight to be 1 actual pixel: 2/width. Expectation: lines, circles and points drawn will be 1 pixel wide. This is true for lines and circles, but not points. It seems that the strokeWeight is not used for points, perhaps because it can't be smaller than 1, but the scale is respected. This results in mega-points covering a quarter of the canvas (because in scaled coordinates the area is 4). Code that fails:
function setup() {
	createCanvas(400, 400);		// square canvas
	background(0);
	noFill();

	const pixperunit = width / 2;	// number of pixels per unit in the desired coordinate system
	translate(pixperunit, pixperunit); // set the origin in the middle
	scale(pixperunit, -pixperunit);	// horizontal and vertical range is now -1..+1
	strokeWeight(1 / pixperunit);	// all lines, arcs and points should be 1 pixel wide

	stroke(255, 0, 0);		// red axes
	line(-1, 0, 1, 0);
	line(0, -1, 0, 1);

	stroke(0, 255, 0);		// green circle
	circle(0, 0, 1);

	stroke(0, 0, 255);		// blue dot
	point(0, 0);
}

The resulting picture, which should have one blue pixel in the centre, not a large blue square:
Schermafbeelding 2020-07-23 om 15 04 48

@ednl ednl added the Bug label Jul 23, 2020
@welcome
Copy link

welcome bot commented Jul 23, 2020

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

@ednl
Copy link
Author

ednl commented Jul 23, 2020

The issue is in https://github.com/processing/p5.js/blob/main/src/core/p5.Renderer2D.js#L559 from line 559 where lineWidth is checked, presumably as a scaled value, but then fillRect is used with hardcoded width=1, height=1 without regard to scaling.

@ednl
Copy link
Author

ednl commented Aug 1, 2020

I could invent some amendment to that function in p5.Renderer2D.js, but I am relatively new to p5js and certainly to the actual code behind it, and I don't know whether this is by design, or what the implications are for a very local change there. Would other parts change behaviour, or break? Maybe the performance would suffer greatly?

@TakumaKira
Copy link
Contributor

TakumaKira commented Aug 19, 2020

Hi @ednl,

I looked into the change history of the repository, and I couldn't find any reason the conditional statement on line 559 is needed. Also, the unit test for my changes passed with a small change. So I believe this library can allow your case and created the PR #4756 to allow drawing a point with strokeWeight < 1.

@ednl
Copy link
Author

ednl commented Aug 19, 2020

@TakumaKira Fantastic, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants