Reviewing merge request #725: SSE2 inplace conversion from ARGB to ARGB_Premultiplied
As discussed extensively with Benjamin.
Commits that would be merged:
Comments
+ #if QT_HAVE_SSE2 + #ifdef QT_LINUXBASE + // this is an evil hack - the posix_memalign declaration in LSB + // is wrong - see http://bugs.linuxbase.org/show_bug.cgi?id=2431 + # define posix_memalign _lsb_hack_posix_memalign + # include <emmintrin.h> + # undef posix_memalign + #else + # include <emmintrin.h> + #endif +
Any reason why not simply #include qsimd_p.h here?
This code is probably there because I forgot to clean it when I introduced qsimd_p.h.
+ #if QT_HAVE_SSE2 + #ifdef QT_LINUXBASE + // this is an evil hack - the posix_memalign declaration in LSB + // is wrong - see http://bugs.linuxbase.org/show_bug.cgi?id=2431 + # define posix_memalign _lsb_hack_posix_memalign + # include <emmintrin.h> + # undef posix_memalign + #else + # include <emmintrin.h> + #endif +
damn bug in gitorious, duplicate comment :(
Looks good to me otherwise.
I had already reviewed your SSE2 code. The change in the build system is exactly the kind of solution I was hoping for SSE :)
I already merged your patches on my local branch. I will add the Neon version before pushing the changes.
The branch does not rebase on top of 4.7 because of conflicting changes in image.pri.
I cannot modify your commits. Could you please rebase your branch and resubmit the merge request? Sorry about that.
Rebased and moved inclusion logic to qsimd_p.h
Good job. I merged locally, I’ll do the Neon variant and push on oslo-staging-2.
Merged and pushed on Oslo-staging-2.


Add a new comment:
Login or create an account to post a comment