Merge pull request #2511 from Spongman/fix-fill-stroke

Fix fill/stroke order bug
remove shader.active flag
begin using _doFill/_doStroke in GL in line with 2D
remove unnecessary userProgram calls
set blend settings at end of draw call

Author: stalgiag

src/color/setting.js

@@ -506,13 +506,29 @@ p5.prototype.fill = function() {
  * rect(20, 20, 60, 60);
  * </code>
  * </div>
+ *
+ * <div modernizr='webgl'>
+ * <code>
+ * function setup() {
+ *   createCanvas(100, 100, WEBGL);
+ * }
+ *
+ * function draw() {
+ *   background(0);
+ *   noFill();
+ *   stroke(100, 100, 240);
+ *   rotateX(frameCount * 0.01);
+ *   rotateY(frameCount * 0.01);
+ *   box(45, 45, 45);
+ * }
+ * </code>
+ * </div>
+ *
  * @alt
  * white rect top middle and noFill rect center. Both 60x60 with black outlines.
+ * black canvas with purple cube wireframe spinning
  */
 p5.prototype.noFill = function() {
-  if (this._renderer.isP3D) {
-    this._renderer.noFill();
-  }
   this._renderer._setProperty('_doFill', false);
   return this;
 };
@@ -531,16 +547,28 @@ p5.prototype.noFill = function() {
  * </code>
  * </div>
  *
+ * <div modernizr='webgl'>
+ * <code>
+ * function setup() {
+ *   createCanvas(100, 100, WEBGL);
+ * }
  *
- * @alt
- *60x60 white rect at center. no outline.
+ * function draw() {
+ *   background(0);
+ *   noStroke();
+ *   fill(240, 150, 150);
+ *   rotateX(frameCount * 0.01);
+ *   rotateY(frameCount * 0.01);
+ *   box(45, 45, 45);
+ * }
+ * </code>
+ * </div>
  *
+ * @alt
+ * 60x60 white rect at center. no outline.
+ * black canvas with pink cube spinning
  */
-
 p5.prototype.noStroke = function() {
-  if (this._renderer.isP3D) {
-    this._renderer.noStroke();
-  }
   this._renderer._setProperty('_doStroke', false);
   return this;
 };

src/webgl/material.js

@@ -170,7 +170,8 @@ p5.prototype.shader = function(s) {
 p5.prototype.normalMaterial = function() {
   this._renderer.drawMode = constants.FILL;
   this._renderer.setFillShader(this._renderer._getNormalShader());
-  this._renderer.noStroke();
+  this._renderer.curFillColor = [1, 1, 1, 1];
+  this.noStroke();
   return this;
 };
 
@@ -262,7 +263,7 @@ p5.prototype.texture = function(tex) {
   shader.setUniform('uSpecular', false);
   shader.setUniform('isTexture', true);
   shader.setUniform('uSampler', tex);
-  this._renderer.noStroke();
+  this.noStroke();
   return this;
 };
 
@@ -303,10 +304,11 @@ p5.prototype.texture = function(tex) {
  * @chainable
  */
 p5.prototype.ambientMaterial = function(v1, v2, v3, a) {
-  var colors = this._renderer._applyColorBlend.apply(this._renderer, arguments);
+  var color = p5.prototype.color.apply(this, arguments);
+  this._renderer.curFillColor = color._array;
 
   var shader = this._renderer._useLightShader();
-  shader.setUniform('uMaterialColor', colors);
+  shader.setUniform('uMaterialColor', this._renderer.curFillColor);
   shader.setUniform('uSpecular', false);
   shader.setUniform('isTexture', false);
   return this;
@@ -349,10 +351,11 @@ p5.prototype.ambientMaterial = function(v1, v2, v3, a) {
  * @chainable
  */
 p5.prototype.specularMaterial = function(v1, v2, v3, a) {
-  var colors = this._renderer._applyColorBlend.apply(this._renderer, arguments);
+  var color = p5.prototype.color.apply(this, arguments);
+  this._renderer.curFillColor = color._array;
 
   var shader = this._renderer._useLightShader();
-  shader.setUniform('uMaterialColor', colors);
+  shader.setUniform('uMaterialColor', this._renderer.curFillColor);
   shader.setUniform('uSpecular', true);
   shader.setUniform('isTexture', false);
   return this;
@@ -362,16 +365,11 @@ p5.prototype.specularMaterial = function(v1, v2, v3, a) {
  * @private blends colors according to color components.
  * If alpha value is less than 1, we need to enable blending
  * on our gl context.  Otherwise opaque objects need to a depthMask.
- * @param  {Number} v1 [description]
- * @param  {Number} v2 [description]
- * @param  {Number} v3 [description]
- * @param  {Number} a  [description]
- * @return {[Number]}  Normalized numbers array
+ * @param  {Number[]} color [description]
+ * @return {Number[]]}  Normalized numbers array
  */
-p5.RendererGL.prototype._applyColorBlend = function(v1, v2, v3, a) {
+p5.RendererGL.prototype._applyColorBlend = function(colors) {
   var gl = this.GL;
-  var color = this._pInst.color.apply(this._pInst, arguments);
-  var colors = color._array;
   if (colors[colors.length - 1] < 1.0) {
     gl.depthMask(false);
     gl.enable(gl.BLEND);

src/webgl/p5.RendererGL.Immediate.js

@@ -115,7 +115,7 @@ p5.RendererGL.prototype.endShape = function(
 ) {
   this._useImmediateModeShader();
 
-  if (this.curStrokeShader.active === true) {
+  if (this._doStroke && this.drawMode !== constants.TEXTURE) {
     for (var i = 0; i < this.immediateMode.vertices.length - 1; i++) {
       this.immediateMode.edges.push([i, i + 1]);
     }
@@ -129,7 +129,7 @@ p5.RendererGL.prototype.endShape = function(
     this._edgesToVertices(this.immediateMode);
     this._drawStrokeImmediateMode();
   }
-  if (this.curFillShader.active === true) {
+  if (this._doFill) {
     this._drawFillImmediateMode(
       mode,
       isCurve,
@@ -258,6 +258,7 @@ p5.RendererGL.prototype._drawFillImmediateMode = function(
         ' not yet implemented in webgl mode.'
     );
   } else {
+    this._applyColorBlend(this.curFillColor);
     gl.enable(gl.BLEND);
     gl.drawArrays(
       this.immediateMode.shapeMode,
@@ -312,6 +313,7 @@ p5.RendererGL.prototype._drawStrokeImmediateMode = function() {
     );
   }
 
+  this._applyColorBlend(this.curStrokeColor);
   gl.drawArrays(gl.TRIANGLES, 0, this.immediateMode.lineVertices.length);
 
   // todo / optimizations? leave bound until another shader is set?

src/webgl/p5.RendererGL.Retained.js

@@ -200,7 +200,7 @@ p5.RendererGL.prototype.drawBuffers = function(gId) {
   this._useColorShader();
   var geometry = this.gHash[gId];
 
-  if (this.curStrokeShader.active !== false && geometry.lineVertexCount > 0) {
+  if (this._doStroke && geometry.lineVertexCount > 0) {
     this.curStrokeShader.bindShader();
 
     // bind the stroke shader's 'aPosition' buffer
@@ -229,11 +229,12 @@ p5.RendererGL.prototype.drawBuffers = function(gId) {
       );
     }
 
+    this._applyColorBlend(this.curStrokeColor);
     this._drawArrays(gl.TRIANGLES, gId);
     this.curStrokeShader.unbindShader();
   }
 
-  if (this.curFillShader.active !== false) {
+  if (this._doFill !== false) {
     this.curFillShader.bindShader();
 
     // bind the fill shader's 'aPosition' buffer
@@ -282,6 +283,7 @@ p5.RendererGL.prototype.drawBuffers = function(gId) {
       );
     }
 
+    this._applyColorBlend(this.curFillColor);
     this._drawElements(gl.TRIANGLES, gId);
     this.curFillShader.unbindShader();
   }

src/webgl/p5.RendererGL.js

@@ -122,10 +122,8 @@ p5.RendererGL = function(elt, pInst, isMainCanvas, attr) {
   this.fill(255, 255, 255, 255);
   //this.stroke(0, 0, 0, 255);
   this.pointSize = 5.0; //default point size
-  this.curStrokeWeight = 2; //default stroke weight
-  this.curStrokeColor = [0, 0, 0, 1];
-  this._setStrokeWeight();
-  this._setStrokeColor();
+  this.strokeWeight(2);
+  this.stroke(0, 0, 0);
   // array of textures created in this gl context via this.getTexture(src)
   this.textures = [];
   this.name = 'p5.RendererGL'; // for friendly debugger system
@@ -425,6 +423,7 @@ p5.RendererGL.prototype.background = function() {
   var _b = _col.levels[2] / 255;
   var _a = _col.levels[3] / 255;
   this.GL.clearColor(_r, _g, _b, _a);
+  this.GL.depthMask(true);
   this.GL.clear(this.GL.COLOR_BUFFER_BIT | this.GL.DEPTH_BUFFER_BIT);
 };
 
@@ -471,77 +470,16 @@ p5.RendererGL.prototype.background = function() {
  */
 p5.RendererGL.prototype.fill = function(v1, v2, v3, a) {
   //see material.js for more info on color blending in webgl
-  var colors = this._applyColorBlend.apply(this, arguments);
-  this.curFillColor = colors;
-  if (this.curFillShader.active === false) {
-    this.curFillShader.active = true;
-  }
+  var color = p5.prototype.color.apply(this._pInst, arguments);
+  this.curFillColor = color._array;
+
   if (this.isImmediateDrawing) {
     this.setFillShader(this._getImmediateModeShader());
   } else {
     this.setFillShader(this._getColorShader());
   }
   this.drawMode = constants.FILL;
-  this.curFillShader.setUniform('uMaterialColor', colors);
-};
-
-/**
- * Does not render fill material
- * @method  noFill
- * @example
- * <div>
- * <code>
- * function setup() {
- *   createCanvas(200, 200, WEBGL);
- * }
- *
- * function draw() {
- *   background(0);
- *   noFill();
- *   stroke(100, 100, 240);
- *   rotateX(frameCount * 0.01);
- *   rotateY(frameCount * 0.01);
- *   box(75, 75, 75);
- * }
- * </code>
- * </div>
- *
- * @alt
- * black canvas with purple cube wireframe spinning
- *
- */
-
-p5.RendererGL.prototype.noFill = function() {
-  this.curFillShader.active = false;
-};
-
-/**
- * Does not render stroke
- * @method  noStroke
- * @example
- * <div>
- * <code>
- * function setup() {
- *   createCanvas(200, 200, WEBGL);
- * }
- *
- * function draw() {
- *   background(0);
- *   noStroke();
- *   fill(240, 150, 150);
- *   rotateX(frameCount * 0.01);
- *   rotateY(frameCount * 0.01);
- *   box(75, 75, 75);
- * }
- * </code>
- * </div>
- *
- * @alt
- * black canvas with pink cube spinning
- *
- */
-p5.RendererGL.prototype.noStroke = function() {
-  this.curStrokeShader.active = false;
+  this.curFillShader.setUniform('uMaterialColor', this.curFillColor);
 };
 
 /**
@@ -576,17 +514,12 @@ p5.RendererGL.prototype.noStroke = function() {
  *
  */
 p5.RendererGL.prototype.stroke = function(r, g, b, a) {
-  if (this.curStrokeShader.active === false) {
-    this.curStrokeShader.active = true;
-  }
   //@todo allow transparency in stroking currently doesn't have
   //any impact and causes problems with specularMaterial
   arguments[3] = 255;
-  var colors = this._applyColorBlend.apply(this, arguments);
-  if (this.curStrokeColor !== colors) {
-    this.curStrokeColor = colors;
-    this._setStrokeColor();
-  }
+  var color = p5.prototype.color.apply(this._pInst, arguments);
+  this.curStrokeColor = color._array;
+  this.curStrokeShader.setUniform('uMaterialColor', this.curStrokeColor);
 };
 
 /**
@@ -630,28 +563,13 @@ p5.RendererGL.prototype.stroke = function(r, g, b, a) {
  *
  */
 p5.RendererGL.prototype.strokeWeight = function(w) {
-  if (this.curStrokeShader.active === false) {
-    this.curStrokeShader.active = true;
-  }
   if (this.curStrokeWeight !== w) {
     this.pointSize = w;
     this.curStrokeWeight = w;
     this.curStrokeShader.setUniform('uStrokeWeight', w);
   }
 };
 
-p5.RendererGL.prototype._setStrokeWeight = function() {
-  // this should only be called after an appropriate call
-  // to shader() internally....
-  this.curStrokeShader.setUniform('uStrokeWeight', this.curStrokeWeight);
-};
-
-p5.RendererGL.prototype._setStrokeColor = function() {
-  // this should only be called after an appropriate call
-  // to shader() internally....
-  this.curStrokeShader.setUniform('uMaterialColor', this.curStrokeColor);
-};
-
 /**
  * Returns an array of [R,G,B,A] values for any pixel or grabs a section of
  * an image. If no parameters are specified, the entire image is returned.
@@ -874,8 +792,7 @@ p5.RendererGL.prototype.setFillShader = function(s) {
     // safe to do this multiple times;
     // init() will bail early if has already been run.
     this.curFillShader.init();
-    this.curFillShader.useProgram();
-    this.curFillShader.active = true;
+    //this.curFillShader.useProgram();
   }
   // always return this.curFillShader, even if no change was made.
   return this.curFillShader;
@@ -893,8 +810,7 @@ p5.RendererGL.prototype.setStrokeShader = function(s) {
     // safe to do this multiple times;
     // init() will bail early if has already been run.
     this.curStrokeShader.init();
-    this.curStrokeShader.useProgram();
-    this.curStrokeShader.active = true;
+    //this.curStrokeShader.useProgram();
   }
   // always return this.curLineShader, even if no change was made.
   return this.curStrokeShader;
@@ -943,6 +859,7 @@ p5.RendererGL.prototype._useImmediateModeShader = function() {
     // note that if we're using the texture shader...
     // this shouldn't change. :)
   }
+  return this.curFillShader;
 };
 
 p5.RendererGL.prototype._getLightShader = function() {