From 8f1c8f05807e810ce489744ebb2d46655948b5f4 Mon Sep 17 00:00:00 2001 From: ExPikaPaka Date: Mon, 12 Oct 2026 08:18:58 +0200 Subject: [PATCH] feat: live displacement in the image-adjustment panel, reached from a per-layer icon Four changes to the image-adjustment panel, all from using it: Entry point. The panel is now opened by an icon in the layer's header row, between the image name and the order arrows, rather than by a button buried under "More settings". It acts on the image the name refers to, so it belongs beside the name. Disabled for a layer with no image. Only "Save as copy". Overwrite is gone, and with it the one path that edited a layer's stored image in place. Writing the result to the texture library and pointing the layer at it is the single way an adjustment becomes permanent, which also means an adjusted texture is always a file the user can find and reuse. encode_texture_image_data() had no other caller and is removed. A window, not a popup. A popup dimmed the canvas and closed on any click outside itself, which between them made it impossible to watch the model while adjusting it. An ordinary floating window leaves the 3D view live, clickable and orbitable, and stays put until Cancel, Save as copy or its close button. The displacement follows the sliders. TextureDisplacementLayer gains a transient preview_adjust that decode_height_texture() applies on top of the decoded image, before smoothing. Everything downstream of the decode then sees the adjusted image without being told the panel exists: the relief, the colour, the thumbnails, the UV backdrop and the bake. It is not serialized by cereal or the .3mf, because it is the state of an open editor rather than a property of the layer; the panel reverts it on every exit that is not a commit, and a commit clears it before pointing the layer at the file that already carries the edit, so the adjustment is never applied twice. Two caches had to learn about it or they would have gone on serving the pixels from before the slider moved, since neither the image_data pointer nor the smoothing value changes while a slider is dragged: the new adjusted-copy cache keys on (image, adjustments), the smoothed cache now keys on both, and the GPU thumbnail, height and colour textures and the UV editor backdrop carry the adjustment in their keys alongside smoothing. Two tests cover exactly that: the same image_data with two different adjustments must decode to two different images, with and without smoothing on. The recompute is lazy. A slider move marks the preview dirty and the existing "rebuild once the mouse is released" rule in the panel consumes it, so a drag recomputes the displacement once, on release, instead of once per frame. The panel's own image still updates continuously, from a background pass over a 384 px copy, so the photo tracks the slider while the model catches up after. --- src/libslic3r/TextureDisplacement.cpp | 54 +++- src/libslic3r/TextureDisplacement.hpp | 76 +++--- .../GUI/Gizmos/GLGizmoTextureDisplacement.cpp | 249 ++++++++++-------- .../GUI/Gizmos/GLGizmoTextureDisplacement.hpp | 33 ++- src/slic3r/GUI/TextureLibrary.cpp | 15 -- src/slic3r/GUI/TextureLibrary.hpp | 13 - tests/libslic3r/test_texture_displacement.cpp | 51 ++++ 7 files changed, 304 insertions(+), 187 deletions(-) diff --git a/src/libslic3r/TextureDisplacement.cpp b/src/libslic3r/TextureDisplacement.cpp index ca02f518f1..38bba5ef28 100644 --- a/src/libslic3r/TextureDisplacement.cpp +++ b/src/libslic3r/TextureDisplacement.cpp @@ -157,12 +157,33 @@ struct SmoothedTextureCache { std::weak_ptr> source; float smoothing = 0.f; + // Part of the key, not of the result: smoothing is applied *after* the panel's adjustments, so a + // blur computed over one set of adjustments is not the blur another set would give. + TextureAdjustments adjust; DecodedHeightTexture texture; }; std::mutex mutex; std::unordered_map entries; }; SmoothedTextureCache g_smoothed_texture_cache; + +// The adjusted copy, cached between the raw decode and the smoothed one. Needed for the same reason the +// smoothed cache is: while a slider in the image-adjustment panel moves, the thumbnail, the shaded +// preview's height and colour textures, the UV editor backdrop and the displacement preview job all ask +// for the same (image, adjustments) pair within one frame, and each of them running a full-size resample +// of its own is what would make the panel unusable on a large texture. +struct AdjustedTextureCache +{ + struct Entry + { + std::weak_ptr> source; + TextureAdjustments adjust; + DecodedHeightTexture texture; + }; + std::mutex mutex; + std::unordered_map entries; +}; +AdjustedTextureCache g_adjusted_texture_cache; } // namespace namespace { @@ -338,6 +359,34 @@ DecodedHeightTexture decode_height_texture(const TextureDisplacementLayer &layer entries[key] = { std::weak_ptr>(layer.image_data), result }; } + // The image-adjustment panel's live edits, on top of the decode and before the smoothing. Before, + // because `preview_adjust` is an edit to the image while `smoothing` is a live low-pass over whatever + // the image is - the same order the panel presents them in, and the same order a commit bakes in. + // + // Nothing applies here once the panel has closed: it resets preview_adjust to identity, and identity + // is a no-op that costs one comparison. + if (!layer.preview_adjust.identity()) { + bool have_adjusted = false; + { + std::lock_guard lock(g_adjusted_texture_cache.mutex); + auto it = g_adjusted_texture_cache.entries.find(key); + if (it != g_adjusted_texture_cache.entries.end() && it->second.adjust == layer.preview_adjust && + it->second.source.lock() == layer.image_data) { + result = it->second.texture; + have_adjusted = true; + } + } + if (!have_adjusted) { + result = apply_texture_adjustments(result, layer.preview_adjust); + std::lock_guard lock(g_adjusted_texture_cache.mutex); + auto &entries = g_adjusted_texture_cache.entries; + for (auto it = entries.begin(); it != entries.end();) + it = it->second.source.expired() ? entries.erase(it) : std::next(it); + entries[key] = { std::weak_ptr>(layer.image_data), + layer.preview_adjust, result }; + } + } + // Smoothing radius scales with the texture so the same slider value blurs the same *fraction* of // the image whatever resolution it came in at, and stays continuous in the slider - see // smooth_height_pixels(). The cap is a cost limit, not part of the mapping: the blur is @@ -348,7 +397,7 @@ DecodedHeightTexture decode_height_texture(const TextureDisplacementLayer &layer std::lock_guard lock(g_smoothed_texture_cache.mutex); auto it = g_smoothed_texture_cache.entries.find(key); if (it != g_smoothed_texture_cache.entries.end() && it->second.smoothing == layer.smoothing && - it->second.source.lock() == layer.image_data) + it->second.adjust == layer.preview_adjust && it->second.source.lock() == layer.image_data) return it->second.texture; } const float span = 0.05f * float(std::min(result.width, result.height)); @@ -372,7 +421,8 @@ DecodedHeightTexture decode_height_texture(const TextureDisplacementLayer &layer auto &entries = g_smoothed_texture_cache.entries; for (auto it = entries.begin(); it != entries.end();) it = it->second.source.expired() ? entries.erase(it) : std::next(it); - entries[key] = { std::weak_ptr>(layer.image_data), layer.smoothing, result }; + entries[key] = { std::weak_ptr>(layer.image_data), layer.smoothing, + layer.preview_adjust, result }; } return result; } diff --git a/src/libslic3r/TextureDisplacement.hpp b/src/libslic3r/TextureDisplacement.hpp index 86b7b07cf2..f88aa7665b 100644 --- a/src/libslic3r/TextureDisplacement.hpp +++ b/src/libslic3r/TextureDisplacement.hpp @@ -154,6 +154,38 @@ struct TextureIsland // chart to its true mm area, so a freshly unwrapped patch has uniform texel density to begin with. void average_island_scales(std::vector &islands); +// Photo-style adjustments to a layer's source image, as the gizmo's image-adjustment panel sets them. +// +// These are *not* layer parameters: nothing here is stored on TextureDisplacementLayer and nothing here is +// re-applied at bake time. The panel applies them once, to the image, and the result becomes the layer's +// image - so the adjusted pixels feed the colour and the displacement together, and a brightness change +// really does change the relief. That is the intended behaviour: one image, one source of truth, rather +// than a second set of knobs the bake would have to replay in the right order. +// +// Contrast with TextureDisplacementLayer::smoothing, which *is* a layer parameter applied on every decode: +// `blur` here is a destructive edit of the image, `smoothing` is a live low-pass over whatever the image is. +struct TextureAdjustments +{ + float brightness = 0.f; // [-1, 1]; 0 unchanged, -1 black, +1 white + float contrast = 0.f; // [-1, 1]; 0 unchanged, -1 flat mid-grey, +1 hard threshold + float sharpness = 0.f; // [0, 1]; 0 unchanged. Unsharp-mask amount + float blur = 0.f; // [0, 1]; 0 unchanged. Fraction of SOURCE_BLUR_SPAN, as `smoothing` is + float hue_deg = 0.f; // [-180, 180]; 0 unchanged. No effect on a grayscale image - grey has no hue + + bool identity() const + { + return brightness == 0.f && contrast == 0.f && sharpness == 0.f && blur == 0.f && hue_deg == 0.f; + } + // Exact equality, which is what is wanted: these come straight off sliders, and the question asked of + // them is "is this still the value the preview on screen was computed from", not "are these close". + bool operator==(const TextureAdjustments &o) const + { + return brightness == o.brightness && contrast == o.contrast && sharpness == o.sharpness && + blur == o.blur && hue_deg == o.hue_deg; + } + bool operator!=(const TextureAdjustments &o) const { return !(*this == o); } +}; + // One texture asset plus its projection/displacement parameters. Several layers may be painted // onto overlapping areas of the same volume: their displacements are combined per vertex, in slot // order, each layer folding into the total via its own TextureBlendMode (see @@ -293,6 +325,18 @@ struct TextureDisplacementLayer // for the boundary to be drawn on. bool color_enabled = false; + // Live, unsaved image adjustments from the gizmo's image-adjustment panel, applied by + // decode_height_texture() on top of the decoded image - so while the panel is open the relief, the + // colour, the thumbnails and the bake all see the adjusted image, and moving a slider really does + // change the displacement. See TextureAdjustments. + // + // Deliberately *not* serialized, by either save()/load() here or the .3mf: it is the state of an open + // editor, not a property of the layer. The panel resets it when it closes, and "Save as copy" resolves + // it by writing a new image and pointing the layer at that - which is the only way an adjustment + // becomes permanent. A project saved mid-adjustment therefore stores the unadjusted image, which is + // the honest answer: nothing was committed. + TextureAdjustments preview_adjust; + bool empty() const { return !image_data || image_data->empty(); } template void save(Archive &ar) const @@ -489,38 +533,6 @@ DecodedHeightTexture decode_height_texture(const TextureDisplacementLayer &layer // texture - cheap enough to ask every frame. Smoothing does not change it, so the raw decode is what is read. bool height_texture_has_color(const TextureDisplacementLayer &layer); -// Photo-style adjustments to a layer's source image, as the gizmo's image-adjustment panel sets them. -// -// These are *not* layer parameters: nothing here is stored on TextureDisplacementLayer and nothing here is -// re-applied at bake time. The panel applies them once, to the image, and the result becomes the layer's -// image - so the adjusted pixels feed the colour and the displacement together, and a brightness change -// really does change the relief. That is the intended behaviour: one image, one source of truth, rather -// than a second set of knobs the bake would have to replay in the right order. -// -// Contrast with TextureDisplacementLayer::smoothing, which *is* a layer parameter applied on every decode: -// `blur` here is a destructive edit of the image, `smoothing` is a live low-pass over whatever the image is. -struct TextureAdjustments -{ - float brightness = 0.f; // [-1, 1]; 0 unchanged, -1 black, +1 white - float contrast = 0.f; // [-1, 1]; 0 unchanged, -1 flat mid-grey, +1 hard threshold - float sharpness = 0.f; // [0, 1]; 0 unchanged. Unsharp-mask amount - float blur = 0.f; // [0, 1]; 0 unchanged. Fraction of SOURCE_BLUR_SPAN, as `smoothing` is - float hue_deg = 0.f; // [-180, 180]; 0 unchanged. No effect on a grayscale image - grey has no hue - - bool identity() const - { - return brightness == 0.f && contrast == 0.f && sharpness == 0.f && blur == 0.f && hue_deg == 0.f; - } - // Exact equality, which is what is wanted: these come straight off sliders, and the question asked of - // them is "is this still the value the preview on screen was computed from", not "are these close". - bool operator==(const TextureAdjustments &o) const - { - return brightness == o.brightness && contrast == o.contrast && sharpness == o.sharpness && - blur == o.blur && hue_deg == o.hue_deg; - } - bool operator!=(const TextureAdjustments &o) const { return !(*this == o); } -}; - // Applies `adj` to `src` and returns the result; `src` is left alone. An empty `src`, or an identity // `adj`, gives back a copy. // diff --git a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp index 645bb3ccc2..d68ac69a6b 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp @@ -2463,9 +2463,12 @@ void GLGizmoTextureDisplacement::update_uv_editor() // before, and independently of, the unwrap - the texture belongs on screen before anything is unwrapped - and // only when the choice, the layer's image or its smoothing changed, because the height image is large. const UVBackground desired_bg = (m_uv_check_mode == UVCheckMode::Checker) ? UVBackground::Checker : UVBackground::Height; - const bool bg_smoothing_changed = (desired_bg == UVBackground::Height) && (m_uv_editor_bg_smoothing != layer->smoothing); + const bool bg_smoothing_changed = (desired_bg == UVBackground::Height) && + (m_uv_editor_bg_smoothing != layer->smoothing || + !(m_uv_editor_bg_adjust == layer->preview_adjust)); if (desired_bg != m_uv_editor_bg || layer->image_data.get() != m_uv_editor_bg_image || bg_smoothing_changed) { m_uv_editor_bg_smoothing = layer->smoothing; + m_uv_editor_bg_adjust = layer->preview_adjust; m_uv_editor_bg_image = layer->image_data.get(); m_uv_editor_bg = desired_bg; if (desired_bg == UVBackground::Checker) { @@ -3691,7 +3694,7 @@ void GLGizmoTextureDisplacement::ensure_panel_icons() "menu_obj_sphere.svg", "menu_obj_cube.svg", "menu_obj_cylinder.svg", "texture_displacement_map_unwrap.svg", "texture_displacement_map_view.svg", "texture_displacement_tile_repeat.svg", "menu_mirror_x.svg", "texture_displacement_adjust.svg", "canvas_drag.svg", "texture_displacement_move_up.svg", - "texture_displacement_move_down.svg", "texture_displacement_drag.svg", + "texture_displacement_move_down.svg", "texture_displacement_drag.svg", "cog.svg", "texture_displacement_select_all.svg", "texture_displacement_erase_all.svg", }; std::vector paths; @@ -4000,22 +4003,22 @@ void GLGizmoTextureDisplacement::open_image_adjust(const TextureDisplacementLaye if (layer.empty() || layer.slot < 0) return; - close_image_adjust(); // drop any previous panel's source image and in-flight pass + close_image_adjust(); // reverts and clears any previous panel - // The *raw* image, not decode_height_texture(layer): that one applies the layer's Smoothing on top, - // and Smoothing is a live layer parameter rather than part of the image. Baking it into the pixels - // here would apply it twice - once destructively now, and again on every later decode. + // The *raw* image: neither the layer's Smoothing nor a previous preview_adjust, so reopening the panel + // always starts from the image as it is actually stored rather than from the last preview of it. TextureDisplacementLayer raw = layer; - raw.smoothing = 0.f; - m_adjust_source = decode_height_texture(raw); + raw.smoothing = 0.f; + raw.preview_adjust = TextureAdjustments{}; + m_adjust_source = decode_height_texture(raw); if (m_adjust_source.empty()) return; m_adjust_preview_source = downscale_height_texture(m_adjust_source, ADJUST_PREVIEW_MAX_PX); m_adjust = TextureAdjustments{}; + m_adjust_applied = TextureAdjustments{}; m_adjust_preview_shown = TextureAdjustments{}; m_adjust_slot = layer.slot; - m_adjust_open_request = true; // The unadjusted image, so the box is filled the moment the panel appears rather than one pass later. m_adjust_preview_tex = upload_height_thumbnail(m_adjust_preview_source, ADJUST_PREVIEW_MAX_PX); m_adjust_preview_valid = m_adjust_preview_tex != nullptr; @@ -4023,6 +4026,18 @@ void GLGizmoTextureDisplacement::open_image_adjust(const TextureDisplacementLaye void GLGizmoTextureDisplacement::close_image_adjust() { + // Revert the layer's live adjustment. Every exit that reaches here is a discard: a commit resets + // preview_adjust itself, before pointing the layer at the file that already carries the edit, so this + // cannot undo a committed adjustment. + bool reverted = false; + if (m_adjust_slot >= 0) + if (ModelVolume *mv = texture_volume()) + for (TextureDisplacementLayer &l : mv->texture_displacement_layers) + if (l.slot == m_adjust_slot && !l.preview_adjust.identity()) { + l.preview_adjust = TextureAdjustments{}; + reverted = true; + } + // std::async's future blocks here until the pass finishes. That is a few milliseconds on an image // this small, and the alternative - a detached thread writing into the members of a closed panel - // is worse. @@ -4032,10 +4047,19 @@ void GLGizmoTextureDisplacement::close_image_adjust() m_adjust_slot = -1; m_adjust = TextureAdjustments{}; + m_adjust_applied = TextureAdjustments{}; m_adjust_source = DecodedHeightTexture{}; m_adjust_preview_source = DecodedHeightTexture{}; m_adjust_preview_tex.reset(); m_adjust_preview_valid = false; + + // Only while the gizmo is up: on_shutdown() also comes through here, and rebuilding a preview for a + // gizmo that is closing would be work for a model nobody is looking at. The reverted layer data is + // what matters there, and that is already done above. + if (reverted && m_state == On) { + rebuild_preview(); + m_parent.set_as_dirty(); + } } void GLGizmoTextureDisplacement::update_image_adjust_preview() @@ -4074,103 +4098,74 @@ void GLGizmoTextureDisplacement::update_image_adjust_preview() m_parent.set_as_dirty(); } -void GLGizmoTextureDisplacement::commit_image_adjust(TextureDisplacementLayer &layer, bool as_copy) +void GLGizmoTextureDisplacement::commit_image_adjust(TextureDisplacementLayer &layer) { if (m_adjust_source.empty() || m_adjust.identity()) { - // Nothing was changed, so there is nothing to apply. Replacing the image with an identical one - // would still cost an undo step and invalidate every cache keyed by the old pointer. + // Nothing was changed, so there is nothing to save. close_image_adjust(); return; } // The full-size pass, once: 13-19 ms grayscale / 40-60 ms colour on a 1024x1024 with every knob // engaged. Run on the UI thread deliberately, unlike the live preview: this is a single click, and the - // rebuild_preview() immediately below it - a CPU mesh displacement of the painted patch - is already - // an order of magnitude more work, synchronously, so moving only this off the thread would buy - // nothing a user could perceive. + // rebuild that follows it - a CPU mesh displacement of the painted patch - is already an order of + // magnitude more work, synchronously, so moving only this off the thread would buy nothing a user + // could perceive. const DecodedHeightTexture adjusted = apply_texture_adjustments(m_adjust_source, m_adjust); - std::string error; - if (as_copy) { - // A new library texture, which set_layer_texture() then points the layer at - so the layer also - // picks up the name, the recently-used ordering and the fresh image_data buffer that path - // already provides, rather than this function repeating any of it. - const std::string stem = layer.name.empty() ? std::string("texture") : layer.name; - const std::optional entry = save_texture_to_library(adjusted, stem, error); - if (!entry) { - show_error(nullptr, error); - // Reopened rather than merely left alone: the popup was closed before this call, and without - // this the next frame would see a closed popup with panel state still set and discard the - // edit. Same reason import_custom_texture() re-requests the picker after its file dialog. - m_adjust_open_request = true; - return; - } - set_layer_texture(layer, *entry); - } else { - // A brand new allocation, which is what invalidates decode_height_texture()'s cache, - // analyze_texture_detail()'s, and the GPU thumbnail and height/colour textures - every one of - // them keyed by this pointer. Writing into the buffer the layer already holds would leave all of - // them serving the pixels from before the edit. - std::shared_ptr> bytes = encode_texture_image_data(adjusted, error); - if (!bytes) { - show_error(nullptr, error); - m_adjust_open_request = true; // as above: keep the edit, bring the panel back - return; - } - Plater::TakeSnapshot snapshot(wxGetApp().plater(), _u8L("Adjust texture image"), - UndoRedo::SnapshotType::GizmoAction); - layer.image_data = std::move(bytes); - // The image is no longer any file on disk, so no library entry should show as this layer's any - // more (the picker highlights by this path). The bytes themselves are what the project stores. - layer.path.clear(); - layer.path_in_3mf.clear(); - rebuild_preview(); - m_parent.set_as_dirty(); + std::string error; + const std::string stem = layer.name.empty() ? std::string("texture") : layer.name; + const std::optional entry = save_texture_to_library(adjusted, stem, error); + if (!entry) { + show_error(nullptr, error); + return; // the panel stays open, and the layer keeps the live adjustment: the edit is not lost } + // Cleared *before* the layer is pointed at the new file, and this ordering is the whole correctness of + // the commit: the saved image already has the adjustment baked into its pixels, so leaving + // preview_adjust set would apply brightness, contrast and the rest a second time on top of itself. + layer.preview_adjust = TextureAdjustments{}; + // set_layer_texture() does the rest - the name, the recently-used ordering, a fresh image_data buffer + // (which is what invalidates every cache keyed by the old pointer), the undo snapshot and the rebuild. + set_layer_texture(layer, *entry); + close_image_adjust(); } -void GLGizmoTextureDisplacement::render_image_adjust_popup(const ImVec2 &panel_min, const ImVec2 &panel_max) +void GLGizmoTextureDisplacement::render_image_adjust_window(const ImVec2 &panel_min, const ImVec2 &panel_max) { + if (m_adjust_slot < 0) + return; + ModelVolume *mv = texture_volume(); TextureDisplacementLayer *layer = nullptr; if (mv != nullptr) for (TextureDisplacementLayer &l : mv->texture_displacement_layers) if (l.slot == m_adjust_slot) layer = &l; - - static const char *const popup_id = "##texture_adjust"; - if (m_adjust_open_request) { - m_adjust_open_request = false; - if (layer != nullptr) - ImGui::OpenPopup(popup_id); - } - - const float img_w = std::round(m_imgui->scaled(16.f)); - const float gap = m_imgui->scaled(0.5f); - // Beside the panel, like the texture picker, so the layer being adjusted stays in view. - const float popup_w = img_w + 2.f * ImGui::GetStyle().WindowPadding.x; - const bool left = panel_min.x - gap - popup_w >= 0.f; - ImGui::SetNextWindowPos(ImVec2(left ? panel_min.x - gap : panel_max.x + gap, panel_min.y + m_imgui->scaled(4.f)), - ImGuiCond_Appearing, ImVec2(left ? 1.f : 0.f, 0.f)); - if (!ImGui::BeginPopupModal(popup_id, nullptr, ImGuiWindowFlags_AlwaysAutoResize | ImGuiWindowFlags_NoTitleBar)) { - // The panel thinks it is open but the popup is not: Escape, or the layer went away. Every one of - // those routes discards the edit, which is exactly what Cancel does. - if (m_adjust_slot >= 0) - close_image_adjust(); - return; - } if (layer == nullptr) { - ImGui::CloseCurrentPopup(); - ImGui::EndPopup(); + // The layer went away under the panel (removed, or the selection changed). close_image_adjust(); return; } - m_imgui->push_bold_font(); - m_imgui->text(_L("Adjust image")); - m_imgui->pop_bold_font(); + const float img_w = std::round(m_imgui->scaled(16.f)); + const float gap = m_imgui->scaled(0.5f); + // Placed beside the panel on first appearance only, and left wherever the user drags it after that - + // it is a window, not a popup, so its position is theirs to keep. + const float win_w = img_w + 2.f * ImGui::GetStyle().WindowPadding.x; + const bool left = panel_min.x - gap - win_w >= 0.f; + ImGui::SetNextWindowPos(ImVec2(left ? panel_min.x - gap - win_w : panel_max.x + gap, + panel_min.y + m_imgui->scaled(4.f)), ImGuiCond_Appearing); + + bool keep_open = true; + if (!ImGui::Begin((_u8L("Adjust image") + "##texture_adjust").c_str(), &keep_open, + ImGuiWindowFlags_AlwaysAutoResize | ImGuiWindowFlags_NoCollapse)) { + ImGui::End(); + if (!keep_open) + close_image_adjust(); + return; + } // Collect a finished pass and start one for the current slider values before the image is drawn, so // the newest available result is what lands on screen this frame. @@ -4194,7 +4189,8 @@ void GLGizmoTextureDisplacement::render_image_adjust_popup(const ImVec2 &panel_m // width comes from its content, and an item asking for "whatever is left" would make the width depend // on itself. The image above sets the content width, and the sliders match it. const float slider_w = std::max(m_imgui->scaled(4.f), img_w - label_w); - const auto row = [&](const char *id, const wxString &label, float *v, float v_min, float v_max, + + const auto row = [&](const char *id, const wxString &label, float *v, float v_min, float v_max, const char *format, bool enabled, const std::string &tip) { const float x0 = ImGui::GetCursorPosX(); ImGui::AlignTextToFramePadding(); @@ -4217,48 +4213,58 @@ void GLGizmoTextureDisplacement::render_image_adjust_popup(const ImVec2 &panel_m _u8L("Brings out the finest detail in the image. Applied after Blur, so the two can be used " "together: blur away the speckle, then sharpen the edges that remain.")); row("##adj_blur", _L("Blur"), &m_adjust.blur, 0.f, 1.f, "%.2f", true, - _u8L("Blurs the image itself, permanently. The layer's own Smoothing slider softens the relief " - "without altering the image, and is the one to reach for first.")); + _u8L("Blurs the image itself. The layer's own Smoothing slider softens the relief without altering " + "the image, and is the one to reach for first.")); // Grey has no hue to turn, and the shipped library is grayscale - so on most textures this is a // slider that would visibly do nothing. Disabled with the reason given, rather than hidden. const bool has_color = m_adjust_source.has_color(); row("##adj_hue", _L("Hue"), &m_adjust.hue_deg, -180.f, 180.f, "%.0f°", has_color, {}); if (!has_color) { ImGui::PushTextWrapPos(ImGui::GetCursorPosX() + img_w); - m_imgui->text(_L("This texture is grayscale, so it has no hue to shift.")); + ImGui::TextDisabled("%s", _u8L("This texture is grayscale, so it has no hue to shift.").c_str()); ImGui::PopTextWrapPos(); } if (m_imgui->button(_L("Reset"))) m_adjust = TextureAdjustments{}; + // Hand the sliders to the layer, which is what puts the adjusted image in front of the displacement: + // decode_height_texture() applies preview_adjust, so the relief, the colour and the thumbnails all + // follow from here. Written every frame rather than only on a change, so an undo snapshot or a + // reselection that resets the layer cannot leave the model showing an adjustment the panel has moved + // on from. + layer->preview_adjust = m_adjust; + if (m_adjust != m_adjust_applied) { + m_adjust_applied = m_adjust; + // Marked dirty rather than rebuilt here: the panel's own flag is consumed by the main panel's + // "rebuild once the mouse is released" rule, so dragging a slider recomputes the displacement + // once, on release, instead of on every frame of the drag. That is what keeps the UI responsive + // while a full-size resample and a mesh displacement happen behind it. + m_preview_params_dirty = true; + // This window is drawn after the block that consumes that flag, so the rebuild happens on a later + // frame - which means there has to *be* one. Cheap insurance: a slider release that generated no + // further canvas event would otherwise leave the flag set and the model unchanged until something + // unrelated redrew. + m_parent.set_as_dirty(); + } + ImGui::Separator(); - // The part a user would not otherwise expect: this is an edit to the image, and the image is also the - // height map. - m_imgui->text_wrapped(_u8L("The adjusted image becomes the layer's image, so it changes the relief as " - "well as the colour."), img_w); + m_imgui->text_wrapped(_u8L("The model updates when you release a slider. Save as copy writes the " + "adjusted image to your texture library and points this layer at it."), img_w); const bool cancel = m_imgui->button(_L("Cancel")); ImGui::SameLine(); - // Nothing to apply until a slider has actually moved. + // Nothing to save until a slider has actually moved. m_imgui->disabled_begin(m_adjust.identity()); - const bool overwrite = m_imgui->button(_L("Overwrite")); - ImGui::SameLine(); const bool as_copy = m_imgui->button(_L("Save as copy")); m_imgui->disabled_end(); - // Closed before anything commits: commit_image_adjust() can put a modal error dialog up, and an ImGui - // popup left open underneath one is what makes the panel come back in a half-drawn state afterwards. - if (cancel || overwrite || as_copy) - ImGui::CloseCurrentPopup(); - ImGui::EndPopup(); + ImGui::End(); - if (cancel) + if (cancel || !keep_open) close_image_adjust(); - else if (overwrite) - commit_image_adjust(*layer, false); else if (as_copy) - commit_image_adjust(*layer, true); + commit_image_adjust(*layer); } void GLGizmoTextureDisplacement::remove_texture_layer(int slot) @@ -4318,6 +4324,7 @@ void GLGizmoTextureDisplacement::swap_layer_slots(int slot_a, int slot_b) std::swap(m_thumbnails[a], m_thumbnails[b]); std::swap(m_thumbnail_source[a], m_thumbnail_source[b]); std::swap(m_thumbnail_smoothing[a], m_thumbnail_smoothing[b]); + std::swap(m_thumbnail_adjust[a], m_thumbnail_adjust[b]); std::swap(m_layer_expanded[a], m_layer_expanded[b]); if (m_active_layer_slot == slot_a) m_active_layer_slot = slot_b; @@ -5581,7 +5588,7 @@ GLTexture *GLGizmoTextureDisplacement::get_layer_thumbnail(const TextureDisplace const size_t slot = size_t(layer.slot); if (m_thumbnails[slot] && m_thumbnail_source[slot] == layer.image_data.get() && - m_thumbnail_smoothing[slot] == layer.smoothing) + m_thumbnail_smoothing[slot] == layer.smoothing && m_thumbnail_adjust[slot] == layer.preview_adjust) return m_thumbnails[slot].get(); // Reuses the already-decoded, already-cached grayscale pixels (see decode_height_texture()'s @@ -5595,6 +5602,7 @@ GLTexture *GLGizmoTextureDisplacement::get_layer_thumbnail(const TextureDisplace m_thumbnails[slot] = std::move(texture); m_thumbnail_source[slot] = layer.image_data.get(); m_thumbnail_smoothing[slot] = layer.smoothing; + m_thumbnail_adjust[slot] = layer.preview_adjust; return m_thumbnails[slot].get(); } @@ -5602,7 +5610,8 @@ GLTexture *GLGizmoTextureDisplacement::get_layer_color_texture(const TextureDisp { if (layer.empty() || !layer.color_enabled) return nullptr; - if (m_color_tex && m_color_tex_source == layer.image_data.get() && m_color_tex_smoothing == layer.smoothing) + if (m_color_tex && m_color_tex_source == layer.image_data.get() && m_color_tex_smoothing == layer.smoothing && + m_color_tex_adjust == layer.preview_adjust) return m_color_tex.get(); std::unique_ptr texture = upload_color_texture(decode_height_texture(layer), HEIGHT_TEX_MAX_PX); @@ -5614,6 +5623,7 @@ GLTexture *GLGizmoTextureDisplacement::get_layer_color_texture(const TextureDisp m_color_tex = std::move(texture); m_color_tex_source = layer.image_data.get(); m_color_tex_smoothing = layer.smoothing; + m_color_tex_adjust = layer.preview_adjust; return m_color_tex.get(); } @@ -5624,7 +5634,8 @@ GLTexture *GLGizmoTextureDisplacement::get_layer_height_texture(const TextureDis // One slot, not one per layer: the preview shader only ever shades the *active* layer, so a single // full-resolution upload is enough and the VRAM cost stays at one texture rather than eight. - if (m_height_tex && m_height_tex_source == layer.image_data.get() && m_height_tex_smoothing == layer.smoothing) + if (m_height_tex && m_height_tex_source == layer.image_data.get() && m_height_tex_smoothing == layer.smoothing && + m_height_tex_adjust == layer.preview_adjust) return m_height_tex.get(); std::unique_ptr texture = upload_height_thumbnail(decode_height_texture(layer), HEIGHT_TEX_MAX_PX); @@ -5634,6 +5645,7 @@ GLTexture *GLGizmoTextureDisplacement::get_layer_height_texture(const TextureDis m_height_tex = std::move(texture); m_height_tex_source = layer.image_data.get(); m_height_tex_smoothing = layer.smoothing; + m_height_tex_adjust = layer.preview_adjust; return m_height_tex.get(); } @@ -6690,7 +6702,7 @@ void GLGizmoTextureDisplacement::on_render_input_window(float x, float y, float ImGui::SameLine(); const std::string not_painted = _u8L("not painted"); - const float right_w = (painted ? 0.f : ImGui::CalcTextSize(not_painted.c_str()).x + gap_s) + 3.f * frame_h + 2.f * gap_s; + const float right_w = (painted ? 0.f : ImGui::CalcTextSize(not_painted.c_str()).x + gap_s) + 4.f * frame_h + 3.f * gap_s; const float name_w = std::max(frame_h, ImGui::GetWindowContentRegionMax().x - card_pad - ImGui::GetCursorPosX() - gap_s - right_w); ImGui::PushStyleColor(ImGuiCol_HeaderHovered, ImVec4(orca.x, orca.y, orca.z, 0.2f)); ImGui::PushStyleColor(ImGuiCol_HeaderActive, ImVec4(orca.x, orca.y, orca.z, 0.35f)); @@ -6704,6 +6716,16 @@ void GLGizmoTextureDisplacement::on_render_input_window(float x, float y, float ImGui::SameLine(); ImGui::TextDisabled("%s", not_painted.c_str()); } + // Between the name and the order arrows: it acts on the image the name refers to, so it + // belongs with the name rather than with the controls that reorder the stack. + ImGui::SameLine(); + m_imgui->disabled_begin(busy || layer.empty()); + if (icon_button(613 + layer.slot, "cog.svg", frame_h, _L("Adjust image"), + _u8L("Brightness, contrast, sharpness, blur and hue, shown on the image and on the " + "model as you set them. Save as copy writes the result to your texture library " + "and points this layer at it."))) + open_image_adjust(layer); + m_imgui->disabled_end(); ImGui::SameLine(); m_imgui->disabled_begin(busy || li == 0); if (icon_button(611, "texture_displacement_move_up.svg", frame_h, _L("Move up"), _L("Move up - applied earlier"))) { @@ -6770,16 +6792,6 @@ void GLGizmoTextureDisplacement::on_render_input_window(float x, float y, float "speckle from a noisy photo. Raise it if the relief looks harsh or grainy; it " "costs fine detail.")); - // Next to Smoothing because both are about the image rather than the projection - but - // note the difference: Smoothing is a live layer parameter, while this edits the image. - m_imgui->disabled_begin(layer.empty()); - if (m_imgui->button(_L("Adjust image") + "##adjust_image")) - open_image_adjust(layer); - m_imgui->disabled_end(); - hover_tip(_u8L("Brightness, contrast, sharpness, blur and hue, shown on the image as you " - "set them. The adjusted image becomes this layer's image, so it changes the " - "relief as well as the colour.")); - // Edge fade: the relief flattens toward the boundary of the painted area. m_preview_params_dirty |= m_imgui->bbl_checkbox(_L("Edge fade") + "##edge_smoothing", layer.edge_smoothing); hover_tip(_u8L("Flattens the relief as it approaches the edge of the painted area, so it " @@ -7513,11 +7525,15 @@ void GLGizmoTextureDisplacement::on_render_input_window(float x, float y, float set_active_layer(m_picker_slot); // In this window's scope, where the popup's open request and the popup itself share one ID stack. + ImVec2 panel_rect_min, panel_rect_max; { const ImVec2 win_min = ImGui::GetWindowPos(); const ImVec2 win_sz = ImGui::GetWindowSize(); render_texture_library_popup(win_min, ImVec2(win_min.x + win_sz.x, win_min.y + win_sz.y)); - render_image_adjust_popup(win_min, ImVec2(win_min.x + win_sz.x, win_min.y + win_sz.y)); + // Only recorded here. The image-adjustment panel is a top-level window rather than a popup, so it + // cannot be begun inside this one; it is drawn after GizmoImguiEnd() below. + panel_rect_min = win_min; + panel_rect_max = ImVec2(win_min.x + win_sz.x, win_min.y + win_sz.y); } // ImGui sliders report "changed" continuously on every frame while being dragged, not just once on @@ -7754,6 +7770,11 @@ void GLGizmoTextureDisplacement::on_render_input_window(float x, float y, float m_parent.request_extra_frame(); } GizmoImguiEnd(); + + // Outside the gizmo's window, because it is a window of its own: a floating, non-modal panel that + // leaves the 3D view live so the relief can be watched while the sliders move. No-op when closed. + render_image_adjust_window(panel_rect_min, panel_rect_max); + ImGuiWrapper::pop_toolbar_style(); // Drawn last, over everything, via the foreground draw list: the +/- add-remove sign next to the 3D diff --git a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp index 1edb9f4103..468c5bae63 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp @@ -407,25 +407,29 @@ private: // layer's *image*, so the adjusted pixels feed the colour and the displacement together and a // brightness change moves the relief too (see TextureAdjustments). // - // Modal, unlike the texture picker beside it, because it has three distinct exits - discard, - // overwrite the layer's image, write a new library texture - and a click outside silently - // discarding the edit is not one of them. - void render_image_adjust_popup(const ImVec2 &panel_min, const ImVec2 &panel_max); + // An ordinary floating window rather than a popup, and deliberately so. A popup dims the canvas + // behind it and closes on any click outside itself, which between them make it impossible to watch the + // relief change on the model - which is the whole point of adjusting the image here rather than in an + // image editor. A window leaves the 3D view live, clickable and orbitable while the panel is open. + void render_image_adjust_window(const ImVec2 &panel_min, const ImVec2 &panel_max); // Opens the panel for `layer` and decodes its image once, into m_adjust_source. Sliders never // re-decode the PNG. void open_image_adjust(const TextureDisplacementLayer &layer); - // Gives `layer` the full-size adjusted image: as its own image_data (`as_copy` false), or by writing - // a new texture into the user's library and pointing the layer at that (`as_copy` true). Closes the - // panel either way. Takes an undo snapshot. - void commit_image_adjust(TextureDisplacementLayer &layer, bool as_copy); + // Resolves the panel: writes the adjusted image into the user's texture library as a new file and + // points `layer` at it, which is the only way an adjustment becomes permanent. Closes the panel. + void commit_image_adjust(TextureDisplacementLayer &layer); + // Closes the panel and reverts the layer's live preview_adjust to identity, so every exit that is not + // a commit - Cancel, the window's close button, the gizmo closing - leaves the layer as it was found. void close_image_adjust(); // Collects a finished preview and starts one for the current slider values. One at a time, so a drag // coalesces into "whatever the sliders last said" instead of a queue of superseded frames. void update_image_adjust_preview(); - int m_adjust_slot = -1; // the layer the panel is open for; -1 when closed - bool m_adjust_open_request = false; - TextureAdjustments m_adjust; // what the sliders currently say + int m_adjust_slot = -1; // the layer the panel is open for; -1 when closed + TextureAdjustments m_adjust; // what the sliders currently say + // What the layer's live preview_adjust was last set from, so a slider move can be told from a frame in + // which nothing happened and only the former asks the displacement to be recomputed. + TextureAdjustments m_adjust_applied; // The layer's image as it was when the panel opened, and a small copy of it. The sliders are applied // to the small copy for the live preview and to the full-size one exactly once, on commit. Measured on @@ -926,6 +930,7 @@ private: enum class UVBackground { None, Height, Checker }; UVBackground m_uv_editor_bg = UVBackground::None; float m_uv_editor_bg_smoothing = -1.f; // smoothing the height backdrop was uploaded at + TextureAdjustments m_uv_editor_bg_adjust; // and the live image adjustment it was uploaded at const void *m_uv_editor_bg_image = nullptr; // the layer image it was uploaded from // Per-chart distortion heatmap colour for the UV pane (#7/#14), computed once when the unwrap is // re-solved (relative stretch doesn't change when islands are merely moved), fed to the canvas only @@ -969,6 +974,10 @@ private: std::array m_thumbnail_source{}; // The smoothing each cached thumbnail was built at, so a smoothing change re-uploads it. std::array m_thumbnail_smoothing{}; + // And the live image adjustment each was built at, so moving a slider in the image-adjustment panel + // re-uploads it. Part of the key for the same reason smoothing is: preview_adjust changes the pixels + // decode_height_texture() hands back, and every one of these textures is a copy of those pixels. + std::array m_thumbnail_adjust{}; // Full-resolution height texture for the preview shader, keyed the same way (see // get_layer_height_texture()). A smoothing change re-uploads it, so the fast preview shows the @@ -976,12 +985,14 @@ private: std::unique_ptr m_height_tex; const void *m_height_tex_source = nullptr; float m_height_tex_smoothing = -1.f; + TextureAdjustments m_height_tex_adjust; // The same, for the layer's *colour*, which the fast preview quantizes per fragment so it shows // the image at texel resolution rather than at the mesh's. Null for a grayscale texture. std::unique_ptr m_color_tex; const void *m_color_tex_source = nullptr; float m_color_tex_smoothing = -1.f; + TextureAdjustments m_color_tex_adjust; // Library textures the picker has shown at least once, keyed by file path (see LibraryTexture). std::map m_library_textures; diff --git a/src/slic3r/GUI/TextureLibrary.cpp b/src/slic3r/GUI/TextureLibrary.cpp index 3f7fa89f83..765ea680ff 100644 --- a/src/slic3r/GUI/TextureLibrary.cpp +++ b/src/slic3r/GUI/TextureLibrary.cpp @@ -411,21 +411,6 @@ std::shared_ptr> load_texture_image_data(const std::s return std::make_shared>(std::move(converted)); } -std::shared_ptr> encode_texture_image_data(const DecodedHeightTexture &image, std::string &error) -{ - std::vector scratch; - const wxImage wx = image_from_decoded(image, scratch); - if (!wx.IsOk()) { - error = _u8L("The selected image is empty."); - return nullptr; - } - - std::vector bytes; - if (!encode_png_bytes(wx, bytes, error)) - return nullptr; - return std::make_shared>(std::move(bytes)); -} - std::optional save_texture_to_library(const DecodedHeightTexture &image, const std::string &stem, std::string &error) { diff --git a/src/slic3r/GUI/TextureLibrary.hpp b/src/slic3r/GUI/TextureLibrary.hpp index faf2f521af..38ad277a84 100644 --- a/src/slic3r/GUI/TextureLibrary.hpp +++ b/src/slic3r/GUI/TextureLibrary.hpp @@ -59,19 +59,6 @@ std::optional import_texture_to_library(const std::string & // Returns nullptr with `error` set if the file cannot be read or decoded at all. std::shared_ptr> load_texture_image_data(const std::string &path, std::string &error); -// Encodes already-decoded pixels back into the PNG form a layer's image_data holds - grayscale in, -// grayscale out, as on import. This is how an edited image (see apply_texture_adjustments()) becomes a -// layer's image. -// -// The returned buffer is always a *fresh allocation*, which is load-bearing rather than incidental: -// decode_height_texture() and analyze_texture_detail() both cache by the image_data pointer, so writing -// new bytes into the buffer a layer already holds would leave every one of those caches - and the GPU -// thumbnail and height/colour textures, keyed the same way - serving the old pixels. Assigning a new -// allocation is what invalidates them. -// -// Returns nullptr with `error` set if the pixels are empty or the encode fails. -std::shared_ptr> encode_texture_image_data(const DecodedHeightTexture &image, std::string &error); - // Writes `image` into user_texture_dir() as a new PNG named after `stem` (uniquified if taken, so an // existing texture is never overwritten) and returns its library entry. The in-memory counterpart of // import_texture_to_library(), for an image that was edited rather than read off disk. diff --git a/tests/libslic3r/test_texture_displacement.cpp b/tests/libslic3r/test_texture_displacement.cpp index b088d3b86d..e6de116e24 100644 --- a/tests/libslic3r/test_texture_displacement.cpp +++ b/tests/libslic3r/test_texture_displacement.cpp @@ -2411,3 +2411,54 @@ TEST_CASE("downscaling a height texture averages the texels it drops and never u CHECK(wide.height == 25); CHECK(wide.has_color()); } + +TEST_CASE("a layer's live image adjustment reaches its decode", "[TextureDisplacement]") +{ + // This is what makes the image-adjustment panel's sliders move the relief: decode_height_texture() + // applies the layer's preview_adjust, so everything downstream of the decode - the displacement, the + // colour, the thumbnails, the bake - sees the adjusted image without being told about the panel. + TextureDisplacementLayer layer; + layer.image_data = make_checkerboard_png(32, 32); + + const DecodedHeightTexture raw = decode_height_texture(layer); + REQUIRE_FALSE(raw.empty()); + + // An untouched panel, and a closed one, cost nothing and change nothing. + layer.preview_adjust = TextureAdjustments{}; + CHECK(decode_height_texture(layer).pixels == raw.pixels); + + layer.preview_adjust.brightness = 0.5f; + const DecodedHeightTexture bright = decode_height_texture(layer); + CHECK(bright.pixels != raw.pixels); + CHECK(bright.pixels == apply_texture_adjustments(raw, layer.preview_adjust).pixels); + + // Back to identity gives the stored image back, so closing the panel really does revert the layer. + layer.preview_adjust = TextureAdjustments{}; + CHECK(decode_height_texture(layer).pixels == raw.pixels); +} + +TEST_CASE("changing a live adjustment is not served the previous one from cache", "[TextureDisplacement]") +{ + // The decode caches by the image_data pointer, which does not change while a slider moves - so the + // adjustment has to be part of the key or every slider move past the first would redraw the same + // pixels. Same for the smoothed copy, which is computed on top of the adjusted one. + TextureDisplacementLayer layer; + layer.image_data = make_checkerboard_png(32, 32); + const DecodedHeightTexture raw = decode_height_texture(layer); + REQUIRE_FALSE(raw.empty()); + + layer.preview_adjust.brightness = 0.5f; + const std::vector bright = decode_height_texture(layer).pixels; + layer.preview_adjust.brightness = -0.5f; + const std::vector dark = decode_height_texture(layer).pixels; + CHECK(bright != dark); + CHECK(dark == apply_texture_adjustments(raw, layer.preview_adjust).pixels); + + // The same question again with smoothing on, which is cached separately and keyed by both. + layer.smoothing = 0.5f; + layer.preview_adjust.brightness = 0.5f; + const std::vector smooth_bright = decode_height_texture(layer).pixels; + layer.preview_adjust.brightness = -0.5f; + const std::vector smooth_dark = decode_height_texture(layer).pixels; + CHECK(smooth_bright != smooth_dark); +}