fixed transparency of background problem

[two-ticks]

Resolves #5175

Changes:

example sketch

• change alpha value according to args[1]

p5.Renderer2D.prototype.background = function(...args) {
  this.drawingContext.save();
  this.resetMatrix();
  if (args[0] instanceof p5.Image) {
    if (args[1] >= 0) {
      // set transparency of background
      const img = args[0];
      const pixels = filters._toPixels(img.canvas);
      const tmpCanvas = document.createElement('canvas');
      tmpCanvas.width = img.canvas.width;
      tmpCanvas.height = img.canvas.height;
      const tmpCtx = tmpCanvas.getContext('2d');
      const id = tmpCtx.createImageData(img.canvas.width, img.canvas.height);
      const newPixels = id.data;
      for (let i = 0; i < pixels.length; i += 4) {
        const r = pixels[i];
        const g = pixels[i + 1];
        const b = pixels[i + 2];
        newPixels[i] = r;
        newPixels[i + 1] = g;
        newPixels[i + 2] = b;
        newPixels[i + 3] = args[1];
      }
      tmpCtx.putImageData(id, 0, 0);
      img.canvas = tmpCanvas;
      this._pInst.image(img, 0, 0, this.width, this.height);
    } else {
      this._pInst.image(args[0], 0, 0, this.width, this.height);
    }

Screenshots of the change:

PR Checklist

• npm run lint passes

[two-ticks]

@montoyamoraga please review this pull request

[montoyamoraga]

hi @two-ticks sorry i'm not familiar with this section of the code,
somebody else will review it :)

[limzykenneth]

I think the implementation here is probably going to be way too slow especially with larger canvases since it requires going through the pixels array of the whole canvas. Can this possibly be implemented in another way?

[two-ticks]

Maybe we can implement it with globalAlpha also. I also tried to implement this method but implementations failed. tint() also uses such method(as in pull request) so it can be an immediate fix.

[limzykenneth]

I'm not quite sure what you mean by globalAlpha, can you elaborate on this a bit more?

[two-ticks]

I have tried something similar to this and some more variations of it. Idea was to change globalAlpha value rather than changing alpha value of each pixel. example

  if (args[0] instanceof p5.Image) {
    if (args[1] >= 0) {
      // set transparency of background
      const ctx = this.getContext("2d");
      ctx.globalAlpha = args[1]; // Turn transparency on
      this._pInst.image(img, 0, 0, this.width, this.height);
    } else {
      this._pInst.image(args[0], 0, 0, this.width, this.height);
    }

p5.js/src/core/p5.Renderer2D.js

Line 197 in 374acfb

p5.Renderer2D.prototype._getTintedImageCanvas = function(img) {

is using loops

[limzykenneth]

Ok, globalAlpha seem like a possible fix. As long as there won't be unexpected bug elsewhere as a result of using it, we can have a try at it.

[Qianqianye]

Thanks @two-ticks for working on it. @limzykenneth, any recommended next steps to merge this PR?
Are there anything @Palaksharma23 can work on, as @two-ticks mentioned in issue #5175 ?

[limzykenneth]

For this we will need an alternate implementation than the one in this PR as it is likely to be too slow. If @Palaksharma23 wants to have a go and @two-ticks isn't currently working on this I'm happy to review any new changes. The solution doesn't have to use globalAlpha as mentioned if there are better alternatives.

[two-ticks]

I would be very happy if @Palaksharma23 wants to work on this and get assigned. Waiting for @Palaksharma23 to reply.

[yashlamba]

Hey @two-ticks @limzykenneth, I am new to p5.js and got issue #5175 in my mail from CodeTriage (thought of investigating it further to learn). I looked at #5225 and the other globalAlpha based implementation with some modification (they both work as expected), I couldn't find any other optimal way to do it as well. Happy to test/look further if you can suggest any leads:)

[two-ticks]

I think globalAlpha implementation would be the fastest, I also couldn't find any other way. If possible share your approach and code so maintainers can review (and assign it to you). @limzykenneth can #5175 issue be assigned to @yashlamba ?

[limzykenneth]

I can't assign @yashlamba on #5175 as only participants of the thread can be assigned but @yashlamba feel free to work on this. Thanks!

[yashlamba]

Thanks @limzykenneth, I'll go ahead and open a new PR.

[Qianqianye]

Thanks @two-ticks and @yashlamba. I will close this PR since it has been fixed on the recent merge of #5683!