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); +}