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

Shaders aren't applied to things drawn with image() and with texture() #6327

Open
1 of 17 tasks
RandomGamingDev opened this issue Aug 4, 2023 · 8 comments
Open
1 of 17 tasks

Comments

@RandomGamingDev
Copy link
Contributor

RandomGamingDev commented Aug 4, 2023

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build Process
  • Unit Testing
  • Internalization
  • Friendly Errors
  • Other (specify if possible)

p5.js version

1.7.0

Web browser and version

115.0.5790.111

Operating System

Windows 10

Steps to reproduce this

Steps:

  1. Draw something either using image() or texture().
  2. Try to apply a custom shader

Snippet:

Here's the sketch containing the code: https://editor.p5js.org/PotatoBoy/sketches/KRrVBTkjH

const vertShader = `
  attribute vec3 aPosition;
  attribute vec2 aTexCoord;

  uniform mat4 uModelViewMatrix;
  uniform mat4 uProjectionMatrix;

  varying vec2 vTexCoord;

  void main() {
    vTexCoord = aTexCoord;
    vec4 pos = vec4(aPosition, 1.0);
    vec4 wPos = uProjectionMatrix * uModelViewMatrix * pos;
    //gl_Position = wPos;
    gl_Position = vec4(0.0); // this is supposed to be a null coordinate that stops this from displaying anything
  }`;
const fragShader = `
  #ifdef GL_ES
  precision mediump float;
  #endif

  varying vec2 vTexCoord;

  uniform sampler2D uSampler;

  void main() {
    //gl_FragColor = texture2D(uSampler, vTexCoord);
    gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0); // this is supposed to be pure red
  }
`;

let img;
let canvas;
let defShad;

function preload() {
  img = loadImage('test.png');
}

function setup() {
  canvas = createCanvas(400, 400, WEBGL);
  noStroke();
  defShad = createShader(vertShader, fragShader);

  canvas.getTexture(img).setInterpolation(NEAREST, NEAREST);
}

function draw() {
  background(0);
  shader(defShad);
  {
    // The shaders didn't apply to what was drawn with image() and to a shape with a texture applied with texture()
    image(img, -width / 2, -height / 2, width, height);
    push();
    {
      texture(img);
      rect(0, 0, 100, 100);
    }
    pop();

    // The shaders only apply to this
    rect(-100, -100, 100, 100);
  }
  resetShader();
}
@RandomGamingDev
Copy link
Contributor Author

I think there are 2 best routes to go here based on whether or not this is intended behavior. If this isn't intended behavior we'll have to change it so that those shaders can be overridden, and if it is intended behavior we'll have to add the fact that those shaders can't be overridden to documentation.

@aferriss
Copy link
Contributor

aferriss commented Aug 4, 2023

I think you're mixing things here. If you want to pass an image to a shader you should use the shader.setUniform() function.

I believe image() was not designed to be used with a shader.

texture() is meant to be used in tandem with a piece of geometry like plane() or rect().

However, I am seeing some odd behavior in your example when I try to set a shader after calling texture.

texture(img);
shader(defShad);
rect(0, 0, 100, 100);

In this case I would have thought that the shader function would take over, but it seems like the texture is somehow overriding the shader. If I add a call to fill() between texture and shader, the shader can take over again. Not sure if this is intended, but it does seem a little weird.

@davepagurek
Copy link
Contributor

Right now, enabling lights or setting a texture makes p5 try to use a light shader, so if a user shader doesn't meet these requirements, it gets overridden:

isLightShader() {
return (
this.attributes.aNormal !== undefined ||
this.uniforms.uUseLighting !== undefined ||
this.uniforms.uAmbientLightCount !== undefined ||
this.uniforms.uDirectionalLightCount !== undefined ||
this.uniforms.uPointLightCount !== undefined ||
this.uniforms.uAmbientColor !== undefined ||
this.uniforms.uDirectionalDiffuseColors !== undefined ||
this.uniforms.uDirectionalSpecularColors !== undefined ||
this.uniforms.uPointLightLocation !== undefined ||
this.uniforms.uPointLightDiffuseColors !== undefined ||
this.uniforms.uPointLightSpecularColors !== undefined ||
this.uniforms.uLightingDirection !== undefined ||
this.uniforms.uSpecular !== undefined
);
}

In the long term, I'd like an easier way of just overwriting e.g. the body of the fragment shader so that one doesn't have to worry about what uniforms are present, and there's some discussion of that here #6144, but in the short term we might want to update that list to include || this.uniforms.uSampler !== undefined.

@RandomGamingDev
Copy link
Contributor Author

I'm not sure whether or not image() is meant to be used with shaders, but I just thought that that'd be a better option since it'd match a lot more nicely with the general p5.js style when programming, which was why I thought that it'd be better to do that then just to use pure uniforms.
Also, the second example does seem pretty weird.
I also agree with @davepagurek

@DarKDevz
Copy link

DarKDevz commented Sep 8, 2023

@RandomGamingDev
you can just set the texture as an uniform yourself
code from texture function:

shader.setUniform('uSampler', texture);

and if your example is just tinting an image you can simply call tint with the color

@RandomGamingDev
Copy link
Contributor Author

Ik u can, but the idea's that this should work too.

@DarKDevz
Copy link

Actually setting the uniform of uSampler didn't work for me, so i just renamed the variable to something else and it worked, any updates on this issue?

@RandomGamingDev
Copy link
Contributor Author

It's an issue with the shaders that originally caused image() to not work with shaders. If you'd like to know more I'd recommnd talking with @davepagurek

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: System Level Changes
Development

No branches or pull requests

4 participants