[calligra] krita/image: Fix crash when updatable rect is empty
boud at valdyas.org
Thu Jun 9 16:34:03 CEST 2011
On Thursday 09 June 2011 Jun, Dmitry Kazakov wrote:
> > I am. The merger shouldn't process empty rects. Unless you want to argue
> > that the merger never should get empty rects in the first place: but even
> > then, the merger should stop processing if it gets an empty rect.
> Well, KisBaseRectsWalker checks for the emptiness of the rect and removes
> empty jobs. But this case is not covered by this check, because applyRect in
> KisUpdateOriginalVisitor is a calculated value and is not available during
> preparational stage in KisBaseRectsWalker.
> So, of course, we should check for the emptiness somewhere. And, currently,
> we check this in filters. I don't think we should check this twice.
We actually don't check in the filters whether we can an empty rect, though I have a patch pending that does that. But I disagree with you that this check is at the wrong place: we check it after calculating the QRect, and if the result is an empty rect, stop processing. That sounds very, very sane to me, regardless of whether filters can handle empty rects or not.
> It would be good to try and fix all filters so they handle empty rects by
> > returning without doing anything. However, that is something completely
> > separate from making sure the merger doesn't try to merge empty rects.
> Well, currently, all the filters should work with empty areas fine, because
> all of them use (or should use) iterators.
We are not talking about areas empty of pixels, we are talking about QRect(0,0,0,). So iterators are irrelevant here.
> If they don't, they are broken.
> Async merger relies on the guarantee, that the filter (at least) will not
> crash when the area is empty. I don't see any reason in moving the guarantee
> to the level of the caller, because in such a case we will have to fix all
> the code calling filters to make our system consistent.
I see a lot of sense in not calling filters if you know you don't have a rect anyway.
Boudewijn Rempt | http://www.valdyas.org, http://www.krita.org
More information about the kimageshop