>> The src also needs to be offset if clipped on the top and/or
>> left sides to ensure that src data is read correctly and appears
>> clipped when drawn rather than shifted.
>
> This is true for top to bottom left to right but what happens for other
> direction blits?
>
>> + /*
>> + * The src must be offset if clipping is applied to the dst.
>> + * This is so that when the source is blit to a dst clipped
>> + * on the top or left the src image is not shifted into the
>> + * clipped region but actually clipped.
>> + */
>> + vis_src.x = ctx->src.x + (vis_dst.x - ctx->dst.x);
>> + vis_src.y = ctx->src.y + (vis_dst.y - ctx->dst.y);
>> + vis_src.width = vis_dst.width;
>> + vis_src.height = vis_dst.height;
>
> These may also need to be calculated differently.
>
ctx->dst.x and ctx->dst.y are normalized to the top left corner of the
blit in setup_2d_blt_ctx (lines 102-105). So this is able to
do the calculations uniformly for all blit direction combinations.
>> + ctx->scissor.width = s->regs.sc_right - s->regs.sc_left + 1;
>> + ctx->scissor.height = s->regs.sc_bottom - s->regs.sc_top + 1;
>> + ctx->scissor.x = s->regs.sc_left;
>> + ctx->scissor.y = s->regs.sc_top;
>
> Does this need to be computed as src and dst coordinates when not top to
> bottom left to right?
>
The scissor rectangle is always in screen coordinates. It isn't affected
by blit direction at all.
Both good questions though. I was a little bit nervous I had missed
something here. So I went ahead and wrote tests that cover clipping
on all sides (including combinations) for both memory and host_data blits.
All tests pass (match framebuffer dumps from r128 hardware). I'll clean
them up and attach an updated test link to v9.
>> DPRINTF("pixman_blt(%p, %p, %d, %d, %d, %d, %d, %d, %d, %d, %d,
>> %d)\n",
>> ctx->src_bits, ctx->dst_bits, src_stride_words,
>> - dst_stride_words, ctx->bpp, ctx->bpp, ctx->src.x,
>> ctx->src.y,
>> - ctx->dst.x, ctx->dst.y, ctx->dst.width, ctx->dst.height);
>> + dst_stride_words, ctx->bpp, ctx->bpp, vis_src.x, vis_src.y,
>> + vis_dst.x, vis_dst.y, vis_dst.width, vis_dst.height);
>
> Maybe log visible rect in addition of src and dst not replacing it here so
> we can debug if clipping was correctly done.
>
> If you don't want to consider different directions than top to bottom left
> to right that's OK but then we probably need a check and a LOG_UNIMP in
> that case and keep using unclipped dst and src (just copying to vis rects
> I guess) in that case.
>
As written right now this logs exactly what is being passed into
pixman_blt(). Definitely agree it might be useful to see the raw src and
dst though too. Do you see it as a second log line before/after this
one?
