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

Performance improvement for tint #3610

Closed Ajayneethikannan closed 5 months ago

Ajayneethikannan commented 5 years ago

Nature of issue?

Most appropriate sub-area of p5.js?

Which platform were you using when you encountered this?

Details about the bug:

Feature enhancement details:

Currently , when the user sets a tint to draw images , the sketch suffers a huge reduction in performance ,as the function _getTintedCanvas runs every time any image is drawn using the p5.Renderer2d.prototype.image function , manipulating every pixel of the image's canvas and returning a new tinted canvas ( here I think we can make a small speed improvement by using globalCompositeOperation = 'multiply' to create the tinted image, instead of manually multiplying every pixel with the tint ).

When many images are present , or high resolution images are used , this can lead to performance issues. So curently, the safest place to use tint is only in setup , as the canvas is only drawn once and the tint calculations are done once.

New feature details:

Some ideas which can be implemented are:

1.Caching the previous tinted canvas:

For every p5.Image object, if we are drawing with a specific tint for that image, we can store the last tinted image canvas, returned by

cnv = this._getTintedImageCanvas(img);

along with the p5.Image instance, as

img._prevTintedImage = cnv;
img._prevTint = tint;

where img refers to a p5.Image instance In case the img hasn't been modified (which we can check) or the tint hasn't changed, we can reuse this canvas. Or we can calculated the new tinted canvas and store it here. This method can be used when the image is not modified frequently, or the tint for the particular image is not changed frequently.

(But this method cannot be applied for video elements )

2.Ability to create new tinted image :

if we can add a functionality like

let tImage = img.tint(12, 121, 12);

we can draw the tinted images as new images, without calling _getTintedCanvas every time.

Would love to hear more from the community if this feature should be considered , Thank you!

micuat commented 5 years ago

you're right, I guess the current implementation of tint is quite useless in draw loop. One way of solving this without changing p5js is to use createGraphics https://editor.p5js.org/micuat/sketches/9xhpkGVF8 which I believe is more intuitive (for a Processing user)

Ajayneethikannan commented 5 years ago

Wow , that's a nice work around @micuat ! If you think it is more intuitive , would this be a good feature implementation,

if we can add a functionality like

let tImage = img.tint(12, 121, 12);

we can draw the tinted images as new images, without calling _getTintedCanvas every time.

instead of using the tinted graphics instance, a new tinted image will be created, which can be used normally in the draw function.

Thank you @micuat !

micuat commented 5 years ago

and you can always hack the image function https://editor.p5js.org/micuat/sketches/S82r42HqN

as you suggested, chaining these image filters is a cool functionality but I don't know if it's something needed for the community. I guess we need input from other people

SrBrahma commented 5 years ago

Same issue here. Tinting is consuming a nice part of the CPU.

stalgiag commented 5 years ago

I feel like @micuat's first suggestion is the intended way to do this. Maybe a documentation change could held nudge people in this direction? I feel like caching the canvas might be a bit too much for this specific problem.

fxcb commented 4 years ago

I would like to change the tint color during the draw(), any idea to improve tint performance on this scenario?

feuerball11 commented 4 years ago

As another performance improvement, for alpha drawing only: You can use drawingContext.globalAlpha = 0.5; before drawing the image, and restoring it with drawingContext.globalAlpha = 1; This allows for performant alpha drawing.

Maybe this could be patched into Processing with a function like alpha(a) noAlpha() ?

micuat commented 4 years ago

@fcasanellas I made a simple sketch of a real-time version using a shader. But it seems the texture coordinate is screwed (you see wireframe of the plane)

https://editor.p5js.org/micuat/sketches/OMRlanQW1

@stalgiag do you have any idea why this is happening? I guess gl_FragCoord does not follow the texture coordinate given by plane()? Do I need to explicitly set texCoord in vertex shader?

b2renger commented 4 years ago

@micuat

I usually do it like this, as you mentioned I usually do explicitly set the texCoord in vs :

https://editor.p5js.org/b2renger/sketches/Pu9PalyUH

Still it seems something is off with plane() so I used a rect instead and everything seems ok

micuat commented 4 years ago

thanks that works perfect. I don't know what the direction now for this thread, but I guess, from p5.js's policy, we won't integrate these hacks into the core. Nevertheless it may be useful to make a documentation about shader based tint (or caching tinted image if static) and point it from the reference.

b2renger commented 4 years ago

I agree, it could be nice to have small library of shaders to manage things like tint, blends or masks in a simple and efficient way - with comprehensive examples

Would love to participate if it were to happen :) I also have a few things ready to go : https://b2renger.github.io/p5js-shaders/shader-blends/ https://b2renger.github.io/p5js-shaders/shader-mask/index.html

quitmeyer commented 4 years ago

Agreed, running into this same issue while trying to use FFT and tinting an image

davepagurek commented 3 years ago

How would you feel about replacing the current implementation with one that preserves the same behaviour but using globalCompositeOperation to do the tinting? The original suggestion of using the multiply blend mode does get the job almost done (it only works on rgb but not alpha), with destination-in doing the alpha tint.

I made a test here and it tints a 1280x1280 image much faster than the existing tint. I have an example here, try commenting out the redefinition of _getTintedImageCanvas to see the current tint's performance: https://editor.p5js.org/davepagurek/sketches/D_ehdpTjO

If you feel OK about a method like this, I can turn this into a PR + unit tests.

limzykenneth commented 3 years ago

@davepagurek I think we can consider something like that if it isn't considered a breaking change.

captainmpc commented 2 years ago

As another performance improvement, for alpha drawing only: You can use drawingContext.globalAlpha = 0.5; before drawing the image, and restoring it with drawingContext.globalAlpha = 1; This allows for performant alpha drawing.

Maybe this could be patched into Processing with a function like alpha(a) noAlpha() ?

Ok maybe this is off topic considering the first function of tint() but as we need to call tintto make an image transparent , this idea seems to be good. Or maybe add ALPHA + factor parameter to blendMode() do the trick :)

24amesquir commented 2 years ago

Video of Performance hit with FPS I recorded a video of me using hue and I am having the same problem it is 5 times slower with the one modifier

Nemo1999 commented 2 years ago

I made a test here and it tints a 1280x1280 image much faster than the existing tint. I have an example here, try commenting out the redefinition of _getTintedImageCanvas to see the current tint's performance: https://editor.p5js.org/davepagurek/sketches/D_ehdpTjO

If you feel OK about a method like this, I can turn this into a PR + unit tests.

Thank you so much! @davepagurek I am making a complex 2D animation which requires tinting multiple images with continuously changing alpha value. (See here for a demo) This effect is impossible with default implementation, but your solution makes it work smoothly !

Qianqianye commented 5 months ago

Thank you all for your feedback. @davepagurek, are we good to close this issue?

davepagurek commented 5 months ago

I think so!