faster rendering by looking at min and max scales before read#299
Conversation
Signed-off-by: sloob <sebastian.loob@ibykus.de>
|
started ci-build : https://ci.eclipse.org/udig/job/uDig-PR/30/ |
| // faster rendering if out of scale | ||
| GridCoverageRenderState state = getRenderState(getContext()); | ||
| double scale = state.context.getViewportModel().getScaleDenominator(); | ||
| if (scale < state.minScale || scale > state.maxScale) |
There was a problem hiding this comment.
Great, change looks good! Any chance to add a test case?
There was a problem hiding this comment.
IMH minScale / maxScale of state object might be null, if one or the other is not set.
Can you explaint how this change is related to line 232/ 227 where Rule of Style is compared to teh currentScale? I guess it is possible to define different rules for the same resource and one rule might fit where another doesn't.
So maybe it makes sense to move the style rule scale comparence before creating a coverage ..
There was a problem hiding this comment.
minScale / maxScale should never be null. The GeoTools implementation of Rule initializes these double values with 0.0 and Double.PositiveInfinity (see https://github.com/geotools/geotools/blob/master/modules/library/main/src/main/java/org/geotools/styling/RuleImpl.java).
The problem is, that the SLD do not have to include a RasterSymbolizer. If there is no RasterSymbolizer, a default Symbolizer is used (line 257) and the layer is rendered.
So using the GridCoverageRenderState with the feature type style seems to be the right way.
Signed-off-by: sloob <sebastian.loob@ibykus.de>
|
Started a CI-Build for this pull request again : https://ci.eclipse.org/udig/job/uDig-PR/32/ |
fgdrf
left a comment
There was a problem hiding this comment.
Great to have unit tests! I'm wondering about different formatings of java class files - they looking different and formatter rules might not applied everywhere. I guess the comments are easy to address and I'd to merge it afterwards till the end of 2018 ;)
| <module>org.locationtech.udig.libs.tests</module> | ||
| <module>org.locationtech.udig.location.test</module> | ||
| <module>org.locationtech.udig.render.feature.basic.test</module> | ||
| <module>org.locationtech.udig.render.gridcoverage.basic.test</module> |
There was a problem hiding this comment.
Looks like tabs are at the beginning of the lin rather that spaces, can we fix it?
| paramMap.put("name", "BasicGridCoverage2DFormat"); //$NON-NLS-1$//$NON-NLS-2$ | ||
| paramMap.put(AbstractGridFormat.READ_GRIDGEOMETRY2D.getName().toString(),new GridGeometry2D(new Rectangle(), new Rectangle())); | ||
|
|
||
| /* GridEnvelope2D gridEnvelope = new GridEnvelope2D(0, 0, mapDisplay.getWidth(), mapDisplay.getHeight()); |
There was a problem hiding this comment.
Do we need the commented lines on intial commit? Are you kind to remove these?
There was a problem hiding this comment.
Sorry for the bad code formatting. Hope everything is fine now :)
Signed-off-by: sloob <sebastian.loob@ibykus.de>
|
Looks good! Build-Job failure hasn't to do anything with your changes (https://ci.eclipse.org/udig/job/uDig-PR/38/) Many thanks again for this improvement |
…ontech#299) * faster rendering by looking at min and max scales before read Signed-off-by: sloob <sebastian.loob@ibykus.de>
To increase the performance of the GridCoverageReaderRenderer, raster rendering will only be performed if the map's current scale is within the min / max scale of the layer.
Signed-off-by: sloob sebastian.loob@ibykus.de