imagej / imagej-ops

ImageJ Ops: "Write once, run anywhere" image processing
https://imagej.net/libs/imagej-ops
BSD 2-Clause "Simplified" License
88 stars 42 forks source link

AbstractPadAndFFTFilter overrides OutOfBoundsFactory parameter of PadAndConvolveFFTF #628

Closed Xanthorapedia closed 4 years ago

Xanthorapedia commented 4 years ago

When ops.filter().convolve() is called with arguments RAI, RAI, OutOfBoundsFactory, the result seems as if the input was padded by OutOfBoundsConstantValueFactory with value 0 instead of as specified by the last argument.

Example:

import net.imagej.ImageJ;
import net.imagej.ops.OpService;
import net.imglib2.img.Img;
import net.imglib2.img.array.ArrayImgs;
import net.imglib2.outofbounds.OutOfBoundsPeriodicFactory;
import net.imglib2.type.numeric.real.FloatType;

public class WrongPadding {
    public static void main(String[] args) {
        ImageJ ij = new ImageJ();

        OpService ops = ij.op();

        Img<FloatType> ramp = ArrayImgs.floats(256, 256);
        Img<FloatType> average = ArrayImgs.floats(64, 64);

        int i = 0;
        for (FloatType iv : ramp)
            iv.set(i / 256 + (i++) % 256);
        for (FloatType kv : average)
            kv.set(1f / (64 * 64));

        ij.ui().show("Input", ramp);
        ij.ui().show("Zero padding", ops.filter().convolve(ramp, average));
        ij.ui().show("Periodic pading", ops.filter().convolve(ramp, average, new OutOfBoundsPeriodicFactory<>()));
    }
}

Results: wrong padding Both outputs look the same, although I expect the later to at least have some white "bleed-through" at the top and left edges.

It seems the input is first padded by the matched Op PadAndConvolveFFTF with the specified OutOfBoundsFactory but later padded again by AbstractPadAndFFTFilter with zeros. The first padding keeps the image dimensions the same while the second extends the boundary by 1/2 kernel size, which catches all out-of-bound access to the input image when computing FFT.

gselzer commented 4 years ago

Hi @Xanthorapedia!

Thanks for pointing this out. I started work on a fix on the pass-oobf branch, if you would like to take a look

bnorthan commented 4 years ago

I also made the same change (it was a simple change) https://github.com/imagej/imagej-ops/commit/a8fe660341993196253af5ba20156422974cc467

I added a test as well if it is useful... feel free to erase my branch. As I mentioned it was a very trivial change.

gselzer commented 4 years ago

Awesome @bnorthan! Great minds think alike!

I might just add my comment to your commit for posterity and then delete my branch, if that is alright with you. Then we can file a PR.

bnorthan commented 4 years ago

No problem.

Xanthorapedia commented 4 years ago

Thanks guys for your super-fast response! One last thing I found a little annoying is this debug message print out. Consider turning it into a log or removing it?

gselzer commented 4 years ago

@Xanthorapedia see #630

Xanthorapedia commented 4 years ago

@gselzer Awesome. Thanks!