Hi For all interested, I have applied the first changes to master (see http://linfiniti.com/2011/08/improvements-to-raster-performance-in-qgis-master/)....and with a unit test! :-)
I still need to apply Etienne's patch for progress bar support which I hope to do in this week. regards Tim On Thu, Aug 4, 2011 at 9:40 AM, Tim Sutton <li...@linfiniti.com> wrote: > Hi > > On Wed, Aug 3, 2011 at 7:38 PM, Etienne <etienne...@yahoo.com> wrote: >> Hi, I'm glad I can help! >> >> 1) regarding the histogram caching: >> GDAL does this automatically, it looks for existing aux file and a >> suitable histogram block. It's all in function >> GDALPamRasterBand::PamFindMatchingHistogram() in file >> gdalpamrasterband.cpp . >> As long as the dataset has support for PAM (stored in the .aux.xml file), it >> should work. >> I found a bug in the floating-point comparison which was already fixed: >> http://trac.osgeo.org/gdal/ticket/4183 >> >> Unlike the stats functions (GetStatistics/ComputeStatistics), there is only >> 1 function which looks for cached data. >> >> > > Ok for me it seems to recompute the histogram every time (which takes > about a minute+ with the sample data set I sent you). I will update my > gdal svn copy to get your patch for #4183 and see if the problem goes > away. > >> 2) regarding the caching of stats: >> Actually the GDAL driver looks for stored metadata (in function >> GDALRasterBand::GetStatistics). >> This works for gtiff files, I don't know for other raster types. >> > > ok > >> >> 3) regarding the progress bar: >> Do you have sufficient authority to ask the GDAL folks to change the >> prototype for GDALGetRasterStatistics() and GetStatistics() for them to >> include a progress call-back? I could make a patch for it, but I don't know >> if they would accept it. >> In the mean time, I can copy the GDAL code into qgsgdalprovider and submit >> that patch to you. >> > > Ok Martin addressed this in his email in this thread. I basically have > no more authority than anyone else in the community - they evauate > each request based on merit. As Martin mentions, breaking ABI > compatibility will not go down well with them so providing a wrapper > function which adds progress support (or perhaps adding the progress > callback as the last param with a default of void so that it can be > left out will be acceptable to them) would be good. > >> 4) >> There is another issue I forgot to mention, and that is the bin count of the >> histogram. GDAL uses 256 bins, and recently the GDAL histogram >> calculation in QgsRasterLayerProperties::refreshHistogram() has been >> changed to use 255 bins. ( const int BINCOUNT = 255; ). Could this be >> put back to 256 for consistency with GDAL, or is there a reason for >> that? >> > > Ok I have changed in my local copy and will commit soon. > >> >> Thanks! Etienne >> > > Thank you! > > Regards > > Tim > >> ________________________________ >> From: Tim Sutton <li...@linfiniti.com> >> To: Etienne <etienne...@yahoo.com> >> Cc: "Qgis-developer@lists.osgeo.org" <Qgis-developer@lists.osgeo.org> >> Sent: Wednesday, August 3, 2011 6:00:56 AM >> Subject: Re: [Qgis-developer] Provider native raster band stats >> >> Hi Again >> >> By the way is there a handy way to get the histogram from the aux.xml >> too? I see it is stored there but haven't figured out how to retrieve >> it from cache again. >> >> >> >> >> ----- Original Message ----- >> From: Tim Sutton <li...@linfiniti.com> >> To: Etienne <etienne...@yahoo.com> >> Cc: "Qgis-developer@lists.osgeo.org" <Qgis-developer@lists.osgeo.org> >> Sent: Wednesday, August 3, 2011 6:00:56 AM >> Subject: Re: [Qgis-developer] Provider native raster band stats >> >> Hi Again >> >> By the way is there a handy way to get the histogram from the aux.xml >> too? I see it is stored there but havent figured out how to retrieve >> it from cache again. >> >> Regards >> >> Tim >> >> On Wed, Aug 3, 2011 at 10:16 AM, Tim Sutton <li...@linfiniti.com> wrote: >>> Hi >>> >>> On Tue, Aug 2, 2011 at 7:06 PM, Etienne <etienne...@yahoo.com> wrote: >>>> Tim, my impression is that it speeds things up, but I have a suggestion to >>>> make it faster. >>>> >>> Cool! >>> >>>> From what I have seen in your code, the QgsGdalProvider::bandStatistics >>>> function calls GDALComputeRasterStatistics (), which computes the >>>> statistics every time - it doesn't fetch pre-calculated data ( which is >>>> stored in gtiff metadata, or aux.xml file). >>>> >>>> A better way would be to use the GDALGetRasterStatistics() which >>>> calls GDALComputeRasterStatistics() when needed (when bForce = TRUE) >>>> double myerval = GDALGetRasterStatistics ( myGdalBand, bApproxOK, TRUE, >>>> &pdfMin, &pdfMax, &pdfMean, &pdfStdDev); >>>> >>> >>> Ah - actually I thought GDALComputeRasterStatistics was using aux.xml >>> cached stats already. I guess I never read the docs carefully enough. >>> I applied your change and my ~8min to load raster from my original >>> email took about a minute to load the first time (while gdal was >>> creating an aux.xml) and subsequent use of the same raster resulted in >>> it opening instantly! So thanks for your excellent advice. I have >>> pushed an updated version to my repo that incorporates your changes >>> should others like to try. I will try to write some unit tests this >>> week and then merge my branch into QGIS master over the weekend. >>> >>>> >>>> >>>> I have tested it on a few rasters, it gets the same stats. >>>> >>>> The downside is that GDALGetRasterStatistics() does not offer a progress >>>> callback, rendering the progressbar useless. >>>> If it"s really needed, I could re-implement the function in >>>> QgsGdalProvider (about 50 lines of code), or try to >>>> have GDALGetRasterStatistics support a progress callback. >>>> >>> >>> Yes this is quite noticeable the first time the raster is loaded and >>> you sit wondering what is happening. I guess the more elegant would be >>> to have the change carried out in gdal itself if that is acceptible, >>> otherwise your patch as per your former suggestion would be greatly >>> appreciated. >>> >>> Best regards >>> >>> Tim >>> >>>> regards, Etienne >>>> >>>> ----- Original Message ----- >>>> From: Tim Sutton <li...@linfiniti.com> >>>> To: Marco Hugentobler <marco.hugentob...@sourcepole.ch> >>>> Cc: qgis-developer@lists.osgeo.org >>>> Sent: Monday, July 11, 2011 7:04:08 PM >>>> Subject: Re: [Qgis-developer] Provider native raster band stats >>>> >>>> Hi >>>> >>>> >>>> On Mon, Jul 11, 2011 at 5:29 PM, Marco Hugentobler >>>> <marco.hugentob...@sourcepole.ch> wrote: >>>>> Hi Tim >>>>> >>>>> I'm not a raster guru, but the changes look good to me. >>>>> +1 for merging. >>>> >>>> Thanks for your feedback Marco. I guess I'll write some unit tests >>>> (wrong way around I know :-) ) and then merge it if they all pass and >>>> there are no other objections. :-) >>>> >>>> Regards >>>> >>>> Tim >>>> >>>> >>>>> >>>>> Regards, >>>>> Marco >>>>> >>>>> Am Donnerstag, 7. Juli 2011, 00.47:25 schrieb Tim Sutton: >>>>>> Hi >>>>>> >>>>>> During the Lisbon hackfest (between all those meetings!) I worked on >>>>>> updating the raster providers to be able to generate stats themselves. >>>>>> The raster provider base class has a default (and historically >>>>>> inefficient as we all know) implementation for collecting stats by >>>>>> walking over every cell in the raster twice. This method remains, but >>>>>> the providers can now supply stats themselves (heopfully using more >>>>>> efficient mechanisms). I have implemented gdal native stats already. >>>>>> Over the last few weeks I did some further testing and cleanups to >>>>>> this work. From my testing some of my worst case files opened in >>>>>> ~1minute rather than ~8+ minutes. It would be great if interested >>>>>> parties could test (details below) before I put it into master. >>>>>> >>>>>> git remote add timlinux git://github.com/timlinux/Quantum-GIS.git >>>>>> git fetch timlinux >>>>>> git branch --track timlinux raster-stats >>>>>> git checkout raster-stats >>>>>> >>>>>> I look forward to your feedback. >>>>>> >>>>>> Regards >>>>> >>>>> >>>>> -- >>>>> Dr. Marco Hugentobler >>>>> Sourcepole - Linux & Open Source Solutions >>>>> Churerstrasse 22, CH-8808 Pfäffikon SZ, Switzerland >>>>> marco.hugentob...@sourcepole.ch http://www.sourcepole.ch >>>>> Technical Advisor QGIS Project Steering Committee >>>>> >>>> >>>> >>>> >>>> -- >>>> Tim Sutton - QGIS Project Steering Committee Member (Release Manager) >>>> ============================================== >>>> Please do not email me off-list with technical >>>> support questions. Using the lists will gain >>>> more exposure for your issues and the knowledge >>>> surrounding your issue will be shared with all. >>>> >>>> Visit http://linfiniti.com to find out about: >>>> * QGIS programming and support services >>>> * Mapserver and PostGIS based hosting plans >>>> * FOSS Consulting Services >>>> Skype: timlinux >>>> Irc: timlinux on #qgis at freenode.net >>>> ============================================== >>>> _______________________________________________ >>>> Qgis-developer mailing list >>>> Qgis-developer@lists.osgeo.org >>>> http://lists.osgeo.org/mailman/listinfo/qgis-developer >>>> >>>> _______________________________________________ >>>> Qgis-developer mailing list >>>> Qgis-developer@lists.osgeo.org >>>> http://lists.osgeo.org/mailman/listinfo/qgis-developer >>>> >>> >>> >>> >>> -- >>> Tim Sutton - QGIS Project Steering Committee Member (Release Manager) >>> ============================================== >>> Please do not email me off-list with technical >>> support questions. Using the lists will gain >>> more exposure for your issues and the knowledge >>> surrounding your issue will be shared with all. >>> >>> Visit http://linfiniti.com to find out about: >>> * QGIS programming and support services >>> * Mapserver and PostGIS based hosting plans >>> * FOSS Consulting Services >>> Skype: timlinux >>> Irc: timlinux on #qgis at freenode.net >>> ============================================== >>> >> >> >> >> -- >> Tim Sutton - QGIS Project Steering Committee Member (Release Manager) >> ============================================== >> Please do not email me off-list with technical >> support questions. Using the lists will gain >> more exposure for your issues and the knowledge >> surrounding your issue will be shared with all. >> >> Visit http://linfiniti.com to find out about: >> * QGIS programming and support services >> * Mapserver and PostGIS based hosting plans >> * FOSS Consulting Services >> Skype: timlinux >> Irc: timlinux on #qgis at freenode.net >> ============================================== >> >> > > > > -- > Tim Sutton - QGIS Project Steering Committee Member (Release Manager) > ============================================== > Please do not email me off-list with technical > support questions. Using the lists will gain > more exposure for your issues and the knowledge > surrounding your issue will be shared with all. > > Visit http://linfiniti.com to find out about: > * QGIS programming and support services > * Mapserver and PostGIS based hosting plans > * FOSS Consulting Services > Skype: timlinux > Irc: timlinux on #qgis at freenode.net > ============================================== > -- Tim Sutton - QGIS Project Steering Committee Member (Release Manager) ============================================== Please do not email me off-list with technical support questions. Using the lists will gain more exposure for your issues and the knowledge surrounding your issue will be shared with all. Visit http://linfiniti.com to find out about: * QGIS programming and support services * Mapserver and PostGIS based hosting plans * FOSS Consulting Services Skype: timlinux Irc: timlinux on #qgis at freenode.net ============================================== _______________________________________________ Qgis-developer mailing list Qgis-developer@lists.osgeo.org http://lists.osgeo.org/mailman/listinfo/qgis-developer