Comment 181 for bug 612377

Revision history for this message
In , Jmuizelaar (jmuizelaar) wrote :

(In reply to comment #163)
> (2) RGB_{RED|GREEN|BLUE} macro conflicts on OS/2 (OS/2? Really?): This seems
> like an innocuous enough patch, although I'd rather have it in jmorecfg.h
> instead of jpeglib.h. I'd also like to know more of the back story as to why
> OS/2 is defining those macros. Is this a generic problem that anyone building
> libjpeg-turbo on OS/2 would encounter? My lingering doubt here is that libjpeg
> supports OS/2, so I wonder why they haven't encountered this problem on that
> platform. Is Mozilla including some header file that causes those macros to be
> defined?

I'd be fine with dropping this altogether. If OS/2 still needs it, we can always add it back.

>
> (3) bitscan instructions: I tested the proposed patch to make jcdctmgr.c use
> GCC 3.4+ compiler intrinsics to replace flss() with bitscan instructions, but I
> cannot observe any measurable speedup from this (tested several different image
> types and platforms, as well as 32-bit and 64-bit.) If it were even 2% faster,
> I would agree to include it, but a performance patch that causes no appreciable
> increase in performance does nothing but needlessly obfuscate the code.

It's probably worth adding a comment to flss saying that it's not performance critical enough to justify using compiler intrinsics.

> (4) The tables in jccolor.c are part of the optimized RGB-to-grayscale
> conversion routines. These are pre-computed R-to-luminance, G-to-luminance,
> and B-to-luminance conversion values. The problem with this approach is that
> it loses a bit of accuracy due to round-off, so I recently checked in code to
> the SVN trunk (1.2 working version) that uses new SIMD routines to perform
> RGB-to-grayscale conversion. This is faster and eliminates the round-off
> issues, as well as the need for the tables in jccolor.c and the #ifdef in
> rgb_gray_convert().

That sounds like a good solution to me.