Patch for speeding up the painting of the autobrush

LukasT.dev@gmail.com lukast.dev at gmail.com
Tue Feb 2 15:15:05 CET 2010


Hello,

I was inspecting some code in our auto brush paintop. 

The code path I wanted to optimize starts in
KisAutoBrush::generateMaskAndApplyMaskOrCreateDab

the important part is here:
....
    for (int y = 0; y < dstHeight; y++) {
        for (int x = 0; x < dstWidth; x++) {

            double x_ = (x - centerX) * invScaleX;
            double y_ = (y - centerY) * invScaleY;
            double maskX = cosa * x_ - sina * y_;
            double maskY = sina * x_ + cosa * y_;

            if (coloringInformation) {
            }
            //cs->setAlpha(dabPointer, OPACITY_OPAQUE - d->shape-
>interpolatedValueAt(maskX, maskY), 1);
            dabPointer += pixelSize;
        }


You can see that we compute the brush mask rotated and the mask is computed 
with function interpolatedValueAt(maskX, maskY);

The interpolation function computes bilinear interpolation of values from 
function KisCircleMaskGenerator::valueAt(double x, double y).

So I decided to not to use the interpolatedValueAt(maskX, maskY ) but using 
directly the KisCircleMaskGenerator::valueAt(double x, double y). The speed is 
like 4x faster drawing now. We don't call valueAt 4-times per pixel but just 
once. 

But the result is somehow shifted by 1px or so. Now the question is if the 
patch introduces the bug or the bug was present somewhere and this fix just un-
hides it.

I suppose this patch as performance fix and as fix towards sub-pixel precision.
I suppose that linear interpolation just introduced the rounding errors and 
that's why the results differs about 1 pixel. I did not take subpixel precision 
away because I did not remove any code which would do it for us.

There is a reason why interpolation was used. The function valueAt a long long 
time ago used to take integer inputs like you can see  
http://websvn.kde.org/trunk/koffice/krita/image/kis_auto_brush.cpp?r1=746489&r2=746490&pathrev=788424&

Cyrille supposed that it is the reason interpolatedValueAt was used for. To 
break x and y into integer coordinates, compute the values at integer 
coordinates and interpolate between them.

Here is the patch (git, but it is one line ;) )
http://lukast.mediablog.sk/patches/dont-interpolate-to-not-call-
valueAt-4times.patch

Here are the results:
http://lukast.mediablog.sk/i/AutoBrush_70px_rotated-new-randomLines.png
http://lukast.mediablog.sk/i/AutoBrush_70px_rotated-old-randomLines.png

Could you please test with and without patch and report issues?

What I want to implement futher:
o There are settings where the brush is symmetrical so I plan to compute 1/4 
of the brush mask
o Lookup table for atan2 ( lookup table for valueAt??? :) seems also possible)
o move the rotation to the end of the pipeline so we will be computing the 
brush mask unrotated and rotate when compositing
o if that's not enough mip mapping (like we do in gbr brush)


Sorry for long e-mail but I could not help myself :)
Can I commit?


More information about the kimageshop mailing list