Skip to content

[Mirror] https://github.com/ggml-org/llama.cpp/pull/21711#97

Closed
ngxson wants to merge 4 commits intongxson:masterfrom
mzsergiu:master
Closed

[Mirror] https://github.com/ggml-org/llama.cpp/pull/21711#97
ngxson wants to merge 4 commits intongxson:masterfrom
mzsergiu:master

Conversation

@ngxson
Copy link
Copy Markdown
Owner

@ngxson ngxson commented Apr 10, 2026

Mirror ggml-org#21711

@coderabbitai review using mirror preset

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Enhanced image resizing algorithm to properly handle edge cases, boundary conditions, and degenerate inputs. The implementation now provides more accurate interpolation results and prevents errors when processing images of various dimensions, delivering reliable and consistent results across different input scenarios.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 10, 2026

📝 Walkthrough

Walkthrough

The img_tool::resize_bilinear function in tools/mtmd/mtmd-image.cpp was updated to gracefully handle degenerate inputs and improve bilinear interpolation. Changes include early returns for zero-sized sources, clamping output dimensions to minimum 1, and revising coordinate mapping from (src-1)/target to (src-1)/(target-1) with refined endpoint clamping.

Changes

Cohort / File(s) Summary
Bilinear Resize Logic
tools/mtmd/mtmd-image.cpp
Updated resize_bilinear to handle degenerate inputs (zero-sized sources) without assertions. Changed coordinate mapping formula and interpolation factor computation for improved numerical accuracy. Added clamping of output dimensions to minimum 1.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops through pixels bright,
Resizing images with interpolated might,
No degenerate inputs shall cause a crash,
Now smooth bilinear magic in a flash! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. It lacks an Overview section explaining the purpose of the changes, additional context about why this mirror is needed, and does not include the required Requirements checklist section from the template. Add a comprehensive Overview section explaining what changes are being mirrored and why. Include the complete Requirements section with checkboxes for contributing guidelines agreement and AI usage disclosure.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly indicates this is a mirror of an upstream PR with a specific reference, which accurately reflects the PR's purpose as a mirror PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tools/mtmd/mtmd-image.cpp (1)

201-233: Add a small regression test matrix for edge dimensions.

Please add coverage for 0xN, Nx0, 1x1, 1xN, and Nx1 through resize_bilinear to lock this fix in.

I can draft a compact table-driven test set if you want.
Based on learnings: For mirrored PRs (those with [Mirror] in title or mirroring upstream), ngxson wants AI-assisted code review with these preferences: check the upstream PR URL for description, be nit-picky about obvious mistakes, don't tag contributors directly (inform ngxson instead), and alert about suspicious AI-generated code comments like "CRITICAL: ..." or "You are correct here...".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/mtmd/mtmd-image.cpp` around lines 201 - 233, Add a compact table-driven
regression test for resize_bilinear covering edge cases 0xN, Nx0, 1x1, 1xN and
Nx1 to lock the fix: create tests that call resize_bilinear (or the public
wrapper that uses the shown implementation) with source images and target
dimensions for each case, assert dst.nx/dst.ny and dst.buf.size() (and pixel
values where meaningful) behave as expected (empty buffer for zero-dimension
targets, correct single-pixel copy for 1x1, correct row/column replication for
1xN/Nx1), and include cases where src.nx or src.ny == 0 to exercise the
early-return path that clears dst.buf; add these table entries into the existing
test suite for mtmd-image (grouped with other bilinear tests) so they run
automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tools/mtmd/mtmd-image.cpp`:
- Around line 201-233: Add a compact table-driven regression test for
resize_bilinear covering edge cases 0xN, Nx0, 1x1, 1xN and Nx1 to lock the fix:
create tests that call resize_bilinear (or the public wrapper that uses the
shown implementation) with source images and target dimensions for each case,
assert dst.nx/dst.ny and dst.buf.size() (and pixel values where meaningful)
behave as expected (empty buffer for zero-dimension targets, correct
single-pixel copy for 1x1, correct row/column replication for 1xN/Nx1), and
include cases where src.nx or src.ny == 0 to exercise the early-return path that
clears dst.buf; add these table entries into the existing test suite for
mtmd-image (grouped with other bilinear tests) so they run automatically.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7b37cc60-4e4a-4d41-bb18-1e2d0cb356cc

📥 Commits

Reviewing files that changed from the base of the PR and between 0893f50 and 266a771.

📒 Files selected for processing (1)
  • tools/mtmd/mtmd-image.cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants