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

Add tiling module and CLI options #41

Merged
merged 3 commits into from
Nov 1, 2024
Merged

Add tiling module and CLI options #41

merged 3 commits into from
Nov 1, 2024

Conversation

clausnagel
Copy link
Member

This PR adds a tiling module and API as well as corresponding tiling options to the CLI command.

@yaozhihang
Copy link
Member

The following error was produced while running the import/export CLI command. The same command works well with the main branch. Is there anything I missed?

[22:27:26 ERROR] Failed to connect to the database.
[22:27:26 ERROR] org.citydb.database.DatabaseException: No database adapter available for the database 'PostgreSQL'.
[22:27:26 WARN] citydb execution failed.

@clausnagel
Copy link
Member Author

Hm, cannot reproduce. Maybe you can debug?

@clausnagel
Copy link
Member Author

Test again. I fixed the dependencies.

@yaozhihang
Copy link
Member

Works now. What should the json config look like? for example in case of matrix tiling like this?

"tiling": {
  "schema": {
    "columns": 2,
    "rows": 2
    }
 }

@yaozhihang
Copy link
Member

The TileMatrix is always based on the database srs. Shouldn't the matrix based on the input SRS (e.g., WGS84) of the DimensionSchema?

Copy link
Member

@yaozhihang yaozhihang left a comment

Choose a reason for hiding this comment

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

Very nice PR. One important remark is about the CRS of the TileMatrix.

@clausnagel
Copy link
Member Author

clausnagel commented Oct 31, 2024

Works now. What should the json config look like? for example in case of matrix tiling like this?

You have to add the type attribute to identify the tiling scheme. And be careful to use scheme not schema. For example:

"tiling":{
	"scheme":{
		"type":"Dimension",
		"height":{
			"value":1000.0
		},
		"width":{
			"unit":"km",
			"value":1.0
		}
	}
}

or

"tiling":{
	"scheme":{
		"type":"Matrix",
		"columns":4,
		"rows":4
	}
}

@clausnagel
Copy link
Member Author

The TileMatrix is always based on the database srs. Shouldn't the matrix based on the input SRS (e.g., WGS84) of the DimensionSchema?

The TileMatrix produces the tiles which are used as spatial filter against the database. I think this should be done in the CRS of the database to avoid distortions. What would be the advantage of doing this in WGS84?

@yaozhihang
Copy link
Member

yaozhihang commented Oct 31, 2024

The TileMatrix is always based on the database srs. Shouldn't the matrix based on the input SRS (e.g., WGS84) of the DimensionSchema?

The TileMatrix produces the tiles which are used as spatial filter against the database. I think this should be done in the CRS of the database to avoid distortions. What would be the advantage of doing this in WGS84?

Tiling based on the input SRS can provide a more generic way. One use case is the current VIS-Exporter, which ist based on the tiling in WGS84.

Another case is for example the CRS used by the iUR data, which is a Geographic CRS. Tiling based an input projected CRS can avoid distortions if we want to export the data in a projected CRS.

@yaozhihang
Copy link
Member

Works now. What should the json config look like? for example in case of matrix tiling like this?

You have to add the type attribute to identify the tiling scheme. And be careful to use scheme not schema. For example:

"tiling":{
	"scheme":{
		"type":"Dimension",
		"height":{
			"value":1000.0
		},
		"width":{
			"unit":"km",
			"value":1.0
		}
	}
}

or

"tiling":{
	"scheme":{
		"type":"Matrix",
		"columns":4,
		"rows":4
	}
}

cool, works well.

@clausnagel
Copy link
Member Author

The TileMatrix is always based on the database srs. Shouldn't the matrix based on the input SRS (e.g., WGS84) of the DimensionSchema?

The TileMatrix produces the tiles which are used as spatial filter against the database. I think this should be done in the CRS of the database to avoid distortions. What would be the advantage of doing this in WGS84?

Tiling based on the input SRS can provide a more generic way. One use case is the current VIS-Exporter, which ist based on the tiling in WGS84.

Another case is for example the CRS used by the iUR data, which is a Geographic CRS. Tiling based an input projected CRS can avoid distortions.

Tiling works in case the user provides the tiling extent in the database CRS or in any other CRS. The database CRS is only used internally for computing the tiles. These are the steps of the VIS export implementation of the old Importer/Exporter:

  1. The user provides the tiling extent in the GUI in any CRS, e.g. WGS84
  2. The VIS exporter transforms the extent into the database CRS to calculate the tile matrix (columns, rows)
  3. The VIS exporter transforms the tile extent back to WGS84
  4. The VIS exporter iterates over all tiles. Because of step 3, the extent of each tile is also in WGS84
  5. To use the tile as spatial filter, the VIS exporter transforms the extent of each tile back to the database CRS
  6. The VIS exporter receives features from the database in the database CRS. To check whether a feature is on the tile, the center point of the feature's envelope is used. Since this center point is in the database CRS but the tile extent in WGS84, the center point must again be transformed to WGS84 to perform the check.

I think this is not a good example. The new API is much clearer in my opinion.

@yaozhihang
Copy link
Member

The new API is clearer. Just hope it to be more generic to support tiling matrix with different CRS. like the old API
https://github.com/3dcitydb/importer-exporter/blob/master/impexp-core/src/main/java/org/citydb/core/query/filter/tiling/Tiling.java#L90

@clausnagel
Copy link
Member Author

Merging now. The extents of both the TileMatrix and of individual tiles can be transformed to any other CRS using the transform method of GeometryAdapter.

@clausnagel clausnagel merged commit ef478f0 into main Nov 1, 2024
4 checks passed
@clausnagel clausnagel deleted the feature-tiled-export branch November 1, 2024 07:27
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