ambientMaterial() requires a call to fill()

[kilian2323]

Most appropriate sub-area of p5.js?

• Accessibility (Web Accessibility)
• Build tools and processes
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Friendly error system
• Image
• IO (Input/Output)
• Localization
• Math
• Unit Testing
• Typography
• Utilities
• [ x] WebGL
• Other (specify if possible)

Details about the bug:

• p5.js version: 1.3.0
• Web browser and version: Chrome 93.0.4577.82
• Operating System: Windows 10
• Steps to reproduce this:

  ambientLight(255);
  noFill();        // calling noFill(), because I want to draw an ellipse without fill
  stroke(0);
  ellipse(0,0,150,150);
  fill(80,200,300); // must call to fill here (no matter with which color as argument), otherwise the following ambient material is not applied
  ambientMaterial(200,100,0);
  translate(150,0,0);
  box(30);

When using ambientMaterial() after a call to noFill(), no material is rendered. This is confusing, since ambient Material() shold work independently of the fill() / noFill() toggle logic. After all, ambientMaterial() requires its own color to be passed as anargument, and so does fill(). being forced to use fill() before ambientMaterial() creates unnecessarily bloated code. The logical question here would be: What color should I pass to fill() if the material color is actually specified in ambientMaterial()? The answer is: It doesn't matter which color is passed to fill(). A possible improvement might be to provide a fill() method which takes no arguments and really just enables any kind of fill. The more elegant solution, however, would be to make ambientMaterial() not related to noFill() at all.

[stalgiag]

Hi @kilian2323 agreed! Calling one of the material functions with a color should turn filling back on. Currently the only function that can turn on filling after noFill() is fill(). This could be changed by adding a this._renderer._doFill=true to ambientMaterial() .

[PrayasJ]

@stalgiag I was going through this issue and I believe checking for this._renderer._enableLighting to be false in noFill() would also be required else it would override the current rendering settings anyways. It seems to work fine, can you review it?

[ArthurGW]

As a side note, another way to get ambientMaterial(...) to work after noFill() is to first call normalMaterial(), which also contains a call to set this._renderer._doFill=true. Since there is precedent for this when setting normal material, it makes sense to me to do the same when setting ambient material (and any other).