P5.js: The location of point light is not changing with mouse in p5.js version 1.0.0

[DivyamAhuja]

Most appropriate sub-area of p5.js?

• [ ] Color
• [ ] Core/Environment/Rendering
• [ ] Data
• [ ] Events
• [ ] Image
• [ ] IO
• [ ] Math
• [ ] Typography
• [ ] Utilities
• [x] WebGL
• [ ] Other (specify if possible)

Details about the bug:

The location of point light is not changing with mouse in p5.js version 1.0.0
pointLight(255, 0, 0, mouseX - width/2, mouseY - height/2, 200);

• p5.js version: 1.0.0
• Web browser and version: Google Chrome Version 80.0.3987.132 (Official Build) (64-bit)
• Operating System: Ubuntu 18.04 LTS
• Steps to reproduce this:
  

let angle;

function setup() {
  createCanvas(800, 800,WEBGL);
  angle = 0;
}

function draw(){
  background(0,0,0,0);
  noStroke();
  pointLight(255, 0, 0, mouseX - width/2, mouseY - height/2, 200);
  specularMaterial(255,0,0);
  rotateX(angle);
  rotateY(angle);

  sphere(100);
  angle += 0.01;
}

This code works fine in p5.js version 0.10.2
https://editor.p5js.org/DivyamAhuja/sketches/ndOqBlh2

but the location of light doesn't change in version 1.0.0
https://editor.p5js.org/DivyamAhuja/sketches/IjnCX8L6

[DivyamAhuja]

@stalgiag @akshay-99 can you help me check this out

[akshay-99]

Sure. Let me have a look. I'll get back to you soon

[DivyamAhuja]

Thanks @akshay-99

[akshay-99]

Hi @DivyamAhuja sorry for the delay. I found that the issue is introduced in #4136, more specifically, this particular commit

If I comment out the if else block here in src/webgl/p5.Shader.js, it starts working again. I am still trying to figure out why this happens.

[akshay-99]

Okay @DivyamAhuja I found the problem. So uniform uPointLightPosition is of the type gl.FLOAT_VEC3
However uniform.isArray is currently defined as

uniform.isArray =
      uniform.type === gl.FLOAT_MAT3 ||
      uniform.type === gl.FLOAT_MAT4 ||
      uniform.type === gl.INT_VEC2 ||
      uniform.type === gl.INT_VEC3 ||
      uniform.type === gl.INT_VEC4;

Just including FLOAT_VEC3 like below fixes the issue

uniform.isArray =
      uniform.type === gl.FLOAT_MAT3 ||
      uniform.type === gl.FLOAT_MAT4 ||
      uniform.type === gl.FLOAT_VEC3 ||
      uniform.type === gl.INT_VEC2 ||
      uniform.type === gl.INT_VEC3 ||
      uniform.type === gl.INT_VEC4;

What happens right now with the code on main is that the uniform.isArray check gives false here. This means we would now be assigning arrays with the = operator, and comparing with ===, both of which should be done for scalars, not arrays.

The only change we need to make is to add gl.FLOAT_VEC3 in the check for isArray

@stalgiag Is the above approach to solve this issue correct? Or was gl.FLOAT_VEC3 intentionally excluded due to some reason?

[stalgiag]

@akshay-99 that is right! It was an unintentional omission. Thanks for finding it!

[akshay-99]

@stalgiag There are also gl.FLOAT_VEC2 and gl.FLOAT_VEC4. I am assuming they should be included too?

[DivyamAhuja]

Thanks @akshay-99 for finding this bug!

[stalgiag]

@akshay-99 yes please add FLOAT_VEC2, FLOAT_VEC3, and FLOAT_VEC4. Thank you!

[akshay-99]

@DivyamAhuja were you working on something, encountered this issue and then incorporated these changes yourself? Or should I go ahead and open up a PR?

[DivyamAhuja]

@akshay-99 You should go ahead and open a PR for this issue, I was just experimenting current material system of p5.js while I encountered this issue, so I haven't incorporated these changes yet.