On 12/4/06, Martin Desruisseaux <[EMAIL PROTECTED]> wrote:
> Hello all
>
> I'm looking at the new operations in the coverage module ("CoverageCrop",
> "Scale", etc.). There is a few comments and questions:
>> - Note: I noticed that all those new operations declare themself as OGC > operations (Citation.OGC). This is not exact: OGC defines "Resample", > but doesn't define the "Crop" and "Scale" operations as far as a know. > Citation.GEOTOOLS would be more appropriate than Citation.OGC for them. > I agree. > - Why "CoverageCrop" is not called "Crop"? No other operation are prefixed > by "Coverage". > I don't remember. I guess it was not to confuse it with the JAI crop operation. > - Aren't "Crop" and "Scale" redundant with "Resample", given that "Resample" > is suppose to detect and treat crop and scales as special cases? I admit > that > their arguments are more convenient than "Resample", so we should keep them. > But should we just define them as wrapper delegating to the same "Resample" > code? It would be one more opportunity to test "Resample" more extensively. > My fault I did not talk much about this operations. Indeed, crop can be seen as a duplicate of Resample and, somehow the same applies to scale. However, rationale behind Crop and Scale is as follows: 1> I have always found Resample too complex. Resample shoud do resample not resample, crop, warp, reproject and scale :-). If I just want to crop a coverage to a certain envelope why should I use resample? On the other hand, if I want to scale a coverage by 2 on each side, why should I use resample and provide a gridgeometry? 2>Performances. Doing a scale using a warp can lead to bad performances, hence I want to have the freedom to choose which algorithm wo use for scaling, and I want to do just a scaling. IMHO, Resample should do just what it is meant for, that is : -resample a certain coverage -reproject a certain coverage -scale coverage in the case that known final dimensions have to be honored (scale and the other similar operation do not honorate ImageLayout bounds while warp does honorate it) Simpler, smaller operation (closer to the JAI ones btw) mean better user experience. And in general following more closely the way JAI is built, that is using simple, well-defined operations for well-recognized tasks, is better ( in my opinion :-) ). > - I noticed that Operation.doOperation, initially with protected access, has > been given public access and is invoked directly by CoverageRenderer. Does > it really make a performance difference, since the cost of looking in a > HashMap since negligible to me compared with the cost of all the remaining > operations? I ask that because one possible way to implement a "dispose" > method is to register CoverageProcessor as a listener, but it will not work > if users invokes "Operation.doOperation" directly instead of using > CoverageProcessor. It is not the only way, but this is one way I'm > considering and a public access to Operation.doOperation prevent that > particular way. > Ok, the answer is complex. First of all, why the public methods. There are two main reasons: 1>JAI allows to create operations directly using the form XXXDescriptor.Create(...) which prevents you from going through the service registry meachanism in case you already know the operation you are looking for. I tried to mimics the same thing in GeoTools. 2>In the current architectire of processing I found no way for changing the rendering hints once a processor has been instantiated. All the operations you could create using an instantiated processor would use the same hints. In case you would want to use different hints you would have to instantiate another processor. 3>I do not think the BufferedProcessor should be the default one. I understand that for your purposes it is a good option but for example in geoserver it was causing me some headaches first with not behing able to dispose coverages right away and second I would suggest using a SoftReference-based cache :-). In general my experience says that server side is better to load the operations registry once and then call the operations directly (especially for WCS having to recreate the Processor each time could be a performance loss even because I know in advance the operation I am looking for). Trying to sum up, I think that we could avoid returning a BufferedProcessor when doing AbstractProcessor.getInstance . To me it seemed strange that, having an implementation of DefaultProcessor, this method, whose javadoc says "Returns a default processor instance" instead of returning me a DefaultProcessor it returns a BufferedProcessor. I think that the default processor should be a DefaultProcessor which would not interfere with disposing coverages. If someone wants buffering we should create a buffered processor and just forget about the disposition. Just a suggestion though, I am sure we can do better. Thx, Simone. > > Cheers > > Martin > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys - and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Geotools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel > -- ------------------------------------------------------- Eng. Simone Giannecchini President /CEO GeoSolutions http://www.geo-solutions.it ------------------------------------------------------- ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
