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