koffice/krita/image

[Boudewijn Rempt]

On Friday 02 October 2009, Dmitry Kazakov wrote:

> Do you mean that for most of the filters this bitBlt() is simply a waste of
> time, right? ;)

Yes. Unfortunately, most of the filters _are_ convolution filters or otherwise 
broken (like the old artistic filters). I briefly considered, way back, to add 
a flag to indicate this sort of brokenness, needsCopy() or something like 
that, but decided that that would just be compounding the problem.

> We do already have several bitBlts during one merge operation, i think this
> one is superfluous.

It makes blur work in 2.1, so it's pretty useful :-)

> If a filter doesn't want to write to some area he should declare this with
> changeRect() function, but not just not write there.

The area where a filter doesn't write isn't always a rect. Look at the funny 
results of the raindrops filter we had previously: 
http://rempt.xs4all.nl/fading/index.cgi/hacking/krita/amusing.comments. 

> > > damn!

> I just want Krita's code to be clean and understandable. And i don't want
>  to see any crutches there. I think if some (old) code is broken it should
>  be fixed or disabled. And we mustn't break any new code too, only for the
>  reason of "giving a crutch to the old one".

That depends a lot on the phase of the project: we're now trying to stabilize 
2.1 and get it ready for release. That means really big refactorings are 
really risky, so I'm not prepared to go for a refactoring of the filter api. 
After 2.1 is branched, I'll happily consider any refactoring scheme again. 

I'm totally happy with placing a big "XXX!!! Why do we need this copy! Fix 
after 2.1" in both places where we do this copy.

> I'm continuously trying to explain this idea... And it seems that i fail...

Well, sometimes you're expressing yourself a little forcefully, which leads to 
friction. And remember the prime rule of software development: there is always 
life after the release!

> Have you got the proof of wrong behavior of convolution painter?

I haven't yet written a unittest for it, no. I was too busy actually fixing 
other bugs.

> hmm.. What do you mean by "manual filter application"? I thought if we
>  apply a filter directly through Filters-menu they are added as a
>  TemporaryMask and merged to the layer then, aren't they?

KisFilterHandler defers to KisFilterJob which applies the filter directly. We 
had the idea once to simply swap projection and original if there were no 
other masks than the temporary mask, but that was never implemented. Also, if 
we ever implement region-of-interest recomposition (i.e., recomposite only the 
visible area) that will fail.

> I don't think it's good to have one, especially if we continue using bitBlt
> for selections.

I'm fine with any discussions on improving this after 2.1 -- there are still 
44 bugs to fix first. After 2.1, I'll probably be happy with anything you and 
Cyrille agree on since my main task will be finishing the paintop preset 
saving and loading and general performance tuning.
-- 
Boudewijn Rempt | http://www.valdyas.org