@samitouri / QOSamiQemu / commits / 27d14251b9

hw/display/cirrus_vga: Fix packed-24 color-expansion transparent copies

For the "color expansion" subtype of raster operations, the source pixel format is a monochrome bitmap, and the destination can be any of 8, 16, 24 or 32bpp. For these pattern operations, the GR2F register includes a field which specifies how much to skip at the start of each scanline. In the 8, 16 and 32 bit cases, this field is 3 bits and is a count of pixels to skip. We get this case right. However, for the 24 bit case, the field is 5 bits and is a count of destination bytes to skip. In commit ad81218e40e27 ("depth=24 write mask fix (Volker Ruppert)") in 2005, we updated the code to (attempt to) handle the 5-bit mask case. However, we don't do the right thing when the 5-bit mask indicates that we need to skip more than 8 bits of the input bitmap: we will right-shift the 0x80 constant completely off the right hand side, and will be off-by-one for all the source bitmap loads. Fix this by calculating the whole number of input bytes we need to skip and the residual number of bits. In the 8/16/32bpp case the bytes to skip is always zero. Cc: qemu-stable@nongnu.org Fixes: ad81218e40e27 ("depth=24 write mask fix (Volker Ruppert)") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Junjie Cao <junjie.cao@intel.com> Tested-by: Junjie Cao <junjie.cao@intel.com> Message-ID: <20260410183249.4046456-3-peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Peter Maydell committed Apr 10, 2026 at 19:32 UTC 27d14251b904e6dd60c1053a893b52e085f48a3a
1 file changed +27 -4
hw/display/cirrus_vga_rop2.h
+27 -4
@@ -108,12 +108,34 @@ glue(glue(glue(cirrus_colorexpand_transp_, ROP_NAME), _),DEPTH)
108 unsigned int col;
109 unsigned bitmask;
110 unsigned index;
111 +
112 + /*
113 + * Raster ops where the source is a monochrome bitmap with
114 + * color expansion to 8/16/24/32bpp destination.
115 + */
116 +
117 #if DEPTH == 24
118 + /*
119 + * For packed-24 modes, GR2F bits [4:0] are a count of destination
120 + * bytes to be suppressed for each scanline, which we keep in
121 + * dstskipleft. We want to track the number of whole bytes
122 + * to skip in the source (always either 0 or 1) and the number
123 + * of bits within the byte to skip.
124 + */
125 int dstskipleft = s->vga.gr[0x2f] & 0x1f;
113 - int srcskipleft = dstskipleft / 3;
126 + int srcskipleftbits = (dstskipleft / 3) & 0x7;
127 + int srcskipleftbytes = (dstskipleft / 3) >> 3;
128 #else
115 - int srcskipleft = s->vga.gr[0x2f] & 0x07;
116 - int dstskipleft = srcskipleft * (DEPTH / 8);
129 + /*
130 + * In all other modes, GR2F bits [2:0] are a count of how many
131 + * destination pixels to suppress for each scanline, which is our
132 + * srcskipleftbits. We get dstskipleft, the number of bytes to
133 + * skip, by multiplying this by the bytes-per-pixel. In these
134 + * modes we never need to skip an entire source byte.
135 + */
136 + int srcskipleftbits = s->vga.gr[0x2f] & 0x07;
137 + int srcskipleftbytes = 0;
138 + int dstskipleft = srcskipleftbits * (DEPTH / 8);
139 #endif
140
141 if (s->cirrus_blt_modeext & CIRRUS_BLTMODEEXT_COLOREXPINV) {
@@ -125,7 +147,8 @@ glue(glue(glue(cirrus_colorexpand_transp_, ROP_NAME), _),DEPTH)
147 }
148
149 for(y = 0; y < bltheight; y++) {
128 - bitmask = 0x80 >> srcskipleft;
150 + bitmask = 0x80 >> srcskipleftbits;
151 + srcaddr += srcskipleftbytes;
152 bits = cirrus_src(s, srcaddr++) ^ bits_xor;
153 addr = dstaddr + dstskipleft;
154 for (x = dstskipleft; x < bltwidth; x += (DEPTH / 8)) {