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

Default ambient color incorrect (in v1.5.0) #5857

Closed
1 task done
jwdunn1 opened this issue Nov 8, 2022 · 1 comment · Fixed by #5858
Closed
1 task done

Default ambient color incorrect (in v1.5.0) #5857

jwdunn1 opened this issue Nov 8, 2022 · 1 comment · Fixed by #5858

Comments

@jwdunn1
Copy link
Contributor

jwdunn1 commented Nov 8, 2022

Most appropriate sub-area of p5.js?

  • WebGL

p5.js version

1.5.0

Steps to reproduce:

  1. Version 1.4.2
    note the ambient color
    https://editor.p5js.org/jwdunn1/sketches/GZTNLHnPd
  2. Version 1.5.0
    note the absence of ambient color (it's suddenly a black cube)
    https://editor.p5js.org/jwdunn1/sketches/Um5DLsasQ
  3. Version 1.5.0 with fix, now matches 1.4.2
    must set the ambientMaterial to 255
    https://editor.p5js.org/jwdunn1/sketches/k6Pi7uvtm

Evident here also: p5js.org/reference/#/p5/lights
The ambient lighting is absent, hence the rendering is much darker.
If ambientMaterial() is set to 255, then the gray returns to what we saw prior to 1.5.0

Cause:

Due to PR#5774, the default ambient color is now 0.
Line 98 in src/webgl/p5.RendererGL.js:

this.curAmbientColor = this._cachedFillStyle = [0, 0, 0, 0];

Potentially repaired by setting this to be [1,1,1,1]
This would match both v1.4.2 and Processing 4.0.1

Snippet:

// is this now required in order to match 1.4.2?
ambientMaterial(255);
@davepagurek
Copy link
Contributor

Thanks for filing this! I noticed this issue too, and found I had to add the same ambientMaterial call to my code. Your proposed fix of defaulting this to [1, 1, 1, 1] makes sense, if you want to make a PR with the change! Otherwise I can also make the change the next time I get around to doing some p5 dev work.

jwdunn1 added a commit to jwdunn1/p5.js that referenced this issue Nov 11, 2022
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