> >>> if (pitch < 0) { > >>> int64_t min = addr > >>> - + ((int64_t)s->cirrus_blt_height-1) * pitch; > >>> + + ((int64_t)s->cirrus_blt_height-1) * pitch > >>> + - s->cirrus_blt_width; > >>> int32_t max = addr > >>> + s->cirrus_blt_width; > >>> if (min < 0 || max > s->vga.vram_size) { > >>> > >> > >> I believe this is incorrect. In this case (AFAIR), "addr" points to the > >> left-most pixel (= lowest address) of the bottom line (= highest > >> address). > > > > If I read the code correctly it is backwards *both* x and y axis, so > > addr is the right-most pixel of the bottom line. > > What is "max" then? If "addr" is the right-most pixel of the bottom > line, then "max" has the highest address just past the rectangle, and > then adding anything non-negative to it makes no sense.
That is (with the patch applied) inconsistent indeed. We must either subtract s->cirrus_blt_width from min (addr == right-most), or add it to max (addr == left-most), but certainly not both. > ... Really as I remember it from the downstream review, the pitch is > negative (bottom-up), but the horizontal direction remains left to right. Looking at cirrus_vga_rop.h I see: - cirrus_bitblt_rop_fwd_*() increment src and dst while walking the scanline, and - cirrus_bitblt_rop_bkwd_*() decrement src and dst ... I still think x axis goes backwards too and therefore addr is the right-most pixel. cheers, Gerd