p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

3D quad(), triangle(), are broken

Open aceslowman opened this issue 3 years ago • 1 comments
trafficstars

Most appropriate sub-area of p5.js?

  • [ ] Accessibility
  • [ ] Color
  • [X] Core/Environment/Rendering
  • [ ] Data
  • [ ] DOM
  • [ ] Events
  • [ ] Image
  • [ ] IO
  • [ ] Math
  • [ ] Typography
  • [ ] Utilities
  • [X] WebGL
  • [ ] Build Process
  • [ ] Unit Testing
  • [ ] Internalization
  • [ ] Friendly Errors
  • [ ] Other (specify if possible)

p5.js version

1.4.2

Web browser and version

105.0.5195.127 (Official Build) (64-bit) (cohort: Stable)

Operating System

Windows 10

Steps to reproduce this

When in WebGL mode both the quad and triangle methods don't seem to be working as described. Quad won't draw at all when using 4 x,y,z coords and triangle() throws an error and seems to truncate the last three args.

Steps:

  1. set up sketch for webgl mode
  2. draw either a quad() or triangle()

Snippet:


function setup() {
  createCanvas(400, 400, WEBGL);
  debugMode();
}

function draw() {
  background(220);
  orbitControl();
  
  scale(20);

  // drawing correctly in 2D
  fill("red");
  quad(
    -1,  1, 
     1,  1, 
     1, -1, 
    -1, -1
  );

  // not drawing at all when using 12 args
  fill("blue");
  quad(
    -1,  1, 0, 
     1,  1, 0, 
     1, -1, 0, 
    -1, -1, 0
  );

  // drawing correctly in 2D
  fill("green");
  triangle(
    0,  0, 
    0, -1, 
    1,  0
  );

  // results in warning, truncates the last 3 args
  fill("purple");
  triangle(
    0,  0, 0, 
    0, -1, 0, 
    1,  0, 0
  );

  // line is working fine in 3D
  fill("brown");
  line(
    0,  0, 0, 
    1, -1, 1
  );
}

aceslowman avatar Sep 19 '22 17:09 aceslowman

to clarify, there is code in the webGL renderer that already draws these shapes! when Austin and I were debugging one of his sketches this morning, there were some friendly error messages in the console suggesting that triangle() had 9 arguments instead of the expected 6. I think the friendly error system is not accounting for webgl mode and throwing errors for code that should work. I'm going to add a label to help indicate this.

kjhollen avatar Sep 19 '22 20:09 kjhollen

Thanks @aceslowman and @kjhollen. I'm adding the FES stewards @outofambit, @almchung to this discussion.

Qianqianye avatar Sep 26 '22 20:09 Qianqianye

Excuse me. If the quad function has 12 arguments, the current definition seems to only use the first 8 arguments.

p5.js/src/core/shape/2d_primitives.js line531~554:

p5.prototype.quad = function(...args) {
  p5._validateParameters('quad', args);

  if (this._renderer._doStroke || this._renderer._doFill) {
    if (this._renderer.isP3D && args.length <= 12) {
      // if 3D and we weren't passed 12 args, assume Z is 0
      this._renderer.quad.call(
        this._renderer,
        args[0], args[1], 0,
        args[2], args[3], 0,
        args[4], args[5], 0,
        args[6], args[7], 0,
        args[8], args[9]);
    } else {
      this._renderer.quad(...args);
      //accessibile outputs
      if (this._accessibleOutputs.grid || this._accessibleOutputs.text) {
        this._accsOutput('quadrilateral', args);
      }
    }
  }

  return this;
};

I believe this can be resolved by doing the following:

before

535    if (this._renderer.isP3D && args.length <= 12) {

after

535    if (this._renderer.isP3D && args.length < 12) {

This way, if there are 11 or fewer arguments, the first 8 arguments will be used as coordinates when drawing, and if there are 12 or 14 arguments, the first 12 arguments will be used as coordinates when drawing.

inaridarkfox4231 avatar Dec 16 '22 17:12 inaridarkfox4231

@inaridarkfox4231 I think you're right! Would you be interested in making a PR with that change?

davepagurek avatar Dec 16 '22 23:12 davepagurek

Thanks for the reply! Yes, I would like to make a pull request.

inaridarkfox4231 avatar Dec 17 '22 00:12 inaridarkfox4231

@inaridarkfox4231 Thanks! I've assigned this issue to you.

davepagurek avatar Dec 17 '22 00:12 davepagurek