From: Crystal Kolipe <kolipe.c@exoticsilicon.com>
Subject: Re: rasops32: fix fonts on byte swapped framebuffers
To: Ted Bullock <tbullock@comlore.com>
Cc: tech@openbsd.org
Date: Thu, 08 Oct 2026 08:23:06 -0000

On Thu, Oct 08, 2026 at 02:52:09AM +0000, Ted Bullock wrote:
> Indeed, the bug fix did grow beyond strictly handling the byte swap.
> The other changes were intended to keep the pixel processing correct
> when the framebuffer was aligned to four bytes rather than eight, or
> when the row stride was not a multiple of eight bytes. Those are
> separate correctness issues.

Have you checked that those conditions can actually occur in practice?

For example, the font width and stride values come directly from the font
files in sys/dev/wsfont/*.h.  All of the supplied fonts fulfill the
requirements, so somebody would have to deliberately install a non-compliant
console font to trigger the problem.  This could easily be checked for, (and
even fixed), at font loading rather than for every single character printed.

> Included below is the byte swap fix on its own. I've built and tested
> it, and it works for me. The framebuffer capture matches the result
> from the larger patch exactly.

Yes, that code looks correct in principle.

The the use of a union for type punning in the existing code is technically
'unspecified behaviour' in C, so it's basically guaranteed to return a uint64
value, (I.E. it's guaranteed _not_ to be a trap value), but that's about it.

Having said that, the observed behaviour is what I would expect, so I'm not
sure if there would be any consensus to remove that logic in favour of just
bit shifting the two uint32s into a uint64, which would be a non-ambiguous way
to achieve the desired result.

> The preprocessing suggestion is interesting, along with some other
> possible performance changes. I think those are worth exploring
> separately.

Compared to the now reduced patch which just does the byte-swapping,
pre-processing the font data likely won't gain any measurable improvement.

To be honest, micro-optimisations in rasops for performance are only worth it
if they don't significantly increase the complexity of the code.  Look at the
putchar() implementation in rasops24.c compared to rasops32.c for example.

The point is to avoid regressions caused by adding too much code to putchar(),
as that runs every single time a character is printed to the framebuffer.

(And the amount of display hardware that actually runs in 24-bit mode and
 cannot run in 32-bit is now vanishingly small.  So those performance hacks in
 rasops24.c are probably mostly pointless because a simple unoptimised blit to
 a 32-bit aligned character cell is likely going to offset any gain from an
 optimised unaligned 24-bit write.)