Gaussian Blur Bug

[Thomas E Deweese <th...@kodak.com>]

Hi all,

   So I spent an excrusiating day and a half tracking this one down.

DirectColorModel.convertData(boolean newAlphaPreMultState) 

   Does nothing (a bug in the JDK).

    The GaussianBlurOp tries to make sure that it does it's processing
with alpha premult.  Since the ConcreteGraphicsNodeRable uses
TYPE_INT_ARGB (not TYPE_INT_ARGB_PRE) the GaussianBlurOp must coerce
it's input data to be premultiplied.  It was using the above method
which sets all the flags to say the image data is now PreMultiplied
but due to an instance of the infamous 'int division into a float bug'
actually does nothing.

    We then blur the data, and return the result as premultiplied data
(no reason to divide out the alpha).  This get's further processed
until we we go to display the image.  This goes through a different
code path that actually does coerce the data (in the drawimage stuff),
we then divide the alpha out of our unpremultiplied image data the
result being some values wrap around.

    This was really, really, really annoying to track down since some
code paths did the conversion and some didn't so you only got a
noticable problem when you mixed the two...

    I now have a version of the GaussianBlurOp that avoides the use of
coerce data (it uses drawImage to coerce the data instead, it seems
reasonably fast).

    I tried ColorConvert but that also messed up alpha...

    While I think we should preserve my work around I also think that
our default mode of operation should be pre-multiplied alpha.  So
ConcreteGraphicsNodeRable should use TYPE_INT_ARGB_PRE (perhaps we
could have a debug flag which switches back so we can test if we
handle non-premult properly).

Code at fault (from java.awt.image.DirectColorModel.java):

        float normAlpha;
        int alphaScale = (1 << nBits[aIdx]) - 1;
	[...]
	int [] pixel;
	[...]
	    pixel = raster.getPixel(rX, rY, pixel);
            normAlpha = pixel[aIdx]/alphaScale;
                        ^^^^^^^^^^^^^^^^^^^^^^ (int)/(int)

	    since 0 <= (pixel[aIdx] / (float)alphaScale) <= 1.0 
	    the result is always either 0 or 1. 
	    if it's zero they do nothing...
	    If it's one  it's the identity element
	    So either way nothing happens...

    I'm still checking all the cases but I should be commiting a new
GaussianBlurOp that solves this problem before to long.

Re: Gaussian Blur Bug

[Vincent Hardy <vi...@eng.sun.com>]

Thomas,

About the ARGB_PRE, I have a question: SVG filters all operate on 
linear RGB, premultiplied, and some in non-premultiplied (a minority).
Shouldn't we use linear RGB premultiplied instrad of sRGB?
V.

Thomas E Deweese wrote:
> 
> Hi all,
> 
>    So I spent an excrusiating day and a half tracking this one down.
> 
> DirectColorModel.convertData(boolean newAlphaPreMultState)
> 
>    Does nothing (a bug in the JDK).
> 
>     The GaussianBlurOp tries to make sure that it does it's processing
> with alpha premult.  Since the ConcreteGraphicsNodeRable uses
> TYPE_INT_ARGB (not TYPE_INT_ARGB_PRE) the GaussianBlurOp must coerce
> it's input data to be premultiplied.  It was using the above method
> which sets all the flags to say the image data is now PreMultiplied
> but due to an instance of the infamous 'int division into a float bug'
> actually does nothing.
> 
>     We then blur the data, and return the result as premultiplied data
> (no reason to divide out the alpha).  This get's further processed
> until we we go to display the image.  This goes through a different
> code path that actually does coerce the data (in the drawimage stuff),
> we then divide the alpha out of our unpremultiplied image data the
> result being some values wrap around.
> 
>     This was really, really, really annoying to track down since some
> code paths did the conversion and some didn't so you only got a
> noticable problem when you mixed the two...
> 
>     I now have a version of the GaussianBlurOp that avoides the use of
> coerce data (it uses drawImage to coerce the data instead, it seems
> reasonably fast).
> 
>     I tried ColorConvert but that also messed up alpha...
> 
>     While I think we should preserve my work around I also think that
> our default mode of operation should be pre-multiplied alpha.  So
> ConcreteGraphicsNodeRable should use TYPE_INT_ARGB_PRE (perhaps we
> could have a debug flag which switches back so we can test if we
> handle non-premult properly).
> 
> Code at fault (from java.awt.image.DirectColorModel.java):
> 
>         float normAlpha;
>         int alphaScale = (1 << nBits[aIdx]) - 1;
>         [...]
>         int [] pixel;
>         [...]
>             pixel = raster.getPixel(rX, rY, pixel);
>             normAlpha = pixel[aIdx]/alphaScale;
>                         ^^^^^^^^^^^^^^^^^^^^^^ (int)/(int)
> 
>             since 0 <= (pixel[aIdx] / (float)alphaScale) <= 1.0
>             the result is always either 0 or 1.
>             if it's zero they do nothing...
>             If it's one  it's the identity element
>             So either way nothing happens...
> 
>     I'm still checking all the cases but I should be commiting a new
> GaussianBlurOp that solves this problem before to long.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: batik-dev-unsubscribe@xml.apache.org
> For additional commands, e-mail: batik-dev-help@xml.apache.org