[gdal-dev] RE: progressive rendering

[Tamas Szekeres]

2008/8/29 Even Rouault <even.rouault at mines-paris.org>:
> Tamas,
>
> The only thing that was in Adam's proposal and that I don't find in yours is
> that, when the RasterIO in async mode returns something valid, the user has
> no way to know which part of the buffer has been updated by the driver. Thus,
> it introduces some extra cost for progressive image display since you then
> need to resend the whole buffer to your display window (before the first
> call, you must initialize the buffer to some default value too).
>
> Question : do we expect that it is going to ruin performance and/or interest
> of progressive rendering ?
>

Yes, this might be a crucial point. I've been considering that it's
more convenient for the user to do the same thing in all rendering
phase as retrieving the whole image than updating only sub-regions. I
consider only rectangle regions would play a role in this case.

However I should somewhat retract my statement here because of one
reason in my mind. Actually we cannot cosider the user can use the
same buffer as the driver with respect to some of the SWIG bindings.
In the RasterIO interface implementations the bindings might copy the
buffers between the target language and the GDAL core. Therefore we
cannot consider the buffer as read-write during the rasterio read
operation. BTW currently we only have ReadRaster/WriteRaster exposed
to the SWIG API.

In conclusion I'm tending to take over what Adam is proposing, however
we should have an additional method (or the synchronous version of the
RasterIO itself) to copy the region at the SWIG API.
This extra copy seems to be inevitable to me, because currently the
user (and the SWIG interface) is not required to keep the buffer
passed to RasterIO permanently. The external LockBuffer/ReleaseBuffer
by this means could be handled at driver level.


> I agree that the driver implementation may not be multi-threaded. But it's
> just a choice of implementation from the driver writer. The LockBuffer() /
> UnlockBuffer() API doesn't necessarily imply that the driver must use an
> auxiliary thread. An empty implementation would do for a non multithreaded
> driver. It just makes it possible for the driver to update-in-place the user
> buffer without the need for an internal buffer.
> But... I agree that the need for the LockBuffer()/UnlockBuffer() API is
> arguable since you need the 3 following conditions to be met for it to be
> really usefull :
> - a multi-threaded driver
> - AND it does not use a temporary buffer
> - AND the user doesn't lock for too long an area of the buffer that is going
> to be updated by the driver, otherwise the thread of the driver will be
> stalled by the mutex on the buffer.
> All other cases do not need that API.
>
> We could also imagine that the API exists but that the documentation of the
> driver explicitely says whether it is necessary to explictely Lock() /
> Unlock() the buffer, depending on how it is implemented.
>

This kind of stuff may lead to various kind of potential issues,
likewise I recall the introduction of of the smart pointers in the COM
world in order to avoid the mistakes when using AddRef, Release in the
right order.


> Hum, I'm thinking that --enable-threading must be explicitely passed to
> configure. So it means that a multi-threaded driver should be disabled if
> threading is not enabled. Maybe not a big deal but until now, no GDAL driver
> had this requirement.
>

Until this point GDAL could mainly avoid to trigger multiple threads
pretty well. I think only the warping code use threads at all.
Therefore using this global setting may affect quite unrelated parts
in GDAL which may be disadvantageous. By any means the multithreaded
behaviour of a driver should be kept internal as much as we can.


Best regards,

Tamas