ADD/sample/marchingSquares.glsl: first draft of marching squares algorithm #90
Conversation
There was a problem hiding this comment.
Hi Guido! This is a fantastic addition.
Not 100% if sample/ is the right place.
I like that you can sample noise fields, would make sense in this folder if it was set up to sample a texture and in the absence of it a noise filed.
What do you think?
Other wise maybe it's place could be on sdf/? geometry/? draw/? Mmm..... I'm really not sure.
| } | ||
| #define SAMPLE_MARCHING_SQUARES_FNC(UV) defaultSampleFunction(UV) | ||
| #endif | ||
|
|
There was a problem hiding this comment.
We use the following pattern to prevent name collisions
#ifndef FNC_SAMPLEMARCHINGSQUARES
#define FNC_SAMPLEMARCHINGSQUARES
...
#endif
There was a problem hiding this comment.
Sorry this comment I made was a bit confusing.
#ifndef FNC_SAMPLEMARCHINGSQUARES
#define FNC_SAMPLEMARCHINGSQUARES
...
#endif
is to wrap the entire function. For example here: https://github.com/patriciogonzalezvivo/lygia/blob/main/sample/shadowPCF.glsl#L18-L19
While for "templeting" just the function use to do sampling have this other pattern:
#ifdef SAMPLEMARCHINGSQUARES_SAMPLE_FNC
#define SAMPLEMARCHINGSQUARES_SAMPLE_FNC(TEX, UV) defaultSampleFunction(TEX, UV)
#endif
For example:
There was a problem hiding this comment.
Ah yes, got it. Really like in C++ world with the header files, thanks for clarifying
sample/marchingSquares.glsl
Outdated
|
|
||
| #define centerOf(p1, p2, v1, v2) mix(p1, p2, 0.5) | ||
|
|
||
| float sampleOutline(in vec2 p, in vec2 cellUv, in vec2 a, in vec2 b, in bool straight) { |
There was a problem hiding this comment.
this can potentially produce name collisions.
Let's put the name of the main function first like
float sampleMarchingSquares_outline(...) {
}
sample/marchingSquares.glsl
Outdated
| return 1.0 - step(lineStrength, length(line)); | ||
| } | ||
|
|
||
| float sampleTile(in vec2 p, in vec2 cellUv, in vec2 a, in vec2 b, in int tile) { |
+ Author → original_author + Description starting on the next line, added '|' + Renaming of macro + Introduce TEX parameter to macro
|
🙏🏽 Thanks for the detaild review just had a quick catch on testing it using a sampled texture, and it seems like there are some artifacts (eventually due to mismatch of texture and screen resolution? 🤔 ). I wasn't sure how to handle the different color channels (for now the red channel is used by default). Might be cool to provide additional alternative functions or come up with a more modular way. Anyway, tried to integrate all of the other requested changes . Also is there a potential field like |
|
Lovely! excited for it! there is no reference. I usually added it to a multi-line on https://github.com/patriciogonzalezvivo/lygia/blob/main/color/mixSpectral.glsl#L8-L12 |
sample/marchingSquares.glsl
Outdated
| } | ||
| } | ||
|
|
||
| vec2 sampleMarchinSquares(in vec2 uv, in sampler2D tex, in float cellSize, in float threshold, in vec2 resolution) { |
There was a problem hiding this comment.
We usually sort the arguments in this orders: sample2D, vec4, vec3, vec2, float, int
There was a problem hiding this comment.
Ah nice, I see. I like this idea of ordering
|
Hi Guido! Thanks for the changes! I add a couple more notes. Hope you don't mind. |
Hey @patriciogonzalezvivo I've added a first version of a marching squares algorithm. Would love to get some feedback on naming, code improvements, potential performance improvements and everything else that comes to your mind ✌🏽
I've also put up an example on how to use it over at my examples.lygia-threejs repo.