Add shaders for filter() constants, and use them by default in P2D

[wong-justin]

Resolves #4820
Continuation of #6237

Changes (in progress):

• add fragment shader source files for BLUR, INVERT, etc
• replace original pixels filter() implementation with those shader filters
• add option to revert back to original pixels filter()

(This PR is a working draft that's part of the GSoC 2023 project 'Supporting shader-based filters in p5.js", mentored by @aferriss and @aceslowman)

---

PR Checklist

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

[wong-justin]

New filter() signature with this approach:

[aceslowman]

I think this is looking good, and from what I can see, I don't think useWebGL is adding too much complexity, it looks like you've gone about it in a straightforward way.

[wong-justin]

The POSTERIZE shader mostly works, but it looks a little different (and a little worse imo) compared to the original pixels POSTERIZE.
example sketch with old pixels filter
example sketch with this new shader

Pixels filter code for comparison of implementation details:

p5.js/src/image/filters.js

Lines 289 to 308 in 6d3405f

	posterize(canvas, level) {
	const pixels = Filters._toPixels(canvas);
	if (level < 2 || level > 255) {
	throw new Error(
	'Level must be greater than 2 and less than 255 for posterize'
	);
	}
	const levels1 = level - 1;
	for (let i = 0; i < pixels.length; i += 4) {
	const rlevel = pixels[i];
	const glevel = pixels[i + 1];
	const blevel = pixels[i + 2];
	pixels[i] = ((rlevel * level) >> 8) * 255 / levels1;
	pixels[i + 1] = ((glevel * level) >> 8) * 255 / levels1;
	pixels[i + 2] = ((blevel * level) >> 8) * 255 / levels1;
	}
	},

• Not sure how to handle validating the extra filter parameter; here it throws a vanilla error when outside 2-255. But others like BLUR and THRESHOLD just do their own little validation and fail silently if the parameter is invalid. Maybe I should add some extra friendly errors for each case? ('🌸 POSTERIZE only accepts a parameter from 2 to 255; received x', or 'THRESHOLD only accepts a parameter between 0.0 and 1.0; received x')

[aferriss]

Interesting, I'll take a look and try to suss out whats going on here.

[aferriss]

I think your round was doing something weird. It should be a floor instead. See: https://editor.p5js.org/aferriss/sketches/mjQKudkIP

[wong-justin]

Not sure if the if statement should be replaced by some sort of step logic. I know it's good to avoid branching if statements in shaders for speed. But a simple if statement seems easier to read and maintain

[wong-justin]

I think I documented all the important features and behaviors between the filter() and createFilterShader() pages. I tried to keep filter() a little less technical or webgl-centric, at least until the bottom, since that's more of an entry point page for beginners. The rest of the gotchas and explanations and further reading are in createFilterShader() for advanced users. Screenshots, with examples omitted for brevity:

filter()

createFilterShader()

Open to suggestions as always, especially @aceslowman - I remember you said it would be nice to have an earlier disclaimer about filter shaders not working on 3D geometries, for people coming from shader land? Maybe swapping the second paragraph of createFilterShader() with the first?

[aferriss]

should we add texelSize to the documentation for createFilterShader?

[aferriss]

lets just simplify this whole block to float spread = max(0.0, filterParameter); and then set a default value for the filterParameter in the js that is overridden if explicitly set.

[aferriss]

How does the output of this blur compare to the cpu one?

[wong-justin]

Here's a sketch for comparison, although bricks.jpg isn't the best example to compare blur:
https://editor.p5js.org/jwong/sketches/r9fxPerPU

[aferriss]

this is fine, haha. Probably just different implementers

[aceslowman]

@jwong I think that having it match what is expected for the non-webgl filters feels best, glad to see you leave a comment about it here, I think it's helpful context.

[aferriss]

I think that this shader can be simplified and maybe doesn't need the luma calculation. Let me know if I'm wrong but take a look here

https://www.shadertoy.com/view/dtScWD

[aferriss]

Same for erode, but it will be a min instead of max

[wong-justin]

max() seems to choose that greatest of each component, compared to the luma() way of choosing the greatest overall vector. Example:

a = (1, 1, 0)
b = (0, 0, 1)
max(a,b) = (1, 1, 1)  # instead of (1, 1, 0)

Not sure if that's more desired or not. The results look similar though

[aferriss]

I think either way is fine, if the luma calculation is more accurate to the CPU mode then disregard my comment!

[aferriss]

I think we should change this shader to use the model view projection matrix instead. See my issue here #6367 where this shader is causing trouble because of the depth testing.

[wong-justin]

I seem to have fixed blur. Still testing and double checking things. It's promising that the parameter works as intended, and I fixed a flipping problem midway, and it works every frame, and there's no performance drops. It's much smoother than the single pass shader was. Some screenshots:

# no filter

filter(BLUR, 1)

filter(BLUR) # default 3

and for reference, filter(BLUR, 3, useWebGL=false)

https://editor.p5js.org/jwong/sketches/X8-AVn4hn

[aferriss]

I feel like the 2 here might be causing some slight pixellation, maybe we should change back to 1 for both horizontal and vertical directions

[aferriss]

why do we need to do these initial passes? I think we could get away with just drawing the source canvas into the pg layer without a shader, and then running the loop.

[aferriss]

What if someone is trying to use the accelerated filters from p2d mode. Won't this cause an error?

[Qianqianye]

Looks great! Thanks @wong-justin for working on it and @aferriss @aceslowman for reviewing. 🎉

[SableRaf]

Hey @wong-justin, @aferriss, and @aceslowman,

First off, major props on integrating shader-based filters in p5.js! The performance gains are amazing and that's gonna dramatically increase the accessibility of the filter() function.

While the shader-based approach undeniably improves performance, I noticed there are still some artifacts, particularly in the filter(BLUR) filter(ERODE) and filter(DILATE) outputs. When we set shaders as defaults, this might lead to some unexpected outputs in older projects, especially those that heavily rely on blur, erode, or dilate. Of course they may just set useWebGL to false but this could create some confusion for people who are not aware of the change.

I've put together a quick visual comparison using a small demo sketch you can check out, or you can just look at the image below that was generated from it.

I'm wondering whether there's potential to refine the shader implementation slightly on those three in particular to minimize these visual discrepancies.

Just thinking out loud here, and definitely not a show stopper but I'm curious to hear what our shader experts here have to say 😃 Again, amazing work on the PR!

[davepagurek]

I think in the updated docs for BLUR, it mentions that it uses a box blur for the shader version as opposed to the Gaussian blur in CPU mode. We could plausibly still use a Gaussian kernel in the shader to get things to look more similar and maybe compare performance between the two to see if the tradeoff is worth it to match output? We could open an issue for that to at least investigate.

There was also some discussion in comments about whether or not to use luma-based dilate/erode, which I believe the CPU implementation uses, and the shaders now do not use (for performance?), which probably explains some of the difference for those two filters.

Also I wonder if pixel density handling affects it? I think the temp graphics layer doesn't specify pixel density so it'd use display density instead of matching the main canvas density, and texelSize may not factor pixel density in (although I don't know how the CPU version handles it, I'd need to look into that.) (Also maybe we should handle canvas resizes done after the temp layer has been created?)

Anyway we could maybe open follow-up issues to track and then iron out the differences?