[fdo-internals] RFC 65 Implement Resampling for GDAL Provider

[Frank Warmerdam]

Guys,

Looks good to me.

Best regards,
Frank

On Tue, Oct 23, 2012 at 2:51 PM, Greg Boone <greg.boone at autodesk.com> wrote:

>  I think the RFC is ok, the final say on the code review is Frank.****
>
> ** **
>
> Greg****
>
> ** **
>
> *From:* fdo-internals-bounces at lists.osgeo.org [mailto:
> fdo-internals-bounces at lists.osgeo.org] *On Behalf Of *Trevor Wekel
> *Sent:* Sunday, October 21, 2012 10:56 PM
>
> *To:* FDO Internals Mail List
> *Subject:* Re: [fdo-internals] RFC 65 Implement Resampling for GDAL
> Provider****
>
>  ** **
>
> Hi Frank,****
>
> ** **
>
> I have attached an updated patch to RFC 65 with the CPLPrintPointer
> modification and compile fixes [Makefile.am updates] for Linux.  The code
> compiles and the FDO unit tests are clean on VC10 32bit Windows, VC10 64bit
> Windows, and 32bit CentOS 5.****
>
> ** **
>
> I was also able to verify the functionality visually in MapGuide on all
> three platforms.  No code changes are required for MapGuide.  Just add the
> following parameter to the feature source XML document:****
>
> ** **
>
> <Parameter>****
>
>     <Name>ResamplingMethod</Name>****
>
>     <Value>bilinear</Value>****
>
> </Parameter>****
>
> ** **
>
> If everyone is ok with the RFC “as is”, can we get it approved?  I can
> check the code in the next day or so.****
>
> ** **
>
> Regards,
> Trevor****
>
> ** **
>
> ** **
>
> ** **
>
> *From:* fdo-internals-bounces at lists.osgeo.org
> [mailto:fdo-internals-bounces at lists.osgeo.org] *On Behalf Of *Frank
> Warmerdam
> *Sent:* October 12, 2012 5:30 PM
> *To:* FDO Internals Mail List
> *Subject:* Re: [fdo-internals] RFC 65 Implement Resampling for GDAL
> Provider****
>
> ** **
>
> ** **
>
> On Fri, Oct 12, 2012 at 3:20 PM, Trevor Wekel <trevor_wekel at otxsystems.com>
> wrote:****
>
> Hi Frank,****
>
>  ****
>
> I was not aware of VRTWarpedDataset so did not consider it initially.****
>
>  ****
>
> So I took a quick look at vrtwarped.cpp.  I have a question.  In
> vrtwarped.cpp, the ProcessBlock() seems to cache the results of the warp
> operation (GDALCopyWords around line 1174) into each raster band.  Does
> this mean that all blocks will end up being cached?****
>
> ** **
>
> Trevor,****
>
> ** **
>
> Yes, VRTWarpedDataset will always push produced tiles into the GDAL block
> cache.  Note that most drivers go through the block cache too.****
>
>  ****
>
>  For FDO, we basically call a ReadNext() to suck the contents of the tile
> into a buffer supplied by the caller.  The ReadNext() drives the calls to
> _getTile().  As far as I can tell, our access pattern is linear and we only
> access each tile once.  If we switch to using VRTWarpedDataset, will we
> have to call ProcessBlock() for each tile?  Will this end up caching all
> the blocks as we do the linear read?****
>
>  ** **
>
>  In a web context, the ideal would be a long lived server which keeps
> around the VRTWarpedDataset for subsequent use.  I not sure how practical
> that is in the FDO/MapGuide use case.   In this situation caching the
> blocks can help subsequent views but there might be better other layers at
> which to do that.****
>
> ** **
>
>   ****
>
> As an alternative to using VRTWarpedDataset, would it be ok to instantiate
> a GDALWarpOperation as a member of the FdoRfpStreamReaderByTileResample?
> Pseudo code would be something like this:****
>
>  ****
>
>
> FdoRfpStreamReaderGdalByTileResample::FdoRfpStreamReaderGdalByTileResample()
> ****
>
> {****
>
> m_warpOp = new GDALWarpOperation();****
>
> }****
>
>  ****
>
> FdoRfpStreamReaderGdalByTileResample::_getTile()****
>
> {****
>
> warpOptions = GDALCreateWarpOptions();****
>
> // populate warp options****
>
> m_warpOp->Initialize(warpOptions);****
>
> m_warpOp->WarpRegion(0,0,origXSize,origYSize);****
>
> }****
>
>  ****
>
> This may improve the speed of the _getTile() call if a) Initialize() can
> be called more than once and b) the new GDALWarpOperation() does a lot of
> setup work****
>
>  ** **
>
> It is more likely to be the Iniitalize() that is expensive and
> reinitialzing an existing WarpOperation is not a well trod path and likely
> to lead to pain.  I advise against it.  In retrospect, sticking with your
> existing approach is likely ok but you might want to get a sense of how
> much time is spent in the setup code vs. the actual warping.   Larger tiles
> will help.****
>
> ** **
>
> Best regards,****
>
> ** **
>
>  ****
>
> --
>
> ---------------------------------------+--------------------------------------
> I set the clouds in motion - turn up   | Frank Warmerdam,
> warmerdam at pobox.com
> light and sound - activate the windows | http://pobox.com/~warmerdam
> and watch the world go round - Rush    | Geospatial Software Developer****
>
> _______________________________________________
> fdo-internals mailing list
> fdo-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/fdo-internals
>
>


-- 
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam,
warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Software Developer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/fdo-internals/attachments/20121023/4d17ddaf/attachment.html>