[Pixman] [PATCH] MIPS: DSPr2: Fix bug in over_n_8888_8888_ca/over_n_8888_0565_ca routines

[Nemanja Lukic]

I'll add this explanation to the commit message.

On your remark, about splitting this into two commits, I really like that
approach, 
but since I didn't do it this time (I don't have this intermediate result,
where only 
bug is fixed, without code refactoring), I'll leave that for future bug
fixes. 

Thanks,
Nemanja Lukic

-----Original Message-----
From: Siarhei Siamashka [mailto:siarhei.siamashka at gmail.com] 
Sent: Wednesday, March 06, 2013 8:57 PM
To: nemanja.lukic at rt-rk.com
Cc: pixman at lists.freedesktop.org
Subject: Re: [Pixman] [PATCH] MIPS: DSPr2: Fix bug in
over_n_8888_8888_ca/over_n_8888_0565_ca routines

On Mon, 4 Mar 2013 10:58:42 +0100
"Nemanja Lukic" <nemanja.lukic at rt-rk.com> wrote:

> > Are you referring to MIPS implementation of the following code?
> > 
> >
>
http://cgit.freedesktop.org/pixman/tree/pixman/pixman-fast-path.c?id=pixman-
> 0.29.2#n389
> 
> Yes.
> 
> > Looks like a lot of changes for only adding a missing shift. Are you
> > really just fixing a single bug and not also introducing something
> > unrelated?
> 
> Yes, it really does look like a huge change for couple of missing shifts.
> When I wrote this code in the first place, I misplaced those shifts, which
> allowed me to combine code for over operation and:
>   UN8x4_MUL_UN8x4 (s, ma);
>   UN8x4_MUL_UN8 (ma, srca);
>   ma = ~ma;
>   UN8x4_MUL_UN8x4_ADD_UN8x4 (d, ma, s);
> where shifts are not present (for ma). So I decided to rewrite that piece
of
> code
> from scratch. I changed logic, so now assembly code mimic code from
> pixman-fast-path.c
> but process two pixels at a time. This code should be easier to debug and
> maintain.

OK, thanks. It would be nice to have this explanation in the commit
message.

Or alternatively, fixing the bug in one commit and then reorganizing
the code to make it cleaner/faster in another commit would have been
much easier to review.

-- 
Best regards,
Siarhei Siamashka