QVAC-21396 fix(mtmd/qwen3vl): grid selection rewrite + --image-max-tiles override

[yingying0906]

Summary

Rewrites Qwen3-VL multi-tile grid selection, adds an --image-max-tiles override, collapses the batched-attention loop, and makes sequential the default tile mode.

• Grid selection rewrite (mtmd-image.cpp): deterministic best-grid search over downscaling grids; falls back to dyn_size when no grid fits. Adds a global overview thumbnail at entries[0].
• --image-max-tiles override (arg.cpp, common.h, mtmd.{h,cpp}, server/cli wiring): user-facing cap on tiles per image (1..256), applied after load_hparams so GGUF defaults don't clobber it.
• Collapse batched attention loop + sequential default (clip.cpp, clip.h, qwen3vl.cpp): zero-copy 3D views over the batched QKV buffer instead of a per-tile loop; fixes the M-RoPE section-major position layout for batch_size>1; defaults --image-tile-mode to sequential.

Grid selection mechanism

For an nx × ny image with per-tile side tile_px (from model hparams) and a tile budget max_tiles. Each image is processed independently:

1. Enumerate every col × row grid with col*row ≤ max_tiles, excluding 1×1, keeping only grids that downscale in both axes (col*tile_px ≤ nx and row*tile_px ≤ ny) — upscaling adds no real detail.
2. Score each by log-ratio error ratio_err = |log(nx/ny) − log(col/row)|. Log space makes 2:1 and 1:2 symmetric around square. This is the per-axis stretch the resize-to-canvas step will impose.
3. No fit → dyn_size. If no grid downscales, use dyn_size (aspect-correct downscale at native detail, single tile).
4. Tolerance band + max tiles. Take all grids within RATIO_ERR_TOLERANCE (0.2) of the best ratio_err and pick the most tiles (more spatial detail), tie-breaking on smaller ratio_err. The band stops a high-tile-count grid from winning when a near-perfect-ratio, fewer-tile grid exists (e.g. 2×2 beating 2×1 on a 2:1 image and squashing it 2×).
5. Compose. Stretch the image to the chosen col × row canvas, prepend a tile_px × tile_px global overview thumbnail (recovers text split across tile seams), then crop into col × row tiles packed row-major. Tiles are encoded per --image-tile-mode: sequential (default, one-by-one), batched (one collapsed attention pass), or disabled (single tile).

Performance

Candidate (this PR) vs published @qvac/llm-llamacpp 0.31.0. Qwen3.5-0.8B, deterministic at temp 0 seed 42, image_max_tokens=4096. The disc-ocr set is 9 real getomni-ai OCR docs scaled aspect-preserved into the band where baseline falls back to dyn_size and candidate tiles.

Addon benchmark: disc-ocr run

All rows from the same run. Speed, lower is faster. Desktop reports mmproj vision-encode; mobile reports TTFT.

Platform · device	metric	baseline	candidate	Δ
linux · GPU	encode ms	211	92	−56%
macos · GPU	encode ms	1510	538	−64%
s25 · CPU	TTFT ms	36636	13101	−64%
s25 · GPU	TTFT ms	50740	19727	−61%
pixel9 · CPU	TTFT ms	112012	38523	−66%
pixel9 · GPU	TTFT ms	118463	41925	−65%

Accuracy, CER, lower is better.

Platform · device	baseline	candidate	Δ
linux · GPU	0.775	0.777	+0.002
macos · GPU	0.782	0.782	0.000
s25 · CPU	0.791	0.782	−0.009
s25 · GPU	0.787	0.785	−0.002
pixel9 · CPU	0.791	0.782	−0.009
pixel9 · GPU	0.781	0.800	+0.019

Raw fabric CLI on-device, per-image mean encode

Platform · backend	baseline encode	candidate encode	Δ
macOS · Metal	2824 ms	791 ms	−72%
S25 · OpenCL	33642 ms	3895 ms	−88%
Pixel 9 · Vulkan	283382 ms	30450 ms	−89%

Test plan

• mtmd-cli on Qwen3-VL with multi-tile images
• batched mode across backends — dedicated integration test added in QVAC-21396 test[api]: cover batched image_tile_mode across platforms qvac#3015 (qwen3-5-image-tile-mode-tokens: batched token-count must match sequential, or a backend mis-iterates ne[3]). CI confirmation still pending — the mobile leg that covers Metal / Vulkan (Android) / OpenCL (Adreno) hit a Device Farm scheduling flake and needs a re-run.
• server path with --image-max-tiles
• qvac consumer rollout (Phase A overlay validation) — PR QVAC-21396 feat: overlay-validate qwen3vl grid-select fabric build qvac#2962

Performance numbers above are sequential-mode only (baseline is sequential too), so they stand independent of the batched-mode verification.

[olyasir]

[security] image_max_tiles is unvalidated at this library boundary → potential OOM/DoS.

The [1,256] bound is enforced only in common/arg.cpp (the CLI/env path). But clip_context_params/mtmd_context_params are the public MTMD_API surface that native bindings populate directly (e.g. this monorepo's packages/llm-llamacpp). Here the override only tests != -1 and assigns straight into hparams.preproc_max_tiles. A large value then reaches fitting.reserve((size_t)(max_tiles * std::log(max_tiles))) in mtmd-image.cpp — an INT_MAX-ish value requests hundreds of GB and throws std::bad_alloc, and the per-image preprocess() call in mtmd.cpp's tokenize path has no surrounding try/catch, so the whole process can crash.

The correct pattern already exists in this same PR two functions up (the GGUF-read path clamps preproc_max_tiles to [1,256] with a warning). Please re-validate here too, ideally via a shared constant reused by both sites.

[yingying0906]

Fixed. The override now clamps to [1, CLIP_PREPROC_MAX_TILES_LIMIT] (256) at this boundary before assigning, with a warning, so a binding that populates clip_context_params directly can no longer drive the grid-fitting reserve into bad_alloc. Uses the same shared constant as the GGUF-read path.

[olyasir]

[correctness] This override (and the min-tiles clamp) is a silent no-op for InternVL.

InternVL builds hparams.image_res_candidates exactly once, inside load_hparams (set_internvl_dhr_res_candidates, called only from that switch case). clip_init applies this --image-max-tiles override after load_hparams, and nothing rebuilds the candidate list afterward. InternVL's tiling (mtmd_image_preprocessor_llava_uhd::get_slice_instructions) reads only image_res_candidates — it never reads preproc_min_tiles/preproc_max_tiles live. So for InternVL, mutating these fields here has zero effect on tiling, yet we still log preproc_max_tiles: N (custom value), and this min-tiles clamp is dead code (the "empty candidate set" it guards against can't occur, since the candidates are already fixed). This contradicts the flag's own help text.

(It works correctly for Qwen3VL, which reads preproc_max_tiles at preprocessor construction, post-override.)

Fix options: rebuild image_res_candidates after applying the override for candidate-list-based preprocessors, or scope the flag + docs to Qwen3VL only.

[yingying0906]

Fixed. Scoped the override to PROJECTOR_TYPE_QWEN3VL which is the only preprocessor that reads preproc_max_tiles live. Non-Qwen3VL projectors now get a "only supported for Qwen3VL; ignoring" warning instead of a silent no-op + misleading (custom value) log. Removed the min-tiles clamp entirely: it only guarded InternVL, whose candidate list is frozen in load_hparams, so it was dead code.

[olyasir]

[consistency] The [1,256] bound is a hardcoded literal duplicated across two sites, and one enforcement site is missing.

This is one of three independent hardcodings of 256: here (CLI), the GGUF-read validation in clip.cpp (preproc_max_tiles < 1 || > 256), and it is absent at the clip_init override site (ctx_vision->model.hparams.preproc_max_tiles = ctx_params.image_max_tiles;) which is the actual library boundary embedders hit. Suggest extracting a shared constexpr (e.g. IMAGE_MAX_TILES_LIMIT = 256) and using it at all three places so the invariant can't drift and the override path gets the bound for free.

[yingying0906]

The override site now enforces the bound via a shared constexpr CLIP_PREPROC_MAX_TILES_LIMIT in clip-impl.h, reused at both clip.cpp sites. Left the literal here on purpose: arg.cpp is in common/ (includes only common.h) and can't reach clip-impl.h without coupling the common lib to mtmd internals for one int. Since the safety-critical bound is now at the library boundary, this CLI check is just a friendly early-reject, a drift here can't cause the OOM.

[olyasir]

[consistency] This warning tells the user to --image-max-tiles for projectors where the flag is inert.

This block lives in a switch case shared by PROJECTOR_TYPE_QWEN2VL, QWEN25VL, and QWEN3VL, and the ternary above names "Qwen2VL"/"Qwen2.5VL". But per mtmd.cpp's preprocessor-selection switch, QWEN2VL/QWEN25VL use mtmd_image_preprocessor_dyn_size, which never reads preproc_max_tiles — tiling is Qwen3VL-only. So suggesting --image-max-tiles to Qwen2/2.5VL users is misleading (the flag does nothing there). Either scope this default/validation/warning block to PROJECTOR_TYPE_QWEN3VL, or drop the --image-max-tiles suggestion from the non-Qwen3VL branches.

[yingying0906]

The "set --image-max-tiles" suggestion is now gated to PROJECTOR_TYPE_QWEN3VL; Qwen2/2.5VL (dyn_size) get the same large-ViT warning without the flag suggestion, since the flag is inert for them.

[DmitryMalishev]

@yingying0906 Looks good to me overall. Couple corner case concerns worth to check and fix:

1. Flag silently dead for InternVL. --image-max-tiles is documented as generic ("multi-tile vision models") but has no effect on InternVL. InternVL bakes its resolution candidates from preproc_min/max_tiles into hparams.image_res_candidates during load_hparams (clip.cpp:1290, 2730–2748); the override in clip_init (clip.cpp:2767) mutates preproc_max_tiles afterwards but never rebuilds that candidate list, so preprocessing keeps using the old budget. Worse, the adjacent comment ("models with a min-tiles floor (e.g. InternVL) … clamp min so the invariant holds") reads as if InternVL were handled, giving reviewers false confidence. Either recompute the candidates after the override or scope the flag/docs to Qwen3-VL.

2. No API-side validation. Range validation exists only at the CLI (arg.cpp enforces 1–256). The mtmd_context_params.image_max_tiles / clip_context_params.image_max_tiles API path — which is what the QVAC addons actually use — applies any value other than -1 unchecked. 0 sets preproc_max_tiles = 0, which makes the qwen3vl grid loop never run and silently forces single-tile mode forever; a huge value allows unbounded tile counts (memory blow-up during preprocessing). The GGUF-read path got a [1,256] sanity check in this same PR — the override path should get the identical check in clip_init.

3. Two conflicting "defaults". The default flipped from batched to sequential, but only for callers going through mtmd_context_params_default(). The raw enum still has 0 = batched (clip.h: "0=batched, 1=sequential (default)"), so any consumer that zero-initializes the params struct silently gets batched — the unverified path from C1. A C API where "default" and "zero value" disagree is a long-term footgun; consider renumbering so 0 = sequential, or at minimum call this out loudly in mtmd.h.

[yingying0906]

Thanks for the comments and all three addressed.

(1) InternVL no-op + misleading comment — resolved in dfc03ffad (I think you saw an earlier diff state). The clip_init override is now scoped to PROJECTOR_TYPE_QWEN3VL, the only preprocessor that reads preproc_max_tiles live. Any other projector (InternVL, Qwen2/2.5VL) now logs "--image-max-tiles is only supported for Qwen3VL; ignoring for this model" and leaves hparams untouched — no more silent no-op mutation or misleading (custom value) log. The "min-tiles floor (e.g. InternVL)… clamp min" comment and its dead clamp are removed (InternVL's candidate list is frozen in load_hparams, so the clamp never applied).

(2) no API-side validation — resolved in the same commit. clip_init now re-validates at the library boundary via a shared CLIP_PREPROC_MAX_TILES_LIMIT (the same [1,256] bound as the GGUF-read path): 0 clamps to 1 with a warning (no more silent single-tile-forever), and a huge value clamps to 256 so it can't reach the grid-fitting reserve() and blow up. Bindings that populate clip_context_params directly get the bound for free.

(3) 0=batched vs sequential default — agreed it's a real footgun. For now I've documented it in clip.h and mtmd.h (5707f9b7c): the comments warn that 0 (batched) is the zero value but not the default, and that callers should init via mtmd_context_params_default() (sequential) or set the field explicitly. My concern with renumbering to 0=sequential is that "0"/"1"/"2" already shipped as a string API (parsed by consumers, e.g. the QVAC addon), so flipping it would silently change existing callers. Maybe worth raising the renumbering separately to decide if we want to take that break.

[gianni-cor]

[compatibility] image_max_tiles zero-initializes to an explicit override.

image_max_tiles uses -1 as the unset sentinel, but this is a new public struct field. Existing bindings/direct C API callers that zero-initialize clip_context_params/mtmd_context_params will pass 0 unless they are updated. This branch treats any value except -1 as an override, then clamps 0 to 1, so those callers silently force Qwen3VL to one max tile instead of using the GGUF/model default.

The CLI path is fine because common_params initializes image_max_tiles = -1, but public API consumers can regress in model quality: multi-tile preprocessing is effectively disabled for large/high-resolution images, especially hurting OCR/doc/screenshot cases where tiling preserves small local details. Consider treating 0 as unset at this library boundary, or adding/updating a public default initializer that bindings use before passing params into clip_init().

[yingying0906]

Fixed. image_max_tiles now treats only a positive value as an explicit override, so both -1 and 0 fall through to the GGUF/model default. A zero-initialized clip_context_params / mtmd_context_params (bindings / direct C API callers that never set the field) keeps the model default instead of being clamped to single-tile. Dropped the dead lower clamp, kept the upper [1,256] bound. Updated the field docs in clip.h and mtmd.h to note -1 or 0 = default, only positive overrides.