[calligra/krita-iterators2-rempt] krita/image/tiles3: Handle the situation where the ng iterators get an empty stretch

[Cyrille Berger Skott]

Hi,

Keep in mind that the driving factor for the design of the NG iterators was 
performance, performance and performance, while retaining as much as possible 
an API that is easy to use. The goal was to be at least as fast as extracting 
layer data in a linear buffer, meaning the mechanism behind the iterators had 
to be as simple as possible, with very few jumps, a reduced number of function 
call and no useless code (remember, a cycle spent in the iterator is actually 
1milion cycles in a 1000x1000 image). And the NG iterators have lived to their 
requirement.

They provide similar performance than copying layer data in a linear buffer, 
and they are twice as fast as the old iterators.

The price we have to pay is that the API is a bit unusual, with a loop check 
at the end, and that the user have to be careful not to use a null 
width/height. I personnaly think that the fact that the problem happened only 
once despite the widespread use of the iterators is proof that it is an 
acceptable drawback.

On Wednesday 25 Apr 2012, Dmitry Kazakov wrote:
> 1) Just make isDone() inlined (probably, not a very good solution)
There are two reasons why there is no "isDone()":

1) most important reason: if there is no isDone, we are guaranteed that 
developers won't use it. And we don't want them to use it, because the last 
thing we want is to see things like:

while(!it->isDone()) { blah; it->nextPixel(); }

Since that would introduce unecesserary function call

2) there is no way to implement isDone without a virtual call, or setting a 
variable in memory

> 2) Do the iteration like Qt does it in Java-style iterators:
> 
> KisRectIteratorSP it = dev->createRectIteratorNG(0, 0, 128, 128);
> while (it->nextPixel()) {
>     // do something
> }
>
> This approach is free from both the issues: it has no additional function
> call for isDone() (what Cyrille wanted to achieve), and it doesn't
> introduce any preconditions for the iterators, which mean they can handle
> empty and invalid rects as well.

On the other hand, it add extra complexity to the "nextPixel" function, hence 
cycles, to cover a corner case. Since you would have to either have a check to 
"nextPixel" to handle the first call or to positionate the iterator at 
beginning-1, and take the risk that the iterator request an useless tile.

If you can find a way that does not require any CPU cycle, then, by all mean 
provide a patch :)

-- 
Cyrille Berger Skott