preparing filters to be threadable

[Boudewijn Rempt]

On Friday 09 March 2007, Schleimer, Ben wrote:
> Hi all,
>   I just finished working on improving KisPreviewWidget (in krita 1.6) so
> it could display the bumpmap filter again. I changed the implementation so
> all filtering for the preview is done inside KisPreviewWidget and instead
> of prescaling the source paint device, I postscale the generated QImage. If
> the supportsThreading is true, then KisPreviewWidget optimizes by rendering
> 128x128 pixel chunks of src->dst device. The work is scheduled by using a
> QTimer although using threads would be better. Otherwise, the whole image
> is rendered for preview at once.
>
> I just have a couple of points I'd like to share and I've attached the
> diffs (against svn 1.6) for approval before submitting them.
>
> 1) The KisFilter::supportsXXX should be false (disabled) in KisFilter.
> (well, at least KisFilter::supportsThreading() ) This is because your new
> filter coder doesn't know how to write his plugin to support these various
> cool features. IMHO it should be his responsibility to figure how/if his
> filter should be threadable/incrementable. Otherwise the KisFilter
> interface is excellent.

You're probably right. For 2.0 we should design a better system for defining 
capabilities, perhaps something on the lines of QPaintEngine's 
PaintEngineFeatures?

> 2) KisBlurFilter doesn't implement the KisFilter::overlapMarginNeeded().
> Nor does any of the implementations except for convolutionFilter (I'm
> thinking about blur filter here...) Currently, this causes nasty lines
> every 128px to display if the filter is "threadable" but doesnt implement
> overlapMarginNeeded(). I don't really know how to use it in the new
> KisPreviewWidget either. I'm hoping that someone (cyrille?) can point out
> the way.

Do we actually multi-thread filters in 1.6? I thought we never implemented 
that. Oh, wait, I should learn to read -- you've just implemented chunked 
filtering.

In 2.0, I've started writing a KisThreadedApplicator, inspired by something 
similar in the nVidia opengl photoshop filter SDK. That should take care of 
all multi-threaded, chunked application of filters (and other things, too).

The MultiThreadedApplicator will take the amount of overlap pixels specified 
by the filter and create and filter overlapping chunks of image, pasting them 
together when done. That way, the filters don't have to be aware of chunked 
processing at all.

> 3) I think that a good approach to help thread the "global" filters
> (especially the histograph-based ones) would be to add a preprocessing
> method to KisFilter. Maybe this could be threaded too?

That sounds like a good idea -- could you expand on how it would work? We 
could easily add a preProcess() method to the KisFilter API and call that 
before calling process(), of course.

> 4) The KisPreviewWidget current "chunks" the images by 128x128 pixels. Is
> there a better size? Maybe the filter should be able to provide an optimal
> chunk size hint depending on the type of the colorspace?

I think it's not the colorspace that makes the difference here, at least not 
directly, but the subtle interplace between threading overhead, function call 
overhead, processor cache etc. So, I'd make this a KisConfig setting so we 
can play with it. In fact, in the KisThreadedApplicator class in trunk it is 
already a setting: threadingtilessize. I'll change that class to use chunks 
instead tiles, though, it's a clearer word and doesn't cause confusion with 
the tile subsystem.

> 5) I'm thinking about adding a KisLineProgress to KisPreviewWidget but it
> doesn't understand threading right now. Would it be better to subclass it
> with a class which understands threading or let the plugins calculate their
> progress based on rect and the size of the source image?

I wouldn't go to that length in 1.6, myself. The whole progress system is 
rewritten in 2.0 so the work would likely be short-lived and the process is 
pretty complicated. And in 2.0 we already have a progress bar implemenation 
(by Thomas Zander) that understands threading very well.


-- 
Boudewijn Rempt 
http://www.valdyas.org/fading/index.cgi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kimageshop/attachments/20070309/7cd822cf/attachment.pgp