From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: lib/fonts: Refactor glyph-rotation helpers
Date: Sun, 12 Apr 2026 13:55:03 +1000
Message-ID: <review-patch7-20260407092555.58816-8-tzimmermann@suse.de>
In-Reply-To: <20260407092555.58816-8-tzimmermann@suse.de>
References: <20260407092555.58816-1-tzimmermann@suse.de> <20260407092555.58816-8-tzimmermann@suse.de>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Verdict: BUG in `__font_glyph_rotate_180` -- incorrect output coordinate for non-byte-aligned widths.**

This patch renames variables and drops `inline`, which is all fine. However, it introduces a bug in the 180-degree rotation.

In the **original** `rotate_ud()`, `width` was overwritten to `(width + 7) & ~7` (the bit pitch), and the output x-coordinate was computed as:

```c
width = (width + 7) & ~7;  // width is now bit_pitch
...
pattern_set_bit(width - (1 + j + shift), height - (1 + i), width, out);
```

Algebraically: `output_x = bit_pitch - 1 - j - shift`. Since `bit_pitch = width_orig + shift`, this simplifies to `output_x = width_orig - 1 - j`. The shift cancels out.

In the **new** `__font_glyph_rotate_180()`, the parameter `width` is no longer overwritten, but the formula still includes the shift:

```c
unsigned int bit_pitch = font_glyph_bit_pitch(width);
...
font_glyph_set_bit(out, width - (1 + x + shift), height - (1 + y), bit_pitch);
```

This computes `output_x = width - 1 - x - shift`, which is **wrong** -- it should be `width - 1 - x` (without the shift), or equivalently `bit_pitch - 1 - x - shift` (preserving the original formula with `bit_pitch`).

For any font width that isn't a multiple of 8 (e.g., 6, 7, 10, 12, 14...), the 180-degree rotation will place bits at incorrect positions. For example, with width=10, bit (0,0) maps to output position 3 instead of 9.

The 90-degree and 270-degree rotations are correct in this patch because they consistently use `out_bit_pitch` / `bit_pitch` (the rounded-up values) rather than the original width/height.

**Fix**: Change:
```c
font_glyph_set_bit(out, width - (1 + x + shift), height - (1 + y), bit_pitch);
```
to:
```c
font_glyph_set_bit(out, bit_pitch - (1 + x + shift), height - (1 + y), bit_pitch);
```

This wasn't caught in testing because the author tested with bochs+QEMU, which likely uses the default 8x16 font (width=8, shift=0).

---

---
Generated by Claude Code Patch Reviewer