[BUG] Dimples and bricks textures not normalized in [0,1]

[adgaudio]

Describe the bug
Thank you for this wonderful library. In the process of making x-mas presents (basically cylinders with different textures) that are then 3-d printed, I found an inconsistency in the design and application of textures.

All textures that I have 3-d printed so far preserve the inner radius of the cylinder that they were applied to. The "dimples" texture, however, preserves the outer radius instead of the inner radius.

• These textures preserve the inner radius: trunc_pyramids, pyramids, diamonds (concave or stripped), cubes, tri_grid)
• This texture preserves the outer radius: dimples

The below code snippet may not be particularly helpful in the analysis of this issue, but I list it here for completeness.
The following code generates cylinders that I 3-d printed (with 0 infill, 5 perimeter walls). Only dimples made a cylinder whose inner radius was too small.

path = circle(r=r, $fa=fa, $fs=fs);
tex = "dimples";
// tex = "trunc_pyramids"  // or (any?) other texture here.
linear_sweep(
    path,
    texture=tex,  tex_size=[20,20], tex_depth=2,
    h=h, style="concave");
}

Upon further inspection, I believe the issue is that the texture(...) matrix contains negative values for dimples and non-negative values for (almost) all other textures available in BOSL2 library. I had incorrectly assumed that all textures would return a non-negative matrix. The important part are the last two numbers in the ECHO lines output by OpenSCAD's console.

min2d = function(matrix) (min([ for (row = matrix) for (val = row) val]));
max2d = function(matrix) (max([ for (row = matrix) for (val = row) val]));

texs = [
"ribs", "trunc_ribs", "trunc_ribs_vnf", "wave_ribs", "diamonds",
"diamonds_vnf", "pyramids", "pyramids_vnf", "trunc_pyramids",
"trunc_pyramids_vnf", "hills", "bricks", "bricks_vnf", "checkers", "cones",
"cubes", "trunc_diamonds", "dimples", "dots", "tri_grid", "hex_grid", "rough"
];

for (tex = texs) {
    // print the texture's min and max value
    if (is_vector(min2d(texture(tex)))) {
        echo(tex, "texture_is_3d", min2d(texture(tex)[0]), max2d(texture(tex)[0]));
    } else {
        echo(tex, "texture_is_2d", min2d(texture(tex)), max2d(texture(tex)));
    }
}
// ECHO: "ribs", "texture_is_2d", 0, 1
// ECHO: "trunc_ribs", "texture_is_2d", 0, 1
// ECHO: 0.25, 0.5, 0.75, 1.25  // (...noise caused by trunc_ribs_vnf)
// ECHO: 0.25, 0.5, 0.75, 1.25  // (...noise caused by trunc_ribs_vnf)
// ECHO: 0.25, 0.5, 0.75, 1.25  // (...noise caused by trunc_ribs_vnf)
// ECHO: "trunc_ribs_vnf", "texture_is_3d", 0, 1
// ECHO: "wave_ribs", "texture_is_2d", 0, 1
// ECHO: "diamonds", "texture_is_2d", 0, 1
// ECHO: "diamonds_vnf", "texture_is_3d", 0, 1
// ECHO: "pyramids", "texture_is_2d", 0, 1
// ECHO: "pyramids_vnf", "texture_is_3d", 0, 1
// ECHO: "trunc_pyramids", "texture_is_2d", 0, 1
// ECHO: "trunc_pyramids_vnf", "texture_is_3d", 0, 1
// ECHO: "hills", "texture_is_2d", 0, 1
// ECHO: "bricks", "texture_is_2d", -0.0241379, 0.524828
// ECHO: "bricks_vnf", "texture_is_3d", 0, 1
// ECHO: "checkers", "texture_is_3d", 0, 1
// ECHO: "cones", "texture_is_3d", 0, 1
// ECHO: "cubes", "texture_is_3d", 0, 1
// ECHO: "trunc_diamonds", "texture_is_3d", 0, 1
// ECHO: "dimples", "texture_is_3d", -1, 1
// ECHO: "dots", "texture_is_3d", 0, 1
// ECHO: "tri_grid", "texture_is_3d", 0, 1
// ECHO: "hex_grid", "texture_is_3d", 0, 1
// ECHO: "rough", "texture_is_2d", 0.000396749, 0.199996

Expected behavior

I would expect all textures to return values normalized in the range [0,1]. Dimples is clearly an outlier, and bricks is also not correctly normalized. I did not find documentation of any such guarantee of consistency in the docs, so I am not entirely sure if this issue is a bug or a feature request. I think that this has not been discussed in a previous issue.

In any case, this is what I would think should be done:

1. Textures should be normalized in [0,1] by default. This means fix dimples and bricks textures. Also consider fixing the rough texture.
2. Perhaps a flag made available (in the texture(...) function?) to set the expectation of normalization in [0,1] (e.g. preserving inner radius of the cylinder in the first example code snippet) or [-1,0] (preserving outer radius) or [-.5, .5] (centered)). The normalization in [-1,0], [0,1], or [-.5, .5] is probably necessary for the flag tex_depth=N to have its intended meaning.
3. The default normalization (and flag option, if implemented) should be documented somewhere in the wiki.
4. We may also want a test to check for inconsistent behavior across textures. The code example above is a start for this test, but I'm not aware of a "global" list of all textures, so I'm not sure how to ensure the test would stay up-to-date with the latest list of textures.
5. (discussion point) Not sure, but maybe we don't need the test (4) or the fixes (1) if the texture(...) function automatically performs 0-1 normalization to all textures.

I unfortunately don't have the bosl2 internals knowledge or bandwidth to try to do undertake this bug/feature, but I thought it might benefit other BOSL2 users to raise this issue.

Thanks again for this awesome library.

Additional context
Add any other context about the problem here.

• OpenSCAD Version: Git commit c442c51 from Nov. 29.
• Other libraries used: None

Updates: TODO tasks after community discussion
(last updated after reviewing comment: #1531 (comment) )

• (TBD what should be done) documentation really hits hard on the [0,1] range idea of textures, so it seems like that needs to be clarified. ... A note can be added to the dimples entry.
• "bricks" needs to be fixed. It has values in the range of [-0.0241379, 0.524828] rather than [0,1]
• remove extra echo statements in trunc_ribs_vnf

[amatulic]

My understanding is that wherever z=0 for a texture, then that is what falls on the radius. I have made textures with both positive and negative z values based on where I want the texture features relative to the surface being textured.

To me, it makes far more sense for dimples to have negative values, because I want my outer surface to be where I expect, with dimples going inward, just as if I had a wooden cylinder and physically made dimples in the surface.

I don't see this as a problem. A flag to control the position of the texture relative to the surface would be useful though. And some notes in the documentation.

[adrianVmariano]

Yeah, I think it's right that dimples project inward from the reference surface. The documentation really hits hard on the [0,1] range idea, so it seems like that needs to be clarified, and certainly a note can be added to the dimples entry. But maybe "bricks" needs to be fixed. And the extra echo statements need to be removed.

A parameter to control texture position relative to the surface already exists: tex_inset. If it's set to -1 it will produce the result that @adgaudio wanted, where the bottom of the dimples are tangent to the original cylinder.

[adgaudio]

Thanks @adrianVmariano - I didn't realize the tex_inset=-1 solves this issue for dimple!

Considering the surface of the example cylinder, dimples is the only texture that is "subtractive" and all others are "additive". Do we really want just the one texture to have behavior opposite to all others?

I like the physical idea of making dimples into wood quite a lot. The same conceptual argument could also be applied to all of the textures. In my opinion, all textures should all share the same mentality - either all of them are conceptually subtractive (like "carving" into wood, e.g. dimples) or all of them are conceptually additive. Having a mixture of additive and subtractive textures is somewhat confusing and requires remembering not just the texture name, but also whether it is additive or subtractive.

I'll try to edit the description with an updated list of tasks in an "update" section at bottom to incorporate @adrianVmariano suggestions.

[amatulic]

I don't know when textures were added, but if it's been a few months or more, it would be a bad idea to change their behavior because the change would break existing models. I do agree it makes sense for all of them to be subtractive, however, while consistency would be nice, at this point it may be better just to clarify the documentation.

[adrianVmariano]

There is always the question of if bugs need to be fixed or left to persist in the name of backward compatibility. The goal---though things have been dragging out a heck of a lot longer than hoped---was to release a fixed 2.0 release that would be the stable release and would only get real honest bug fixes, like things that crash or produce obviously useless results, so that models could reliably run there. I have two open issues that we were hoping to fix prior to that release marked with the red "release blocker" flag. We have generally prioritized getting things right somewhat more than backward compatibility, which I know can be annoying. I also have a list of breaking interface changes I was thinking I would make right when the 2.0 release comes out so they could then be documented on a release notes page. Maybe fixing the bricks texture could be one. I'm not sure why it of all the textures is half-height. Maybe to "make room" for the rough texture?

Textures are substantially more than a few months old, so if we changed the bricks texture it could definitely cause trouble.

Note that Revar wrote all the texture code and the textures, so I might have done it differently. Regarding the dimple texture being exceptional, the simple thing is, dimples go inward. The word "dimple" tells you that the texture goes in. I would find it very surprising to ask for a dimple texture and discover that my shape got bigger---that's not how you add dimples to something. I perused the list of textures to see to what extent they include "direction". It seems like cones, dots, hills, pyramids and trunc pyramids are obviously textures that stick out from their names. The only one that obviously sticks in is dimples. Diamonds visually looks like things sticking out so that's a visual expectation and the same with the wide margin hex_grid. Checkers looks like it should be centered---half in half out. And several seem naturally like they should project in, like tri_grid and hex_grid. As noted above, changing this might break stuff and isn't particularly compelling. But maybe a note in the docs emphasizing the directions that textures point wouldn't hurt, as well as a discussion about making inward textures.

Note that the only really compelling reason for the [0,1] bound is so that the "depth" parameter corresponds to something meaningful rather than being arbitrary. I'll have to ponder how to modify the docs. I suppose one reason to favor outward textures generally speaking is that the texture tiles are probably not going to self-intersect. That might actually be a driving reason for doing it that way a lot of the time. Like if you tried to do inset bricks you're probably going to run into a problem at corners with the tiles on different faces crossing each other where the gutter between the bricks is located.