Download raw body.
rasops32: fix fonts on byte swapped framebuffers
Hello,
I recently sent an email with a proposed patch to improve the Radeon
console on sparc64. After removing much of the special case sparc64
code, the framebuffer finally shows a usable console. That change was
not enough on its own, although I still recommend accepting it as is.
The console text is still blurred.
This patch fixes font rendering on 32 bit byte swapped framebuffers.
The function uses a union to write two 32 bit pixels together as one
64 bit value. With sparc64's byte swapped access, that reverses the
positions of the two pixels. Writing one pixel at a time avoids this
problem, since the existing colour handling already accounts for byte
order within each pixel. I have preserved the optimization by putting
the paired pixels in the correct order before writing them. There is
also a path that draws one pixel at a time when the optimized path
cannot be used. This is considerably more complicated than the previous
patch and needs review.
Radeon DRM is the only case I have confirmed. Its KMS setup changes
the console from the firmware's 8 bit framebuffer to 32 bit, exposing
the problem. The other sparc64 drivers using byte
swap, machfb, vgafb and raptor, retain the firmware's console depth,
with 8 bit as the default. When firmware sets up an 8 bit console,
they stay on the 8 bit renderer and avoid this problem. My onboard
Rage XL is running that way, and I suspect that explains why this
has not been obvious on the other cards. If those drivers use the
same byte swapped access at 32 bit, I would expect the same problem,
and this patch should address it.
This is shared code used on alpha, amd64, arm64, armv7, i386,
loongson, macppc, powerpc64, riscv64 and sparc64. In the source I
checked, only sparc64 sets RI_BSWAP for this renderer. Neither macppc
nor powerpc64 sets it on this path, despite being big endian. The
optimized loop bodies remain unchanged, but the alignment, font
padding and fallback changes can also affect the other platforms.
I have only tested this on my Sun Blade, so testing on other systems
would be welcome.
Here are two captures showing the framebuffer before and after this
change, both after the previous Radeon console patch. I used the same
console font test for both. The after image also matches a fresh
capture with this final version.
Before:
https://files.catbox.moe/tb0t7r.png
After:
https://files.catbox.moe/awfmik.png
There are other changes that need to be made, but this is the next
logical step. I can confirm it solves the blurred text problem on my
machine. I commented the logic as best I could without being too
verbose, but this is a fiendishly complex function. Please review it
and suggest any adjustments needed.
Patch follows inline.
diff /usr/src
path + /usr/src
commit - 9a41316fee4f8507967e487ca8ed12429c06d449
blob - ec189be00b1232e0e25f7db1ee2c4416b74b82ce
file + sys/dev/rasops/rasops32.c
--- sys/dev/rasops/rasops32.c
+++ sys/dev/rasops/rasops32.c
@@ -60,6 +60,10 @@ rasops32_init(struct rasops_info *ri)
/*
* Paint a single character.
+ *
+ * A font bitmap uses one bit per pixel; the framebuffer uses a 32 bit
+ * color for each pixel. A zero bit selects the background color, and
+ * a one bit selects the foreground color.
*/
int
rasops32_putchar(void *cookie, int row, int col, u_int uc, uint32_t attr)
@@ -68,6 +72,8 @@ rasops32_putchar(void *cookie, int row, int col, u_int
uint32_t fb, clr[2];
struct rasops_info *ri;
int64_t *rp;
+ /* dp is the destination pixel pointer; each pixel is four bytes. */
+ int *dp;
union {
int64_t q[4];
int32_t d[4][2];
@@ -85,7 +91,8 @@ rasops32_putchar(void *cookie, int row, int col, u_int
return 0;
#endif
- rp = (int64_t *)(ri->ri_bits + row*ri->ri_yscale + col*ri->ri_xscale);
+ dp = (int *)(ri->ri_bits + row*ri->ri_yscale +
+ col*ri->ri_xscale);
height = ri->ri_font->fontheight;
width = ri->ri_font->fontwidth;
@@ -102,15 +109,39 @@ rasops32_putchar(void *cookie, int row, int col, u_int
while (height--) {
/* the general, pixel-at-a-time case is fast enough */
for (cnt = 0; cnt < width; cnt++)
- ((int *)rp)[cnt] = b;
- rp += step;
+ dp[cnt] = b;
+ dp += ri->ri_stride / 4;
}
} else {
uc -= ri->ri_font->firstchar;
fr = (u_char *)ri->ri_font->data + uc * ri->ri_fontscale;
fs = ri->ri_font->stride;
- /* double-pixel special cases for the common widths */
+ /*
+ * Writing two pixels together needs an eight byte boundary.
+ * uintptr_t lets us check the alignment address as an integer.
+ */
+ if ((uintptr_t)dp % 8 != 0)
+ goto pixel_at_a_time;
+
+ /* Distance between rows must be a multiple of eight bytes. */
+ if (ri->ri_stride % 8 != 0)
+ goto pixel_at_a_time;
+
+ /* The fast cases expect no padding bytes between font rows. */
+ if (fs != (width + 7) / 8)
+ goto pixel_at_a_time;
+ rp = (int64_t *)dp;
+
+ /*
+ * Writing two pixels together to a byte swapped framebuffer
+ * reverses their order. Reverse each mixed pair here to keep
+ * the correct pixel order.
+ */
+ if (ri->ri_flg & RI_BSWAP) {
+ u.d[1][0] = f; u.d[1][1] = b;
+ u.d[2][0] = b; u.d[2][1] = f;
+ }
switch (width) {
case 6:
while (height--) {
@@ -194,20 +225,37 @@ rasops32_putchar(void *cookie, int row, int col, u_int
}
break;
- default: /* there is a 5x8 font, so fall back to per-pixel */
+ default:
+ pixel_at_a_time:
clr[0] = b;
clr[1] = f;
+ /*
+ * This renderer accepts widths from 4 to 32 pixels.
+ * Pack each font row in fb, reading only the bytes
+ * needed for the width. Put the leftmost pixel at bit
+ * 31. Use it to select the pixel color, then
+ * shift the next font bit into place.
+ *
+ * Write each 32 bit pixel individually to avoid the
+ * pixel order issue with the union's 64 bit pixel pairs
+ * on byte swapped framebuffers.
+ */
while (height--) {
- fb = fr[3] | (fr[2] << 8) | (fr[1] << 16) |
- (fr[0] << 24);
+ fb = (uint32_t)fr[0] << 24;
+ if (width > 8)
+ fb |= (uint32_t)fr[1] << 16;
+ if (width > 16)
+ fb |= (uint32_t)fr[2] << 8;
+ if (width > 24)
+ fb |= (uint32_t)fr[3];
fr += fs;
for (cnt = 0; cnt < width; cnt++) {
- ((int *)rp)[cnt] = clr[fb >> 31];
+ dp[cnt] = clr[fb >> 31];
fb <<= 1;
}
- rp += step;
+ dp += ri->ri_stride / 4;
}
break;
}
@@ -215,9 +263,16 @@ rasops32_putchar(void *cookie, int row, int col, u_int
/* Do underline a pixel at a time */
if ((attr & WSATTR_UNDERLINE) != 0) {
- rp -= step;
+ /*
+ * The drawing paths have advanced their pointers. Find the
+ * character's bottom row again for the foreground underline.
+ */
+ dp = (int *)(ri->ri_bits + row*ri->ri_yscale);
+ dp += col * ri->ri_xscale / 4;
+
+ dp += (ri->ri_font->fontheight - 1) * ri->ri_stride / 4;
for (cnt = 0; cnt < width; cnt++)
- ((int *)rp)[cnt] = f;
+ dp[cnt] = f;
}
return 0;
rasops32: fix fonts on byte swapped framebuffers