Add required parameters in _flipPixels function

[mohitbalwani]

Resolves #6724

Changes:

The flipPixels function don't has access to width and height so it will give undefined error whenever we call it.

Screenshots of the change:

PR Checklist

• npm run lint passes
• Inline documentation is included / updated
• Unit tests are included / updated

[welcome]

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

[davepagurek]

Thanks for the fix! Does this still work correctly when you've set pixelDensity(2) in setup? I wonder if we need to be multiplying by the density here

[mohitbalwani]

I tried using pixelDensity() in setup but nothing was being rendered. Am I missing something in this code?

let sketch = function(p) {
  p.setup = function() {
    pixelDensity(2);
    p.createCanvas(500, 500, p.WEBGL);
  };

  p.draw = function() {
  };

  p.keyPressed = function() {
    if (p.key === 's') {
      p.saveGif('test', 5);
    }
  }
};

let myp5 = new p5(sketch);

[davepagurek]

In instance mode I think you need p.pixelDensity(2).

[mohitbalwani]

Oh ya. Thanks!

The current fix works as expected when setting different densities of pixels.

I also checked the behaviour by multiplying the dimensions with density while passing in function call but I was getting another console error saying : offset is out of bounds. So, looks like some calculations go wrong inside flipPixels function in the case when we multiply dimensions with density.

I saw a similar implementation in readPixelsWebGL() function and stackoverflow answers (https://stackoverflow.com/questions/41969562/how-can-i-flip-the-result-of-webglrenderingcontext-readpixels) so looks like we should not consider multiplying by density here? Or maybe we need to recalculate the flip pixels logic considering density (I tried that but wasn't able to figure out).

[davepagurek]

Ah, wait, logically it should have it, but we just set it to 1 here while exporting:

p5.js/src/image/loading_displaying.js

Line 311 in 58821e2

this.pixelDensity(1);

So I think width/height on its own is fine!

[allcontributors]

@davepagurek

I've put up a pull request to add @mohitbalwani! 🎉