Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raster processing and digital elevation models #890

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

bchapuis
Copy link
Member

@bchapuis bchapuis commented Sep 1, 2024

This pull request adds the ability to handle raster data and digital elevation models to produce vector contour and vector hillshades. Apache SIS is used to read raster data. While not used in core, a wrapper for GDAL has also been developped and may be used in workflow steps in the future. A few algorithms for converting raster data to vectors are also introduced.

The screenshot below depicts the convertion of a dem geotiff as terrarium png tiles, the creation of a raster hillshade, the creation of a vector hillshade, and the creation of vector contours.

Screenshot 2024-09-01 at 23 32 08

- The idea is to access gdal in a more idiomatic way.
- For instance AutoCloseable is used to release the underlying resources systematically.
- Objects ease the creation of options.
- The ContourTracer implements marching squares to trace or polygonize contours.
- The HillshadeCalculator computes raster hillshades.
- Martini is a port of mapbox's martini algorithm to java.
- The ChaikinSmoother enable the smoothing of linestring and polygons.
- Some tests and utilities enable the verification of the results.
- Improve the TileStore abstraction so that it can support both raster and vector tiles.
- Add some TileStore for dealing with raster tiles (geotiff, dem, cache, etc.).
- The server can now serve raster and vector tiles.
- New assets allow the previsualisation of DEM data
@bchapuis bchapuis requested a review from sebr72 September 1, 2024 22:27
Copy link

sonarcloud bot commented Sep 6, 2024

Copy link
Contributor

@sebr72 sebr72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked great, first time round. Thanks for the great work. Feel free to ignore my comments. One last suggestion: A legend informing of the height difference between 2 consecutive lines would be great.

},
sortOptions = false)
@SuppressWarnings("squid:S106")
public class DEM implements Runnable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could add some javadoc explaining DEM, maybe with external links
This comment applies to the Key classes (entry points of this new functionality)

import picocli.CommandLine.Option;

@Command(name = "serve", description = "Start a tile server that serves elevation data.")
public class Serve implements Callable<Integer> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a line to define purpose of the Server


var tileStoreSupplier = (Supplier<TileStore>) () -> tileCache;
var tileStoreSupplier = (Supplier<TileStore<ByteBuffer>>) () -> tileCache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A<B> = slippery slope but looks good and makes sense.

values);

return values;
} catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Limit the catch to declared exceptions (A | B |RuntimeException) ? (for consistency with line 78). This comment also applies to other places.

/** {@inheritDoc} */
@Override
public void close() throws Exception {
tileStore.close();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If close fail. Don't we want to cleanup anyway ?

var grid = geoTiffReader.read(tileCoord, 256, 4);

int increment = switch (tileCoord.z()) {
case 1 -> 2000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link (in javadoc) to some justification behind the reasoning of this mapping, rather than leaving "magic" numbers.


/**
* A geometry transformer that applies the Chaikin smoothing algorithm to the coordinates of a
* geometry.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a link explaining why/when to use this algorithm.

import javax.swing.*;
import org.locationtech.jts.geom.Geometry;

public class ContourRenderer {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc especially important since it has a main()

import org.locationtech.jts.geom.Polygon;
import org.locationtech.jts.geom.util.AffineTransformation;

public class MarchingSquareRenderer extends JPanel {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc especially since it has a main()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants