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

Sepia and Invert Effects work but why not the rest? #341

Closed
NTMNathan opened this issue Apr 3, 2020 · 11 comments · Fixed by #342
Closed

Sepia and Invert Effects work but why not the rest? #341

NTMNathan opened this issue Apr 3, 2020 · 11 comments · Fixed by #342

Comments

@NTMNathan
Copy link

NTMNathan commented Apr 3, 2020

Hi, I'm developing an Image Gen API website and making effects endpoints such as Grayscale, Sepia, Silhouette etc.

I could be wrong here but for some reason, only my Sepia and Invert endpoints are working, when trying to require grayscale, blur, brightness, darkness and the rest. They either result in a black or white generated image. See screenshots and code for what I mean.

Removed links to avoid people copying code since issue was fixed - sorry
for the Grayscale code.
for Sepia code.
Screenshots: https://imgur.com/a/PIxvdls

Using 3.1.0 as the version.

@kyranet
Copy link
Owner

kyranet commented Apr 3, 2020

node-canvas implements extra effects which are non-standard, which canvas-constructor uses since it has been used for Node.js exclusively (this will change in the future):

https://github.com/kyranet/canvasConstructor/blob/db85b53bf71b62283461ba0d2919be8b85dde0b7/src/lib/filters/filters.js#L19-L25

It is possible that they changed the behaviour of the global composite operation, I'd have to check, though.

silhouette works on images that are transparent, it modifies the colour for all pixels without touching the alpha channel.

For non-transparent images, you have threshold, which should definitely work, after all, it uses a mathematical formula for extracting the luminosity of each pixel, compares it with threshold, and if higher, it prints white, else black (invertedThreshold does the opposite)

Also, to prettify your code, instead of this:

let canvas = new Canvas(width, height)
	.addImage(avatar, 0, 0, width, height)
        
canvas = grayscale(canvas)
	.toBuffer();

You can do this:

const canvas = new Canvas(width, height)
	.addImage(avatar, 0, 0, width, height)
	.process(grayscale)
	.toBuffer();

Those utils that have more than one argument (e.g. threshold) you pass extra arguments to process, docs available here.

@NTMNathan
Copy link
Author

Alright thanks, i've tested out silhouette, yes. It works on transparent images. Threshold, no not really for me.

Thanks for letting me know of the cleaner code. Yes, I am making my existing Image Gen commands from my Discord Bot to the image gen website.

Was also not really able to understand the extra arguments for process. If you could give me an example for threshold, grayscale etc. Would be cool.

@kyranet
Copy link
Owner

kyranet commented Apr 3, 2020

Have you tried more images in threshold? If almost the entire surface of the image is pastel, threshold simply cannot work due to how its formula works.

For example threshold has 2 arguments, canvas and threshold, where you'd normally do threshold(canvas, 140) in a functional way, you'd do .process(threshold, 140) instead.

@kyranet
Copy link
Owner

kyranet commented Apr 3, 2020

I can confirm threshold working on canvas-constructor@3.1.0 with canvas@2.6.0 and node.js@12.16.1 with this image:

image

greyscale failed, if the global composite was removed, it'll be a shame, since I was abusing the fact I could do the conversion in the C++ layer (which is many times faster than in the JavaScript layer).

Maybe for a canvas-constructor@4.0.0 written in TypeScript I'll try out re-writting the effects to run on WebAssembly instead, that should give a considerable speed boost and performance consistency along all filters.

@NTMNathan
Copy link
Author

Whats working for me:

  • Sepia
  • Blur (barely blurs best if the args set at 9)
  • Threshold and InvertedThreshold
  • Brightness

the rest aren't working unfortunately

@kyranet
Copy link
Owner

kyranet commented Apr 3, 2020

I'm honestly surprised blur worked for you, given that it did not when I tested it. Can you list those that aren't working, so I can check them as soon as possible, please?

@NTMNathan
Copy link
Author

Sure Kyra, I can do that for you :)

If you're wondering about the Blur, yes, it barely blurs and the args is best set at 9 to actually see the blur. Therefore you can pretty much see a difference. Got my Logo icon I made to compare with the blur effect https://imgur.com/a/GpH1UYQ

The working ones:

  • invert
  • sepia
  • silhouette (Like you said, only with transparent images)
  • threshold and invertedThreshold (Non transparent images)
  • brightness (More like a white colour overlay with an opacity args between 5-100)
  • Blur (See above)

Broken for me:

  • grayscale
  • invertGrayscale
  • darkness
  • sharpen
  • blur (Probably get this one fixed too)
  • convolute (Whatever this one does)

Perhaps, I got some suggestions for more effects that you can possibly make. If you like me to suggest, I can either in the support server or here will be good

@kyranet
Copy link
Owner

kyranet commented Apr 3, 2020

brightness and darkness seem to be working, yeah, it does a white and a black colour overlay with opacity. Is the desired result any other (such as increasing using the HSL workspace, which is more compute-intensive?).

image

@kyranet
Copy link
Owner

kyranet commented Apr 3, 2020

I might drop blur, sharpen, and convolute, someday node-canvas will finally have filters (Automattic/node-canvas#1063) which would allow us to use a bunch of filters that are too computational expensive and hard to implement (I wish this were easy, but every page in Internet points me to a library, which I don't want to implement as a dependency, since they're often focused to browsers).

@kyranet
Copy link
Owner

kyranet commented Apr 3, 2020

Hey, do you want to try #342? I hopefully fixed all the things:

  • grayscale has been changed to a loop
  • invertGrayscale has been changed to a loop
  • brightness has been changed to a loop (alpha should not be touched now)
  • darkness has been changed to a loop (alpha should not be touched now)
  • convolute has been fixed to not mutate its source as it's being used to read, now the destiny buffer is cloned from the one that it has been read from, this should fix blur and sharpen.

@NTMNathan
Copy link
Author

I did a test, please see the post in #342

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

Successfully merging a pull request may close this issue.

2 participants