From 7acdbb1f191d869bad8c5da9d4a2121defa340af Mon Sep 17 00:00:00 2001 From: BlackFoil <127078112+BlackFoil@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:11:19 +0900 Subject: [PATCH] mtmd: fix LFM2 image tiling threshold (#27057) * mtmd: fix LFM2 image tiling threshold * refactor testing * fix * fix on windows --------- Co-authored-by: Xuan Son Nguyen --- tests/CMakeLists.txt | 3 ++ tests/test-mtmd-impl.cpp | 88 +++++++++++++++++++++++++++++++++++++++ tools/mtmd/CMakeLists.txt | 3 ++ tools/mtmd/clip-impl.h | 4 ++ tools/mtmd/mtmd-image.cpp | 21 +++++++++- tools/mtmd/mtmd-image.h | 2 + 6 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 tests/test-mtmd-impl.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 08c6f5a47..3ee51b519 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -310,6 +310,9 @@ llama_build_and_test(test-mtmd-c-api.c) target_link_libraries(${LLAMA_TEST_NAME} PRIVATE mtmd) unset(LLAMA_TEST_NAME) +llama_build_and_test(test-mtmd-impl.cpp) +target_link_libraries(test-mtmd-impl PRIVATE mtmd) + # GGUF model data fetcher library for tests that need real model metadata # Only compile when cpp-httplib has SSL support (CPPHTTPLIB_OPENSSL_SUPPORT) if (TARGET cpp-httplib) diff --git a/tests/test-mtmd-impl.cpp b/tests/test-mtmd-impl.cpp new file mode 100644 index 000000000..41cb53237 --- /dev/null +++ b/tests/test-mtmd-impl.cpp @@ -0,0 +1,88 @@ +#include "testing.h" + +#include "mtmd-image.h" + +#include +#include +#include +#include + +// this test file contains: +// 1. test cases for mtmd helpers +// 2. test cases for internal mtmd components +// internal headers can be included here + +struct test_registry { + using fn_t = void (*)(testing &); + + struct entry { + std::string name; + fn_t fn; + }; + + static std::vector & all() { + static std::vector entries; + return entries; + } + + test_registry(const char * name, fn_t fn) { + all().push_back({ name, fn }); + } +}; + +#define MAKE_TEST(name) \ + static void name(testing & t); \ + static const test_registry test_registry_ ## name(#name, &name); \ + static void name(testing & t) + + +// +// mtmd_image +// + +MAKE_TEST(test_image_preprocessor_lfm2) { + clip_hparams hparams; + hparams.patch_size = 16; + hparams.n_merge = 2; + hparams.set_limit_image_tokens(64, 256); + + // { image size, expected tiling } + const std::vector> cases = { + { { 704, 704 }, false }, + // 720 / (patch_size * n_merge) is exactly 22.5, so this only matches HF + // if round_by_factor rounds half to even (22) instead of away from zero (23) + { { 720, 720 }, false }, + { { 736, 736 }, true }, + { { 1024, 977 }, true }, + { { 1056, 384 }, false }, + }; + + for (const auto & [size, expected] : cases) { + const bool actual = mtmd_image_preprocessor_lfm2::should_tile(hparams, size); + + t.assert_equal( + "tiling for " + std::to_string(size.width) + "x" + std::to_string(size.height), + std::string(expected ? "tiled" : "single"), + std::string(actual ? "tiled" : "single")); + } +} + +// +// main +// + +int main(int argc, char ** argv) { + testing t(std::cout); + t.verbose = true; + + // usage: test-mtmd-impl [filter_regex] + for (int i = 1; i < argc; i++) { + t.set_filter(argv[i]); + } + + for (const auto & e : test_registry::all()) { + t.test(e.name, e.fn); + } + + return t.summary(); +} diff --git a/tools/mtmd/CMakeLists.txt b/tools/mtmd/CMakeLists.txt index 95aa853ea..aa97c768a 100644 --- a/tools/mtmd/CMakeLists.txt +++ b/tools/mtmd/CMakeLists.txt @@ -90,6 +90,9 @@ if (BUILD_SHARED_LIBS) set_target_properties (mtmd PROPERTIES POSITION_INDEPENDENT_CODE ON) target_compile_definitions(mtmd PRIVATE LLAMA_BUILD) target_compile_definitions(mtmd PUBLIC LLAMA_SHARED) + + # export all symbols so that internal components can be tested by test-mtmd-impl + set_target_properties (mtmd PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON) endif() set(MTMD_PUBLIC_HEADERS diff --git a/tools/mtmd/clip-impl.h b/tools/mtmd/clip-impl.h index 2c9ea499c..ea8549776 100644 --- a/tools/mtmd/clip-impl.h +++ b/tools/mtmd/clip-impl.h @@ -858,6 +858,9 @@ static std::ifstream open_ifstream_binary(const std::string & fname) { } #endif +// in test-mtmd-impl, we include woth common.h and this file, and these functions are duplicated +// this is a quick fix to avoid compilation errors +#ifndef DIRECTORY_SEPARATOR static std::string string_format(const char * fmt, ...) { va_list ap; va_list ap2; @@ -915,6 +918,7 @@ inline bool string_ends_with(std::string_view str, std::string_view suffix) { return str.size() >= suffix.size() && str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0; } +#endif // // gguf utils diff --git a/tools/mtmd/mtmd-image.cpp b/tools/mtmd/mtmd-image.cpp index 769b6efe6..0d9db4f62 100644 --- a/tools/mtmd/mtmd-image.cpp +++ b/tools/mtmd/mtmd-image.cpp @@ -1013,14 +1013,31 @@ mtmd_image_preproc_out mtmd_image_preprocessor_lfm2::preprocess(const clip_image return output; } +bool mtmd_image_preprocessor_lfm2::should_tile( + const clip_hparams & hparams, + const clip_image_size & original_size) { + const int align_size = hparams.patch_size * hparams.n_merge; + + const auto round_by_factor = [align_size](float x) { + // see https://github.com/ggml-org/llama.cpp/pull/27057#discussion_r3796264887 + return static_cast(std::nearbyint(static_cast(x) / align_size)) * align_size; + }; + + const int h_bar = std::max(hparams.patch_size, round_by_factor(original_size.height)); + const int w_bar = std::max(hparams.patch_size, round_by_factor(original_size.width)); + + return static_cast(h_bar) * static_cast(w_bar) > + static_cast(hparams.image_max_pixels) * max_pixels_tolerance; +} + mtmd_image_preprocessor_llava_uhd::slice_instructions mtmd_image_preprocessor_lfm2::get_slice_instructions(const clip_image_size & original_size) { mtmd_image_preprocessor_llava_uhd::slice_instructions inst; const int align_size = hparams.patch_size * hparams.n_merge; inst.overview_size = img_tool::calc_size_preserved_ratio( original_size, { align_size, hparams.image_min_pixels, hparams.image_max_pixels, 0 }); - // tile if either dimension exceeds tile_size with tolerance - const bool needs_tiling = original_size.width > tile_size * max_pixels_tolerance || original_size.height > tile_size * max_pixels_tolerance; + + const bool needs_tiling = should_tile(hparams, original_size); if (!needs_tiling) { inst.refined_size = clip_image_size{0, 0}; diff --git a/tools/mtmd/mtmd-image.h b/tools/mtmd/mtmd-image.h index 40dfea7eb..732e27379 100644 --- a/tools/mtmd/mtmd-image.h +++ b/tools/mtmd/mtmd-image.h @@ -148,6 +148,8 @@ struct mtmd_image_preprocessor_lfm2 : mtmd_image_preprocessor_llava_uhd { mtmd_image_preproc_out preprocess(const clip_image_u8 & img) override; slice_instructions get_slice_instructions(const clip_image_size & original_size) override; + static bool should_tile(const clip_hparams & hparams, const clip_image_size & original_size); + private: clip_image_size find_closest_aspect_ratio( float aspect_ratio,