processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.67k stars 3.33k forks source link

bug when use callback in loadShader() #5810

Closed Omri-Bergman closed 2 years ago

Omri-Bergman commented 2 years ago

Most appropriate sub-area of p5.js?

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[bot] commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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: image

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

davepagurek commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.