bug when use callback in loadShader()

[Omri-Bergman]

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

v1.4.1

Web browser and version

105.0.5195.127

Operating System

Windows

Steps to reproduce this

Steps:

1. load the shader on the setup and use a callback to know when it's done
2. console log a message on the setup()
3. run the sketch and see its not stop running the setup()

Snippet:

let theShader;
let loaded = false;

function setup() {
  pixelDensity(1);
  createCanvas(windowWidth, windowHeight, WEBGL);
  theShader = loadShader('shader.vert', 'shader.frag',
    () => loaded = true
  );
  noStroke();
  console.log("DONE SET UP")
}

function draw() {
  if (!loaded) {
    background(20);
    fill(200)
    circle(width/2, height/2, 200);
  } else {
    theShader.setUniform('u_resolution', [width, height]);
    shader(theShader);
    rect(0, 0, width, height);
  }
}

[welcome]

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

[davepagurek]

To clarify, is loaded never being set to true, or is it being set to true but the shader isn't valid? If you provide an editor.p5js.org link with your shader, it might help narrow the problem down.

[Omri-Bergman]

I added to the console.log("DONE SET UP") the load and I see it is set to true.
Also, after a few seconds it raises these errors:

Here is an editor.p5js.org link:
https://editor.p5js.org/omribergman05/sketches/Ac9lnf53WR

[davepagurek]

Thanks for the example! This is an interesting bug, it seems like decrementPreload is causing setup to be run again, which triggers it to load again, which reruns setup again, etc.

In the mean time, it looks like the behaviour is correct if you loadShader in preload instead of setup.

[Omri-Bergman]

In my project I load a buch of shaders so while it's loading I want to run an animation, that's why I can't use the preload() and wanted to do it in the setup() with the callback.
Do you have any idea what I can do to get over the bug/fix it maybe?

[davepagurek]

For now, you can copy-and-paste this into your code to override loadShader. This is the existing method, but it additionally overwrites _decrementPreload with a function that does nothing. It's not a good fix in general but for your project should stop it from looping setup:

p5.prototype.loadShader = function(
  vertFilename,
  fragFilename,
  callback,
  errorCallback
) {
  p5._validateParameters('loadShader', arguments);
  if (!errorCallback) {
    errorCallback = console.error;
  }

  const loadedShader = new p5.Shader();

  const self = this;
  let loadedFrag = false;
  let loadedVert = false;

  const onLoad = () => {
    self._decrementPreload();
    if (callback) {
      callback(loadedShader);
    }
  };
  
  // Prevent actually decrementing preload by turning it into a noop
  self._decrementPreload = () => {}

  this.loadStrings(
    vertFilename,
    result => {
      loadedShader._vertSrc = result.join('\n');
      loadedVert = true;
      if (loadedFrag) {
        onLoad();
      }
    },
    errorCallback
  );

  this.loadStrings(
    fragFilename,
    result => {
      loadedShader._fragSrc = result.join('\n');
      loadedFrag = true;
      if (loadedVert) {
        onLoad();
      }
    },
    errorCallback
  );

  return loadedShader;
};

[Omri-Bergman]

thanks for that patch!
why isn't it a good fix in general? e.i what it might cause?
just to know what bugs it may do in my code if I'll use it

[davepagurek]

Because this disables _decrementPreload entirely, it means that if you try to load anything in preload, it will never finish preloading. I more long-term fix will check whether or not setup has already been run to decide whether or not to run it again. There's a _setupDone variable, so it probably will involve checking that.