[webkit-reviews] review denied: [Bug 28308] allow down-sampling images during decoding : [Attachment 34867] modified for coding style...

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Yong Li <yong.li at torchmobile.com>'s
request for review:
Bug 28308: allow down-sampling images during decoding
https://bugs.webkit.org/show_bug.cgi?id=28308

Attachment 34867: modified for coding style...
https://bugs.webkit.org/attachment.cgi?id=34867&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
 126 #ifdef IMAGE_DECODER_DOWN_SAMPLING_MAX_NUMBER_OF_PIXELS
 127		
m_decoder->setMaxNumPixels(IMAGE_DECODER_DOWN_SAMPLING_MAX_NUMBER_OF_PIXELS);
 128 #else
 129		 m_decoder->setMaxNumPixels(1024 * 1024);
 130 #endif

Would be better to check #ifdef
IMAGE_DECODER_DOWN_SAMPLING_MAX_NUMBER_OF_PIXELS at the top fo the file, and
define IMAGE_DECODER_DOWN_SAMPLING_MAX_NUMBER_OF_PIXELS to 1024 * 1024 if not
defined.

Seems we'd benifit from some nicely named local variables:
47     case Exact:
 48	    return pos != dataEnd && *pos == orig ? pos - dataStart : -1;
 49	case LowerBound:
 50	    return pos != dataEnd && *pos == orig ? pos - dataStart : pos -
dataStart - 1;
 51	case UpperBound:
 52	default:
 53	    return pos != dataEnd ? pos - dataStart : -1;

Style:
 84	double zoom = 1. /shrink;

I would have made this a static inline function to prevent copy/paste:
94     m_scaledRows.reserveCapacity(height * shrink + 0.5);
 95	for (int scaledY = 0;;) {
 96	    int y = scaledY * zoom + 0.5;
 97	    if (y < height) {
 98		m_scaledRows.append(y);
 99		++scaledY;
 100	     } else
 101		 break;
 102	 }

 64		bool good = ImageDecoder::setSize(width, height);

good is not the word you want.	success would be better.  setSizeOK would be
another option.