koffice/krita/image

[Dmitry Kazakov]

On Sat, Oct 3, 2009 at 12:12 PM, Boudewijn Rempt <boud at valdyas.org> wrote:

> 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


like colorsfilters, right? :p


> or otherwise
> broken (like the old artistic filters). I briefly considered, way back, to
> add
> a flag to indicate this sort of brokenness, needsCopy()


no-no-no =) Just fix that.


> 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.
>

The filter can check whether dst==src and optimize it's work... Just an
idea...




> > > > 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.


The code should be clean and tidy all the time, not depending on the phase
of the Moon =) Otherwise it will never be good.
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.


> 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.
>

Filter api is absolutely ok. KisConvolutionPainter simply has a bug, and you
are suggesting to fix a mask to hide this problem.


>
> 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.
>

Please do this.




>  > 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!
>

"Never put off till tomorrow what you can do today". Hiding a problem is
even worse.



> > 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.
>

We should join "manual" application and masks application. That is another
code duplication.


-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20091003/cb17342d/attachment.htm