feat: add MultiPassKawaseBlur pass #635
Conversation
| // p1.addBinding(kawaseBlurPass.fullscreenMaterial, "scale", { min: 0, max: 2, step: 0.01 }); | ||
| // p1.addBinding(kawaseBlurPass.resolution, "scale", { label: "resolution", min: 0.5, max: 1, step: 0.05 }); | ||
| const p1 = tab.pages[1]; | ||
| // TODO: is it ok that the following two scale parameters are independant from each other? |
There was a problem hiding this comment.
Not sure about this: "is it ok that the following two scale parameters are independent from each other?"
There was a problem hiding this comment.
It's probably not safe to scale the sampling patterns like this.
| let { width, height } = this.resolution; | ||
|
|
||
|
|
||
| // TODO: unsure about what to assign as the fullscreen material when the pass requires multiple |
There was a problem hiding this comment.
also not sure about the following: "unsure about what to assign as the fullscreen material when the pass requires multiple"
There was a problem hiding this comment.
"when the pass requires multiple..." materials? It's fine like this. The fullscreenMaterial can be set to any material defined in the class declaration. Choosing a fullscreen material depends on the render task(s) that should be performed right after. Most passes only use one, but switching is fine too.
|
I'm a bit short on time right now - I'll take a look when I'm free. |
|
This Dual Kawase implementation looks very similar to the existing It seems like this implementation introduces aliasing and shimmering when the The existing I'm not completely sure about that though - I'll need to take a closer look at the reference materials at some point. |
| * @category Effects | ||
| */ | ||
|
|
||
| export class MultiPassKawaseBlurEffect extends Effect { |
There was a problem hiding this comment.
There's no need to create an effect for a blur pass - the TextureEffect can be used to draw the blurred image.
| * @category Passes | ||
| */ | ||
|
|
||
| export interface MultiPassKawaseBlurPassOptions { |
There was a problem hiding this comment.
The pass should be named DualKawaseBlurPass.
| // p1.addBinding(kawaseBlurPass.fullscreenMaterial, "scale", { min: 0, max: 2, step: 0.01 }); | ||
| // p1.addBinding(kawaseBlurPass.resolution, "scale", { label: "resolution", min: 0.5, max: 1, step: 0.05 }); | ||
| const p1 = tab.pages[1]; | ||
| // TODO: is it ok that the following two scale parameters are independant from each other? |
There was a problem hiding this comment.
It's probably not safe to scale the sampling patterns like this.
| let { width, height } = this.resolution; | ||
|
|
||
|
|
||
| // TODO: unsure about what to assign as the fullscreen material when the pass requires multiple |
There was a problem hiding this comment.
"when the pass requires multiple..." materials? It's fine like this. The fullscreenMaterial can be set to any material defined in the class declaration. Choosing a fullscreen material depends on the render task(s) that should be performed right after. Most passes only use one, but switching is fine too.
Related issue: #580
Description
Adds a MultiPassKawaseBlur pass,
Adds the aforementioned pass to the blur demo, comparing it against the GaussianBlur pass,
Adds associated tests.
Notes
@vanruesc I've allowed the Kawase Blur pass to be applied at a custom number of levels, as I thought this may be more flexible than the default dual pass approach. There might be tradeoffs to this I'm not aware of however, and so have made these changes in a commit I can easily revert if it's preferred to do so.
I've also left the previously existing KawaseBlurMaterial, as the Tilt Shift effect relies on it, and I wanted to get this in first.
I've left some TODO comments in the two areas I'm a little unsure about.