[GEOT-5392] Mosaicking refactor #1253
Conversation
|
It seems there are conflicts |
|
Yep, I'll take a look this morning. Hopefully nothing too crazy. |
dvntucker
force-pushed
the
mosaicking_refactor
branch
2 times, most recently
from
983f0c0
to
67ffe61
Compare
|
The first two commits create a lot of new public API (visible to anyone using Geotools, not just inside the mosaic) apparently only for the sake of moving 3 classes in their own package. I recommend moving those classes back and return the visibility to package private. |
| * Responsible for creating subsets of the whole imagemosaic. Since there are frequently parts of | ||
| * a mosaic that need to be handled differently, a submosaic producer is responsible for handling | ||
| * those parts separately before they're added to the whole mosaic. For example a single dimension | ||
| * can be handled on its own before being |
There was a problem hiding this comment.
"before being..." incomplete javadoc
dvntucker
force-pushed
the
mosaicking_refactor
branch
from
3e687d1
to
8b55f10
Compare
|
This is now rebased off of the formatting changes. |
| @@ -137,14 +140,25 @@ | |||
|
|
|||
| OverviewsController overviewsController; | |||
|
|
|||
| private CoordinateReferenceSystem granuleCRS; | |||
|
|
|||
| public GeneralEnvelope coverageEnvelope; | |||
There was a problem hiding this comment.
The granule CRS is added and exposed via a getter, but this one is public?
The name is also confusing, what you really have is the granule/reader envelope, not the "coverage" one, indeed the descriptor does not deal with coverages at all.
There was a problem hiding this comment.
I'll change the name to be more accurate and take a look at the visibility to make sure it's correct.
There was a problem hiding this comment.
This has been renamed and been made private.
dvntucker
force-pushed
the
mosaicking_refactor
branch
from
8b55f10
to
6a9e38e
Compare
| SimpleFeature granuleFeature = granuleDescriptor.getOriginator(); | ||
| SimpleFeature templateFeature = templateDescriptor.getOriginator(); | ||
| for (SortBy sortKey : sortBy) { | ||
| Object incomingAttribute = granuleFeature |
There was a problem hiding this comment.
I'm not sure I understand why this is necessary. I though ensuring uniform CRS in the producer was enough. E.g., if I have all features in the same CRS but I'm sorting on date, do I really need to roll each feature in its own sub-mosaicker?
|
Just added a few more minor comments, everything looks otherwise pretty good. Looks like there are merge conflicts though. |
|
I'll squash and rebase. I also want to rename one of the packages (granule collector) before merging. |
Creating a GranuleCollectorFactory to abstract out creating granule collectors, moving actual mosaic creation logic into granule collector, eventually going to rename these classes to reflect their new responsibilities. Had to make some inner classes static to accomplish this. Moving GranuleCollector to its own package + all the necessary modifier changes for that Adding granule collector factory to raster layer response Indexer is now initialized when ImageMosaic is initialized. this was done so that we have access to its configuration while reading the image mosaic. A decent number of things shuffling around, in particular moving some subclasses out of RasterLayerResponse to top level classes for clarity (since these classes are now used outside of this class) Added the correct test results image Don't use default Operations object, instead create one with hints we may need. Removed some unused code that was missed Removed some unused imports Fixed a test case with a fixed number and the size check in RasterLayerResponse to account for nulls in the list Fixed an issue where indexer file wasn't getting initilized correctly Reformatting the code in Eclipse Renamed isHasAlpha and isDoTransparency to more sensible names - Changed ReprojectingConfigHandler to properly respect sort order - Relax requirement on property collectors in order to support collectors that don't need config - Added some defaults to properties collectors and granulecollector when HETEROGENEOUS_CRS is true - Rename coverageEnvelope to granuleEnvelope and make its access private - Removing unused granuleCRS - Rename old references to GranuleCollectors to SubmosaicProducers - Move Mosaicker into its own compilation unit. - Add another utility method to indexer utils for setting params on an Indexer - Remove submosaic producer artifacts from GranuleCatalog and CatalogConfiguration bean, now moved to RasterManager - Removed some debugging code that had been left in. Simplified the sorting Fixed a test that had been messed up by a mangled rebase
dvntucker
force-pushed
the
mosaicking_refactor
branch
from
6c6af02
to
2068bd9
Compare
|
Simplified the Sorting along the lines we discussed last week. |
|
@aaime Is this ready to be merged now? |
|
All good. I noticed at least one file with missing copyright header, I'll add it post-merge |
Lots of changes here. Refactored a lot of the Mosaicker into its own package.
Mainly putting this PR together for ease of review/commenting