diff --git a/src/libslic3r/TextureDisplacement.cpp b/src/libslic3r/TextureDisplacement.cpp index 93626fdbaf..264809d79c 100644 --- a/src/libslic3r/TextureDisplacement.cpp +++ b/src/libslic3r/TextureDisplacement.cpp @@ -91,12 +91,16 @@ bool DecodedHeightTexture::texel_tap(const Vec2f &uv, bool tile_enabled, Texture const float fy = v * float(height); const int x0 = std::clamp(int(std::floor(fx)), 0, width - 1); const int y0 = std::clamp(int(std::floor(fy)), 0, height - 1); - // Neighbour for bilinear filtering: wrap for tiling methods, clamp at the edge otherwise (a - // repeating neighbour would incorrectly blend against the opposite edge of the image). + // Neighbour for bilinear filtering. Repeat wraps, because its next tile really does start at the + // opposite edge. The other two clamp: an untiled placement has nothing past its border, and a + // mirrored tile folds back on itself there, so the texel past the edge is the edge again. Wrapping + // a mirrored seam blends the two opposite edges of the image together, which is both a visible + // line and a disagreement with the preview - the GPU path binds GL_MIRRORED_REPEAT, which folds. + const bool wrap_neighbour = tile_enabled && tile_method != TextureTileMethod::MirroredRepeat; tap.x0 = x0; tap.y0 = y0; - tap.x1 = tile_enabled ? (x0 + 1) % width : std::min(x0 + 1, width - 1); - tap.y1 = tile_enabled ? (y0 + 1) % height : std::min(y0 + 1, height - 1); + tap.x1 = wrap_neighbour ? (x0 + 1) % width : std::min(x0 + 1, width - 1); + tap.y1 = wrap_neighbour ? (y0 + 1) % height : std::min(y0 + 1, height - 1); tap.tx = fx - std::floor(fx); tap.ty = fy - std::floor(fy); return true; diff --git a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp index 54f50c2a78..4e45a23824 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp @@ -286,6 +286,14 @@ constexpr float PREFER_PURE_DE = 2.f; // the quantizer's lookup cube. constexpr size_t MIX_TARGET_SAMPLES = size_t(1) << 20; constexpr size_t MIX_TARGET_BINS = 256; +// The longest interleave cycle a mix may use, as the denominator of its ratio. The slicer alternates +// a mix layer by layer, so the cycle is how many layers one repeat spans; past this it stops reading +// as a colour and starts reading as bands of its two filaments. It also sets how many mixes exist at +// all - the coprime ratios per pair of filaments - so it is what decides how finely a gradient can be +// stepped. Ten gives 33 ratios per pair against the 11 of six: measured on four filaments, a +// green-to-red gradient goes from 11 usable mixes to 16, and the shipped Colour Bricks from 6 to 10. +constexpr int MIX_MAX_DENOMINATOR = 10; + // rank_mixes() stops once the best remaining mix would improve the match by less than this FRACTION // of what the best mix bought: a mix that only touches a few stray pixels is not worth a filament // slot. Relative rather than a fixed amount of CIEDE2000, because the gains decay smoothly and a @@ -5355,7 +5363,7 @@ std::vector GLGizmoTextureDisplacement std::vector candidates; for (int i = 0; i < n; ++i) for (int j = i + 1; j < n; ++j) - for (int den = 2; den <= 6; ++den) + for (int den = 2; den <= MIX_MAX_DENOMINATOR; ++den) for (int num = 1; num < den; ++num) { if (std::gcd(num, den) != 1) continue; // 2/4 is 1/2, already there diff --git a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp index bd6ec6f345..65694ddf2c 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp @@ -103,7 +103,7 @@ public: // actually pick it. Stops early once another mix would make no noticeable difference, so a texture // the filaments already cover gets few mixes or none. // - // Ratios are the short-cycle ones (k/d for d up to 6): the slicer interleaves a mix layer by layer, + // Ratios are k/d for d up to MIX_MAX_DENOMINATOR: the slicer interleaves a mix layer by layer, // and a long cycle prints as visible bands rather than as a colour. A mix's `rgb` is the colour its // mixed filament slot will show (blend_color_multi(), as the sidebar computes it), so the match, the // preview and the slot all agree on what the mix looks like. diff --git a/tests/libslic3r/test_texture_displacement.cpp b/tests/libslic3r/test_texture_displacement.cpp index cfd134b461..e78593fecb 100644 --- a/tests/libslic3r/test_texture_displacement.cpp +++ b/tests/libslic3r/test_texture_displacement.cpp @@ -2653,6 +2653,33 @@ TEST_CASE("TextureDisplacement: a texture of flat colours is told apart from a c CHECK(d.flat_share < 0.85f); } +TEST_CASE("A mirrored tile folds at its seam instead of wrapping to the far edge", "[TextureDisplacement]") +{ + // Mirrored repeat exists so a tile edge lines up with itself, which means the texel past the edge + // is the edge again. Wrapping the bilinear neighbour instead blends the image's two opposite edges + // together: on a ramp from black to white it turned the white edge into near-black at every seam, + // a visible line and a disagreement with the preview, where the GPU binds GL_MIRRORED_REPEAT. + TextureDisplacementLayer layer; + layer.image_data = make_rgb_png(8, 8, [](size_t x, size_t) { + const uint8_t v = uint8_t(x * 36); // 0 at the left edge, 252 at the right + return std::array{ v, v, v }; + }); + const DecodedHeightTexture tex = decode_height_texture(layer); + REQUIRE(tex.width == 8); + const float bright = 252.f / 255.f; + + // Either side of a seam the fold puts the bright edge, not a blend with the dark one. + for (const float u : { 0.999f, 1.001f, 2.999f, 3.001f }) { + INFO("u = " << u); + CHECK_THAT(tex.sample(Vec2f(u, 0.5f), true, TextureTileMethod::MirroredRepeat), + WithinAbs(bright, 0.02f)); + } + + // Plain repeat is the opposite case and must keep wrapping: its next tile really does start at the + // far edge, so just inside the seam the neighbour is the dark one. + CHECK(tex.sample(Vec2f(0.999f, 0.5f), true, TextureTileMethod::Repeat) < 0.1f); +} + TEST_CASE("A two-colour ramp is not mistaken for an image of flat colours", "[TextureDisplacement]") { // The histogram alone cannot tell these two apart: a ramp between two colours visits as few