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:[email protected]] Sent: Wednesday, March 06, 2013 8:57 PM To: [email protected] Cc: [email protected] 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" <[email protected]> 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 _______________________________________________ Pixman mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pixman
