[Webkit-unassigned] [Bug 42273] Convolution computation causes bad alpha channel values

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42273





--- Comment #7 from Eric Seidel <eric at webkit.org>  2010-07-14 14:56:00 PST ---
(In reply to comment #6)
> | WebCore/svg/graphics/filters/SVGFEConvolveMatrix.cpp:251
> |  +                  paintingData.dstPixelArray->set(pixel++, std::min(clampRGBAValue(totals[0] / m_divisor + paintingData.bias), alpha));
> | Bah.  This should be a function of some form.  Copy/pasting 4 lines in a row is silly.
> 
> This just follows the pattern that was already in the code, including a side effect on pixel and the only change between lines being totals[0] -> 1, 2. I could make it prettier but it's not germane to the fix.

Agreed.  The problem is you're adding more bad copy/paste-ness.

Would you prefer:
> 
> paintingData.dstPixelArray->setPixel(pixel++, computeChannel(totals[0], alpha));
> paintingData.dstPixelArray->setPixel(pixel++, computeChannel(totals[1], alpha));
> paintingData.dstPixelArray->setPixel(pixel++, computeChannel(totals[2], alpha));
> 
> If that looks better it's trivial enough for me to put into this patch. If you'd like a bigger rearrange of the code can we put it on a follow up bug? Thanks for your feedback!

compute channel, or simply a inlline like:

doTheSetPixelStuff(paintingData, totals, alpha);

I don't know exactly what parts I woudl throw into an inline, but there is clearly code to share here.

Part of the goal of using inlines for things like this is to elmiinate the possibliy of copy/paste errors.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.