Ah, forgot to add kimageshop as a recipient. =)<br><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">Dmitry Kazakov</b> <span dir="ltr"><></span><br>Date: Sat, Sep 18, 2010 at 9:58 AM<br>
Subject: Re: patch for moving the calculation of image colours to a background thread<br>To: "Adam C." <><br><br><div class="im"><br><br><div class="gmail_quote">On Sat, Sep 18, 2010 at 12:08 AM, Adam C. <span dir="ltr"><></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
hi,<br>
here is the patch. i've never done something with multiple threads, so i'm completely green in that area.<br>
<br>
I need a call to KisImage::convertToQImage(..), which is quite slow, because it has to do much work on a big image. this is the reason i want it in a thread. otherwise it's not possible to update after every stroke. if we can replace it with something safer or faster, i'm fine with that of course.<br>
</blockquote></div><br></div>1) Yes, convertToQImage() is too slow. I think we shouldn't do it for the entire image every time. Just because it is very slow and inefficient.<br>2) convertToQImage() is not thread safe. It reads from rootLayer->projection(), that can be in a random state at arbitrary moments of time. In theory, you can wrap it into KisImage::lock()/unlock(), but that wouldn't be a good idea, because all the updates will stop for a while.<br>
<br>I think you need to stick to one of the updater threads somehow. I see three ways to do this:<br><br>1) You make a direct connection to KisImage::sigImageUpdated(rc) (<span style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Arial; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; font-size: medium;"><span style="border-collapse: collapse; font-family: monospace;">Qt::DirectConnection</span></span>). Then you'll get threading and locking for granted. You will have a guarantee of safe reads from @rc in a context of that signal. But this method has a drawback: you need to guarantee somehow, that your connection will be called *after* KisCanvas2::startUpdateCanvasProjection(). In other case, recalculation of colors would delay updates of the canvas, that is really a bad thing.<br>
<br>2) As an alternative, you can stick to a ui thread and do the job after KisCanvas2::updateCanvasProjection() finished. You will know the updated rect, but you will have access to a projection cache only. Actually, i don't know whether it is possible to read from OpenGL textures, so i'm not sure about this way.<br>
<br>3) Actually, i don't think it is absolutely important to update colors after every single stroke, is it? We can update them after, say, 2 seconds of inactivity. Every stroke made by user can reset the timer, so we will never irritate him with this. So, how will it look like:<br>
<br>- you connect recalculation of colors to QTimer::timeout() (timeout is set to 2 sec)<br>- connect (with auto connection) KisImage::sigImageUpdated(rc) to QTimer::start()<br><br>then you have two options:<br>a) on every timeout, you call KisImage::lock(); KisImage::convertToQImage(); KisImage::unlock() and do your work in UI thread<br>
b) more complicated, but more efficient way: you store a QRegion of dirty rects and call convertToQImage() for them only.<br><br><br>As for me, i like 3,b) option :)<br></div><br>-- <br>Dmitry Kazakov<br>