Feature implementation: specularLight

[sanketsingh24]

Implemented specularLight()

This function separates the diffuse and specular quantities of pointLight() and directionalLight(). Defaults to (255,255,255) so that this does not become a breaking change.

See manual tests /webgl/lights/specularLight/ and /webgl/material/specular_ambient/.

Work done-

• Implemented uDirectionalSpecularColors and uPointLightSpecularColors uniform in both shaders
• Implemented specularLight() in /src/webgl/light.js
• Added a manual-test-example in /webgl/lights/specularLight/
• Unit tests
• Documentation
• Example

To do-

@aferriss

[Spongman]

shouldn't this be called specularMaterial() ?

the name specularLight() seems to fit in the with the naming scheme for defining lights (eg, pointLight, directionalLight) whereas this is defining a material property.

is there an issue associated with this? https://github.com/processing/p5.js/tree/master/developer_docs#how-to-contribute

[sanketsingh24]

This method intends to distinguish the specular and diffuse quantities from the pointLight and other lighting methods. It affects the lights directly so I thought it would be better to name it specularLight, although it is not a light, but then ambientlight is too. I am open to suggestions though, and If a better name comes to my mind, i will post about it.

[stalgiag]

Hi @sanketsingh24 and @aferriss just a heads up that the transition to ES6 is happening this week. See this post for a brief overview.. It is okay if this PR doesn't get through in time to be incorporated into that shift, at this point I think it unlikely, but just be prepared for a rather large rebase to fix merge conflicts. I think it will be manageable but just wanted you to be prepared.

Also I understand that this is part of Summer of Code, so the issue-opening protocol didn't quite make sense here since a proposal was already written, but these things are never set in stone, so perhaps some more info about the intention of this feature could be posted along with some screenshots and how it relates to processing's lighting features such as lightSpecular. This kind of accompanying information could be helpful in allowing other contributors to offer opinions about API.

Thanks!

[sanketsingh24]

specularLight(color)

What this method does is differentiates the specular and diffuse components from the pre-existing lighting methods, which improves the customizability of the sketch. The default value is (255,255,255), which is used when this method is not called at all in the sketch, which prevents this feature to become a breaking change. It affects only the methods which are created after it in the sketch. The method is per-light, so it doesn't add up

Artists will be able to emulate something like this now-

Code-

function setup() {
  createCanvas(windowWidth, windowHeight, WEBGL);
  setAttributes('perPixelLighting', true);
  noStroke();
}

function draw() {
  background(0);

  shininess(10);
  ambientLight(50);
  specularLight(250, 0, 0);
  pointLight(255, 0, 0, 0, -100, 100);
  specularLight(0, 250, 0);
  pointLight(0, 255, 0, 0, 100, 100);
  specularMaterial(255);
  sphere(80);
}

Whereas, this was the previous behavior

I think it is very close in behaviour to Processing's lightSpecular, and probably behaves in the same way.

[aferriss]

@stalgiag Thanks for the heads up re: ES6 changes! @sanketsingh24 and I spoke about this and feel comfortable making the changes needed to resolve any merge conflicts.

I also wanted to chime in with my understanding of the specularMaterial() and specularLight() functions and how they differ.

specularMaterial()
A specular material is used to set the diffuse color of a geometry's material, and tells the renderer that this material should reflect specular highlights. Currently in p5, all specular highlights are 100% white, regardless of light or material color.

The color parameters passed to specularMaterial() work in tandem with the colors passed to pointLight(). Here's a few different scenarios:

1. A green point light and a red specular material: the only thing that you will see is the white specular highlight.

  // green light
  pointLight(0, 255, 0, 0, 100, 100);
  
  // White specular material
  specularMaterial(255,0,0);

  sphere(80);

2. A red point light, and a red specular material: you will see a diffuse red light, and a white specular highlight.

  // red light
  pointLight(255, 0, 0, 0, -100, 100);
  
  // red specular material
  specularMaterial(255,0,0);

  sphere(80);

3. A red point light, a green point light, and a red specular material: you will see a diffuse red light with a white specular highlight, and a white specular highlight for the green light (but no green diffuse).

  // red light
  pointLight(255, 0, 0, 0, -100, 100);
  
  // green light
  pointLight(0, 255, 0, 0, 100, 100);
  
  // red specular material
  specularMaterial(255,0,0);
  sphere(80);

4. A red specular light, a green specular light, a white specular material. You will see a diffuse red light with a white specular highlight, and a diffuse green light with a white specular highlight.

  // red light
  pointLight(255, 0, 0, 0, -100, 100);
  
  // green light
  pointLight(0, 255, 0, 0, 100, 100);
  
  // white specular material
  specularMaterial(255);
  sphere(80);

specularLight()

The proposed function specularLight(color) would allow you to change the color of the reflected specular highlights. In all of the examples above, the reflected highlight was always full (255, 255, 255) white regardless of the material color being used. specularLight() would allow you to set the color of this highlight to a color of your choosing. Changing the specular light color has no impact on the diffuse light that is reflected by the material.

A white specular highlight is what we expect in most circumstances but there are times when the specular highlights should not be white. For instance, many metals reflect specular colors tinted towards their material colors. Additionally, you might just want to render funky things.

We have chosen to implement this in such a way that each light can have it's own specular highlight color (up to the max 8 lights that the renderer supports) rather than just one global specular highlight color.

If you omit a call to specularLight() the renderer's current functionality will remain the same and return white specular highlights.

Calls to specularLight() rely on a specular material being in the scene. If there is no specular material, specular light has nothing to reflect, and so you likely won't see anything (actually I'm not 100% sure about this, you'll probably see pure white or whatever other light/material functions were previously called).

Also to @Spongman 's point above, I also agree that the names of these functions are confusing and wonder if there is a better name for this. ambientLight() does break the mold a bit but it makes sense to me because ambient lights aren't tied a specific location / material and are often just set once for a scene. What about naming it specularColor() or specularHighlight()?

@sanketsingh24 If you could make a few more variations and examples of using different combinations of colors and lights as I've done above I think that might help make this a little more clear.

[aferriss]

@stalgiag @Spongman Just wanted to check in to see if you all had any feedback on this PR. It would be great to get this merged since most of the rest of our GSOC work is tied together with this function.

@sanketsingh24 And I decided that renaming to specularColor made more sense than specularLight, but I think we're both still open to other alternatives.

I feel good about the state of this but thought I would check in to see if anyone else had thoughts.

[stalgiag]

This looks great! Thank you @sanketsingh24.

I think this should be merged. I really like the naming with diffuse and specular here. Verbose and clear is the preference with this codebase in my opinion. This PR does bring up general feelings that I have about how we are storing the state of the lighting. My personal feelings are that the lighting state should be consolidated into a single property belonging to the renderer, with more of a hierarchy. Something like:

lighting = {
  active: true, 
  directional: { diffuseColors: [], specularColors: [], directions: []},
  point etc
}

This might be personal preference. I just feel like the lighting state is spread out between a ton of different properties all of which need to be reset individually, and which teeter on the boolean that enables or disables lighting entirely. Anyways! That is mostly unrelated to this PR and definitely didn't start here, but this just reminded me of that concern.

Great work!

[aferriss]

Thanks for the vote of confidence @stalgiag ! I'm going to merge but if anyone has further suggestions re: naming or clarification of this function down the road please feel free to reach out.