Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion qml/PropertiesPanel.qml
Original file line number Diff line number Diff line change
Expand Up @@ -2111,9 +2111,17 @@ Rectangle {
// image was picked (ready by now — no UI-blocking
// captioning here). Empty falls back to a neutral prompt
// inside generateMeshTextureMultiView.
//
// Views: front/back/left/right (MV-Adapter #805 slice 1
// — its 6-view orthographic layout minus the two poles).
// 4 equatorial views give full horizontal coverage +
// seam overlap for the baker's cross-view blend, a clear
// step up from the old 2 (front+back left the sides to
// stretch/blur) without the ~3× cost of all 6. The two
// poles (top/bottom) rarely help typical subjects.
MaterialEditorQML.generateMeshTextureMultiView(
MeshGenController.caption, 512, 512, 0.9,
["front", "back"],
["front", "back", "left", "right"],
"", // no photo pinning
mgPbr.checked)
}
Expand Down
25 changes: 22 additions & 3 deletions src/ImageTo3D/ImageCaptioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <mtmd.h>
#include <mtmd-helper.h>

#include <mutex>
#include <string>
#include <vector>
#endif
Expand Down Expand Up @@ -117,6 +118,16 @@
if (image.isNull() || !modelsPresent())
return {};

// Captioning runs on a detached worker thread (MeshGenController), and the
// user can pick a new image while a previous caption is still in flight —
// spawning a second concurrent call. llama.cpp model load + inference is NOT
// safe to run from two threads at once (global ggml backend state); doing so
// produced garbage captions (e.g. a person still described from the earlier
// image). Serialise: only one caption() runs at a time. The caller's
// per-path guard (setCaptionResult) drops any now-stale result.
static std::mutex s_captionMutex;
std::lock_guard<std::mutex> lock(s_captionMutex);

Check warning on line 129 in src/ImageTo3D/ImageCaptioner.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Avoid explicitly specifying the template arguments by relying on the class template argument deduction.

See more on https://sonarcloud.io/project/issues?id=fernandotonon_QtMeshEditor&issues=AZ9HyBeLF8Pg92pVErTa&open=AZ9HyBeLF8Pg92pVErTa&pullRequest=827

Check warning on line 129 in src/ImageTo3D/ImageCaptioner.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this use of "std::lock_guard" with "std::scoped_lock"

See more on https://sonarcloud.io/project/issues?id=fernandotonon_QtMeshEditor&issues=AZ9HyBeLF8Pg92pVErTb&open=AZ9HyBeLF8Pg92pVErTb&pullRequest=827
Comment on lines +121 to +129

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the enclosing function of the caption() call in MaterialEditorQML and check its thread context
rg -nP -C40 'ImageCaptioner::caption\s*\(' src/MaterialEditorQML.cpp | rg -nP -C3 '(std::thread|QtConcurrent|QThread|invokeMethod|QueuedConnection|::run|detach)'

Repository: fernandotonon/QtMeshEditor

Length of output: 164


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate ImageCaptioner::caption usages and inspect surrounding context.
rg -n -C 30 'ImageCaptioner::caption\s*\(' src

# Find the specific MaterialEditorQML.cpp location if it exists under a different path.
fd -a 'MaterialEditorQML.cpp' src

# Search for the captioning flow and any thread dispatch around it.
rg -n -C 20 'startCaptioning|setCaptionResult|caption\(' src/MaterialEditorQML.cpp src/ImageTo3D/ImageCaptioner.cpp src

Repository: fernandotonon/QtMeshEditor

Length of output: 48898


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the surrounding class/interface to determine whether the SD flow is a QML/UI entry point.
rg -n -C 20 'class MaterialEditorQML|Q_INVOKABLE|generateSelected|sdGenerationNotice|sdGenerationError' src/MaterialEditorQML.h src/MaterialEditorQML.cpp

# Show the function signature around the caption call for direct context.
sed -n '4628,4685p' src/MaterialEditorQML.cpp

Repository: fernandotonon/QtMeshEditor

Length of output: 50382


Move captioning off the UI thread MaterialEditorQML.cpp:4664-4671 calls ensureModelBlocking() and ImageCaptioner::caption() inline in the QML SD flow, so the mutex can stall the editor for the full model load/inference. Mirror the detached-worker path used by MeshGenController.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ImageTo3D/ImageCaptioner.cpp` around lines 121 - 129, The QML SD flow is
still calling ImageCaptioner::caption() and ensureModelBlocking() synchronously
from MaterialEditorQML, so the new mutex can block the UI for the full
load/inference. Move this captioning work off the UI thread by mirroring the
detached-worker pattern used by MeshGenController, and keep the existing
per-path stale-result handling via setCaptionResult so only the async completion
updates state.


// ---- Load the text model + its mtmd (vision projector) context ----------
llama_model_params mparams = llama_model_default_params();
// Vision projector runs on CPU/accelerator via ggml; the small LM can put a
Expand Down Expand Up @@ -168,10 +179,16 @@
// ---- Build the prompt with the media marker + tokenize --------------
const QString instruction = prompt.isEmpty()
? QString::fromLatin1(kDefaultPrompt) : prompt;
// SmolVLM (idefics3) chat format; the media marker is replaced by the
// image chunk during tokenization.
// SmolVLM (idefics3) chat format, matching the GGUF's embedded Jinja
// template EXACTLY. For a user turn whose FIRST content item is an image
// the template emits "User:" with NO trailing space (a text-first turn
// would use "User: "), then the image, then the text:
// <|im_start|>User:<image>{instruction}<end_of_utterance>\nAssistant:
// The media marker is substituted for the real image chunk during
// tokenization. (The earlier "User: " + marker form had a stray space
// that mis-framed the turn.)
const std::string text =
std::string("<|im_start|>User: ") + mtmd_default_marker()
std::string("<|im_start|>User:") + mtmd_default_marker()
+ instruction.toStdString() + "<end_of_utterance>\nAssistant:";

mtmd_input_text itext;
Expand Down Expand Up @@ -210,6 +227,8 @@
char buf[256];
const int n = llama_token_to_piece(vocab, tok, buf, sizeof(buf), 0, true);
if (n > 0) out.append(buf, n);
// pos == nullptr here → llama_decode auto-assigns positions from the
// KV state (seq_pos_max + 1), continuing correctly past the image.
llama_batch nb = llama_batch_get_one(const_cast<llama_token*>(&tok), 1);
if (llama_decode(lctx, nb) != 0) break;
}
Expand Down
26 changes: 17 additions & 9 deletions src/ImageTo3D/ImageCaptioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,24 @@
namespace ImageCaptioner {

// True only when built with ENABLE_LOCAL_LLM.
// A DETAIL-seeking prompt: SmolVLM-500M defaults to terse answers ("a
// rabbit"), which makes a weak texture prompt. Explicitly ask for a rich,
// comma-separated visual description (colours, materials, markings, surface)
// and forbid the one-word reply, so the caption feeds SD with real texture cues.
// A DETAIL-seeking prompt: SmolVLM-500M defaults to terse answers ("a rabbit"),
// which makes a weak texture prompt, so we ask for a rich comma-separated
// visual description (colours, materials, markings, surface).
// IMPORTANT: do NOT include a concrete worked EXAMPLE here. SmolVLM-500M is
// small enough that it parroted a detailed example ("a fluffy brown and white
// rabbit, …") verbatim as its answer, ignoring the actual image entirely —
// which is exactly how a person's photo came back described as a rabbit. Keep
// the instruction abstract so the model has to look at the picture.
static constexpr const char* kDefaultPrompt =
"Describe this object in detail as a comma-separated visual texture prompt: "
"its type, all colours, materials, surface texture, patterns and markings. "
"Be specific and descriptive, not a single word. Example: 'a fluffy brown "
"and white rabbit, soft short fur, pink inner ears, dark eyes'. "
"Answer with only the description.";
"Describe ONLY the main subject of this image (ignore the background and "
"any scenery) as a detailed, comma-separated visual description for "
"texturing a 3D model of it. START by naming WHAT the subject is (for "
"example: a woman, a man, a rabbit, a car, a chair), then describe its "
"surface appearance: for a person or character give skin tone, hair colour "
"and style, eye colour, makeup, and clothing colours/materials; for an "
"animal or object give its colours, materials, surface texture, and any "
"patterns or markings. Be specific, not a single word, and do not mention "
"the background. Answer with only the description.";

bool isAvailable();

Expand Down
Loading
Loading