Skip to content

Fix label line-height in image-annotations skill (getbbox bottom-top)#2266

Open
drognanar wants to merge 1 commit into
github:mainfrom
drognanar:fix/image-annotations-getbbox-line-height
Open

Fix label line-height in image-annotations skill (getbbox bottom-top)#2266
drognanar wants to merge 1 commit into
github:mainfrom
drognanar:fix/image-annotations-getbbox-line-height

Conversation

@drognanar

Copy link
Copy Markdown

Pull Request Checklist

  • I have read and followed the CONTRIBUTING.md guidelines.
  • I have read and followed the Guidance for submissions involving paid services. (N/A — no paid services.)
  • My contribution updates an existing skill file.
  • The file follows the required naming convention.
  • The content is clearly structured and follows the example format.
  • I have tested my change (see Validation below).
  • I have run and verified README.md is up to date. (No README change — this is a code-logic fix inside an existing skill; no frontmatter/titles changed.)
  • I am targeting the main branch for this pull request.

Description

Fixes a small line-height bug in the annotate.py module embedded in skills/image-annotations/SKILL.md.

ImageFont.getbbox() returns (left, top, right, bottom). Text height must therefore be bottom - top = bbox[3] - bbox[1]. The automatic label-placement code computed it as bbox[3] - bbox[0] (bottom - left), which mixes an x-coordinate into a height:

# Before
line_h = font.getbbox('Ay')[3] - font.getbbox('Ay')[0]

# After
bbox = font.getbbox('Ay')
line_h = bbox[3] - bbox[1]

Why it matters: line_h feeds the label-box height (th), so the black label background is sized slightly wrong. The error equals the glyph's top bearing — empirically 0–4px depending on font (≈4px Consolas, ≈2px Pillow default, ≈1px Ink Free @32). It's most visible on multi-line labels where the error accumulates per line.

The debug heatmap labeling a few lines down in the same module already uses the correct bbox[3] - bbox[1], so this simply brings label placement in line with the rest of the file.

Found while vendoring this skill into another repo.


Type of Contribution

  • Update to existing instruction, prompt, agent, plugin, skill, workflow, or canvas extension.

Additional Notes

Validation:

  • Extracted the module code block and confirmed it compiles (python -m py_compile).
  • Ran annotate_image() end-to-end on a 2326×1486 screenshot with both single-line and multi-line labels — output renders correctly with correctly-sized label backgrounds.
  • Diff is 1 line changed (2 insertions, 1 deletion).

font.getbbox() returns (left, top, right, bottom), so text line height must
be bottom - top (bbox[3] - bbox[1]). The label placement code used
bbox[3] - bbox[0] (bottom - left), mixing an x-coordinate into a height and
producing a slightly wrong label-box height (empirically off by 0-4px
depending on font). The debug heatmap labeling in the same module already
uses the correct bbox[3] - bbox[1], so this brings placement in line.
@drognanar drognanar requested a review from aaronpowell as a code owner July 10, 2026 09:05
Copilot AI review requested due to automatic review settings July 10, 2026 09:05
@github-actions github-actions Bot added the skills PR touches skills label Jul 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔒 PR Risk Scan Results

Scanned 1 changed file(s).

Severity Count
🔴 High 0
🟠 Medium 0
ℹ️ Info 0

✅ No matching risk patterns were detected in changed files.

This is an automated soft-gate report. Findings indicate review targets and do not block merge by themselves.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a text measurement bug in the embedded annotate.py code within the image-annotations skill, ensuring label background sizing uses the correct font bounding-box height calculation.

Changes:

  • Corrects line-height computation from bottom - left to bottom - top by using bbox[3] - bbox[1].
  • Stores the 'Ay' bbox once before computing line_h, aligning this code path with other bbox usage in the same module.

@github-actions github-actions Bot added the skill-check-error Skill validator reported errors label Jul 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Vally Lint Results

⛔ Findings need attention

Scope Checked
Skills 1
Agents 0
Total 1
Severity Count
❌ Errors 1
⚠️ Warnings 0
ℹ️ Advisories 0

Summary

Level Finding
image-annotations (1/2 checks passed, 1 failed)
Full linter output
### Linting skills/image-annotations
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: 'commander@15.0.0',
npm warn EBADENGINE   required: { node: '>=22.12.0' },
npm warn EBADENGINE   current: { node: 'v20.20.2', npm: '10.8.2' }
npm warn EBADENGINE }
npm warn deprecated prebuild-install@7.1.3: No longer maintained. Please contact the author of the relevant native addon; alternatives are available.
❌ image-annotations (1/2 checks passed, 1 failed)
    ✗ [spec-compliance] 1 of 1 skill(s) have spec violations.
        ✗ spec-compliance: Spec checks failed.
            ✗ File length (602 lines) exceeds limit (500).
    ✓ [valid-refs] All file references across 1 skill(s) are valid.
        ✓ valid-refs: All file references resolve to existing files within the skill directory.

1 skill(s) linted, 1 failed

Note: Vally lint returned a non-zero exit code. Please review the findings above before merge.

@nohwnd

nohwnd commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

LGTM, but I have no power here.

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

Labels

skill-check-error Skill validator reported errors skills PR touches skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants