createShader() assumes _renderer causing bug when used with p5.Graphics

[duskvirkus]

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
• WebGL
• Other (specify if possible)

Details about the bug:

• p5.js version: 1.3.0
• Web browser and version: Firefox 86.0 (64-bit) and Chromium 87.0.4280.66 (Official Build) Built on Ubuntu
• Operating System: Zorin OS 15 (based on Ubuntu 18.04)
• Steps to reproduce this:

Sketches That Demonstrate the Problem

bug when using p5.Graphics with createShader(): https://editor.p5js.org/duskvirkus/sketches/dJlFCUjEi
comparison with loadShader(): https://editor.p5js.org/duskvirkus/sketches/LiqLNendr
work around: https://editor.p5js.org/duskvirkus/sketches/6EsuQi93w
(see below for full sketch code)

Explanation of the Problem

createShader() creates a new shader with this._renderer see:

p5.js/src/webgl/material.js

Line 176 in 9d12b8c

return new p5.Shader(this._renderer, vertSrc, fragSrc);

Which means that when using it on a p5.Graphics object the shader breaks (silently, which was frustrating).

Compared to loadShader() which doesn't specify a _renderer for the shader see:

p5.js/src/webgl/material.js

Line 71 in 9d12b8c

const loadedShader = new p5.Shader();

This isn't a problem because when the shader() function is called it will use this._renderer if s._renderer is undefined see:

p5.js/src/webgl/material.js

Line 255 in 9d12b8c

s._renderer = this._renderer;

Which might actually be better because then it'll use the _renderer of a p5.Graphics object when called on that.

The work around I came up with was including <shader-variable>._renderer = undefined when using a shader that was created with createShader().

Suggested Solution

I'd like to suggest removing the _renderer property because it's not defined when someone loads a shader. And it causes problems when working with p5.Graphics objects. This problem would probably also occur if you were trying to use a shader on two (or more) p5.Graphics objects in the same sketch. So I think it would make more sense to just use whatever this._renderer is when shader() is called by default. Does someone have a better suggestion? Would this break other things? I could work on a pull request if not.

It also looks like it might worth reworking shader creation so it's more consistent and has more error message. (Something I'd also be interested in working on.)

Full Sketch Code

Be sure to run with 1.3.0 because there's another bug in 1.2.0 that will cause them to break.

bug when using p5.Graphics with createShader():

// sketch.js
let sh;
let tex;

function setup() {
  createCanvas(400, 400, WEBGL);
  noStroke();
  
  sh = createShader(
    `
// vert
#ifdef GL_ES
precision mediump float;
#endif

attribute vec3 aPosition;

void main() {
vec4 positionVec4 = vec4(aPosition, 1.0);
positionVec4.xy = positionVec4.xy * 2.0 - 1.0; 
gl_Position = positionVec4;
}
`,
    `
// frag
#ifdef GL_ES
precision mediump float;
#endif

uniform vec2 u_resolution;

void main() {
vec2 st = gl_FragCoord.xy/u_resolution.xy; 
gl_FragColor = vec4(st.x,0.0,1.0,1.0); // R,G,B,A
}
`
  );
  
  tex = createGraphics(width, height, WEBGL);
  tex.noStroke();
}

function draw() {
  background(255, 0, 0);
  
  sh.setUniform('u_resolution', [width, height]);

  tex.shader(sh);
  tex.rect(0, 0, width, height);
  texture(tex);
  
  translate(-width/2, -height/2);
  rect(0,0,width, height);
}

comparison with loadShader():

// shader.vert
// vert
#ifdef GL_ES
precision mediump float;
#endif

attribute vec3 aPosition;

void main() {
  vec4 positionVec4 = vec4(aPosition, 1.0);
  positionVec4.xy = positionVec4.xy * 2.0 - 1.0; 
  gl_Position = positionVec4;
}

// shader.frag
// frag
#ifdef GL_ES
precision mediump float;
#endif

uniform vec2 u_resolution;

void main() {
  vec2 st = gl_FragCoord.xy/u_resolution.xy; 
  gl_FragColor = vec4(st.x,0.0,1.0,1.0); // R,G,B,A
}

// sketch.js
let sh;
let tex;

function preload() {
  sh = loadShader('shader.vert', 'shader.frag');
}

function setup() {
  createCanvas(400, 400, WEBGL);
  noStroke();
  
  tex = createGraphics(width, height, WEBGL);
  tex.noStroke();
}

function draw() {
  background(255, 0, 0);
  
  sh.setUniform('u_resolution', [width, height]);

  tex.shader(sh);
  tex.rect(0, 0, width, height);
  texture(tex);
  
  translate(-width/2, -height/2);
  rect(0,0,width, height);
}

work around:

// sketch.js
let sh;
let tex;

function setup() {
  createCanvas(400, 400, WEBGL);
  noStroke();
  
  sh = createShader(
    `
// vert
#ifdef GL_ES
precision mediump float;
#endif

attribute vec3 aPosition;

void main() {
vec4 positionVec4 = vec4(aPosition, 1.0);
positionVec4.xy = positionVec4.xy * 2.0 - 1.0; 
gl_Position = positionVec4;
}
`,
    `
// frag
#ifdef GL_ES
precision mediump float;
#endif

uniform vec2 u_resolution;

void main() {
vec2 st = gl_FragCoord.xy/u_resolution.xy; 
gl_FragColor = vec4(st.x,0.0,1.0,1.0); // R,G,B,A
}
`
  );
  
  sh._renderer = undefined;
  
  tex = createGraphics(width, height, WEBGL);
  tex.noStroke();
}

function draw() {
  background(255, 0, 0);
  
  sh.setUniform('u_resolution', [width, height]);

  tex.shader(sh);
  tex.rect(0, 0, width, height);
  texture(tex);
  
  translate(-width/2, -height/2);
  rect(0,0,width, height);
}

[duskvirkus]

Might have made this issue prematurely. Got some input from Naoto over on the processing forum and have to see if my assessment of the problem was correct.

Thread on discourse: https://discourse.processing.org/t/createshader-not-working-on-p5-graphics/28617

[aferriss]

Hi @duskvirkus !

Thanks for the detailed writeup. If I'm understanding the problem correctly, I think you just need to call createShader as a member function of your graphics object. In your example that would be tex.createShader(vertSrc, fragSrc)

  tex = createGraphics(width, height, WEBGL);

  sh = tex.createShader(
    `
// vert
#ifdef GL_ES
precision mediump float;
#endif

attribute vec3 aPosition;

void main() {
vec4 positionVec4 = vec4(aPosition, 1.0);
positionVec4.xy = positionVec4.xy * 2.0 - 1.0; 
gl_Position = positionVec4;
}
`,
    `
// frag
#ifdef GL_ES
precision mediump float;
#endif

uniform vec2 u_resolution;

void main() {
vec2 st = gl_FragCoord.xy/u_resolution.xy; 
gl_FragColor = vec4(st.x,0.0,1.0,1.0); // R,G,B,A
}
`
  );  

[duskvirkus]

@aferriss great solution! I didn't think of doing that.

Might be worth adding an example to createShader() but seems like this might be an obscure problem so don't know how many people would benefit from that. I still see an edge case where someone might want to use a shader on two or more p5.Graphics objects but don't know if that's worth working on because I could see a number of alternatives solutions to that problem.

Closing this issue as it seems like my methodology was wrong and I think I better understand how things are working behind the scenes.

[ofZach]

Just jumping on in this closed issue to mention that I also stumbled on this and find this a little inconsistent. See also related --

processing/p5.js-website#1231

As a relative p5js beginner, it's not immediately clear what the difference between createCanvas and createGraphics is, or how you can get a "graphics" object from a canvas allocated to webgl -- my intuition is that if I have a shader working with createCanvas(...., WEBGL) and loadShader -- it should also just work with createShader. Or if not, there should be a clear / easy way to understand how to get it to work. As is, it fails silently and is a little hard to debug.

In my case, I was trying to switch in the same code from preloading a shader to loading from a string (for example, in case you wanted to process the shader, etc). Some more concrete examples about how you might switch these up and/or making it more unified I think will help.