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

No need to specify 255 as the fourth argument in stroke() #5988

Closed
inaridarkfox4231 opened this issue Feb 1, 2023 · 1 comment · Fixed by #5989
Closed

No need to specify 255 as the fourth argument in stroke() #5988

inaridarkfox4231 opened this issue Feb 1, 2023 · 1 comment · Fixed by #5989

Comments

@inaridarkfox4231
Copy link
Contributor

Topic

Currently(p5.js/1.5.0), stroke() is being processed to specify 255 as the fourth argument.

p5.RendererGL.prototype.stroke = function (r, g, b, a) {
  //@todo allow transparency in stroking currently doesn't have
  //any impact and causes problems with specularMaterial
  arguments[3] = 255;
  var color = _main.default.prototype.color.apply(this._pInst, arguments);
  this.curStrokeColor = color._array;
};

This seems to be a process that always sets the transparency of the line to 1, but it is a meaningless process. Because you can achieve line transparency like this:

function setup() {
  createCanvas(400, 400, WEBGL);
  background(0);
  strokeWeight(32);
  stroke(color(0, 128, 255, 128));
  noFill();
  beginShape();
  vertex(-100, -100);
  vertex(100, -100);
  vertex(100, 100);
  vertex(-100, 100);
  endShape(CLOSE);
}

trapLine
(However, it seems that strokeJoin will be implemented, so it will look different in the next version.)
This is because if there are two or fewer arguments, the third and subsequent arguments are ignored.
Therefore, you can draw a transparent line even if you pass two arguments as follows.

function setup() {
  createCanvas(400, 400, WEBGL);
  background(0);
  strokeWeight(32);
  stroke(64, 64);
  line(-100, -100, 100, -100);
  stroke(64, 64, 64);
  line(-100, 100, 100, 100);
}

trpline2
I don't know why p5.js has to set the line opacity to 1 in webgl, and it's not something to discuss here, but if p5.js wants to achieve this, the resulting color object should set the alpha value with setAlpha().
Or it can be more easily realized if the 4th component is set to 1 when setting curStrokeColor.

However, it was decided that this specification will be abolished in the previous pull request(#5985). At that time, the code to set to 255 remained.

p5.RendererGL.prototype.stroke = function(r, g, b, a) {

  if(arguments[3] === undefined){
    arguments[3] = 255;
  }

  const color = p5.prototype.color.apply(this._pInst, arguments);
  this.curStrokeColor = color._array;
};

This is unnecessary processing. because it is ignored if there are less than 2 arguments. In the case of 3, the maximum value is automatically set if nothing is specified. For four, it goes without saying.
(To be more specific, there is no guarantee that 255 is the maximum alpha value.)
If the user doesn't explicitly specify alpha, the process for color has already been implemented, so I think this should be omitted.

@davepagurek
Copy link
Contributor

Makes sense, I think we can take out that if statement then.

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.

2 participants