<br><br><div class="gmail_quote">On Sat, Oct 3, 2009 at 12:12 PM, Boudewijn Rempt <span dir="ltr"><<a href="mailto:boud@valdyas.org">boud@valdyas.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On Friday 02 October 2009, Dmitry Kazakov wrote:<br>
<br>
</div><div class="im">> Do you mean that for most of the filters this bitBlt() is simply a waste of<br>
> time, right? ;)<br>
<br>
</div>Yes. Unfortunately, most of the filters _are_ convolution filters</blockquote><div><br>like colorsfilters, right? :p<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
or otherwise<br>
broken (like the old artistic filters). I briefly considered, way back, to add<br>
a flag to indicate this sort of brokenness, needsCopy()</blockquote><div><br>no-no-no =) Just fix that.<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
or something like<br>
that, but decided that that would just be compounding the problem.<br>
<div class="im"><br>
> We do already have several bitBlts during one merge operation, i think this<br>
> one is superfluous.<br>
<br>
</div>It makes blur work in 2.1, so it's pretty useful :-)<br>
<div class="im"><br>
> If a filter doesn't want to write to some area he should declare this with<br>
> changeRect() function, but not just not write there.<br>
<br>
</div>The area where a filter doesn't write isn't always a rect. Look at the funny<br>
results of the raindrops filter we had previously:<br>
<a href="http://rempt.xs4all.nl/fading/index.cgi/hacking/krita/amusing.comments" target="_blank">http://rempt.xs4all.nl/fading/index.cgi/hacking/krita/amusing.comments</a>.<br></blockquote><div><br>The filter can check whether dst==src and optimize it's work... Just an idea...<br>
</div><div><br><br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> > > damn!<br>
<div class="im"><br>
> I just want Krita's code to be clean and understandable. And i don't want<br>
> to see any crutches there. I think if some (old) code is broken it should<br>
> be fixed or disabled. And we mustn't break any new code too, only for the<br>
> reason of "giving a crutch to the old one".<br>
<br>
</div>That depends a lot on the phase of the project: we're now trying to stabilize<br>
2.1 and get it ready for release. </blockquote><div><br>The code should be clean and tidy all the time, not depending on the phase of the Moon =) Otherwise it will never be good.<br>And, i think, there is no reason to be stabilizing code for several months, and then refactor it completely right after release to make it unstable again. There is no reason to make the same work twice. The code should be clean since the beginning.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">That means really big refactorings are<br>
really risky, so I'm not prepared to go for a refactoring of the filter api.<br>
After 2.1 is branched, I'll happily consider any refactoring scheme again.<br></blockquote><div><br>Filter api is absolutely ok. KisConvolutionPainter simply has a bug, and you are suggesting to fix a mask to hide this problem.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
I'm totally happy with placing a big "XXX!!! Why do we need this copy! Fix<br>
after 2.1" in both places where we do this copy.<br></blockquote><div><br>Please do this.<br><br><br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">
> I'm continuously trying to explain this idea... And it seems that i fail...<br>
<br>
</div>Well, sometimes you're expressing yourself a little forcefully, which leads to<br>
friction. And remember the prime rule of software development: there is always<br>
life after the release!<br></blockquote><div><br>"Never put off till tomorrow what you can do today". Hiding a problem is even worse.<br> </div><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">
> Have you got the proof of wrong behavior of convolution painter?<br>
<br>
</div>I haven't yet written a unittest for it, no. I was too busy actually fixing<br>
other bugs.<br>
<div class="im"><br>
> hmm.. What do you mean by "manual filter application"? I thought if we<br>
> apply a filter directly through Filters-menu they are added as a<br>
> TemporaryMask and merged to the layer then, aren't they?<br>
<br>
</div>KisFilterHandler defers to KisFilterJob which applies the filter directly. We<br>
had the idea once to simply swap projection and original if there were no<br>
other masks than the temporary mask, but that was never implemented. Also, if<br>
we ever implement region-of-interest recomposition (i.e., recomposite only the<br>
visible area) that will fail.<br></blockquote><div><br>We should join "manual" application and masks application. That is another code duplication.<br><br></div></div><br>-- <br>Dmitry Kazakov<br>