Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement for tint #3610

Closed
5 of 7 tasks
Ajayneethikannan opened this issue Mar 16, 2019 · 19 comments
Closed
5 of 7 tasks

Performance improvement for tint #3610

Ajayneethikannan opened this issue Mar 16, 2019 · 19 comments

Comments

@Ajayneethikannan
Copy link
Contributor

Ajayneethikannan commented Mar 16, 2019

Nature of issue?

  • Found a bug
  • Existing feature enhancement
  • New feature request

Most appropriate sub-area of p5.js?

  • Core/Environment/Rendering

Which platform were you using when you encountered this?

  • Mobile/Tablet (touch devices)
  • Desktop/Laptop
  • Others (specify if possible) Device independent

Details about the bug:

  • p5.js version: 0.7.3
  • Web browser and version: Browser independent
  • Operating System: OS independent
  • Steps to reproduce this:

https://editor.p5js.org/TheSkepticSniper/sketches/go2nvAoAF
This sketch can be used for checking the performance of tint.
(Uncomment the tint line to see the frame rate drop)

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
Copy link
Member

micuat commented Mar 22, 2019

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
Copy link
Contributor Author

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
Copy link
Member

micuat commented Mar 22, 2019

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
Copy link

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

@stalgiag
Copy link
Contributor

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
Copy link

fxcb commented Apr 29, 2020

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

@feuerball11
Copy link

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
Copy link
Member

micuat commented Jun 4, 2020

@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
Copy link
Member

b2renger commented Jun 4, 2020

@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
Copy link
Member

micuat commented Jun 4, 2020

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
Copy link
Member

b2renger commented Jun 4, 2020

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
Copy link

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

@davepagurek
Copy link
Contributor

davepagurek commented Sep 15, 2021

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
Copy link
Member

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

@captainmpc
Copy link

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
Copy link

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
Copy link

Nemo1999 commented Jun 5, 2022

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
Copy link
Contributor

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

@davepagurek
Copy link
Contributor

I think so!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests