Skip to content

556 geometry extrusion #616

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

Open
wants to merge 8 commits into
base: geosparql-1.3
Choose a base branch
from
Open

Conversation

ar-chad
Copy link
Collaborator

@ar-chad ar-chad commented Feb 28, 2025

@jabhay
Copy link
Collaborator

jabhay commented Mar 5, 2025

  • As discussed in the meeting, final step required is to add the requirement for the function in spec/sections/11-geometry_extension.adoc

@jabhay
Copy link
Collaborator

jabhay commented Mar 19, 2025

  • New requirement identified to add a function to enable extrusion by height: geof:extrudeToHeight

====
Implementations shall support the functions
<<Function: geof:extrude, `geof:extrude`>>
<<Function: geof:extrudeInTime, `geof:extrudeInTime`>>
Copy link
Collaborator

Choose a reason for hiding this comment

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

geof:extrudeToHeight needs to be added here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

----

The function http://www.opengis.net/def/function/geosparql/extrudeToHeight[`geof:extrudeToHeight`] returns a spatial Feature with a 3D geometric object that represents all Points in the extruded form of 2D `geometry` where the `height` marks the distance between points of the base `geometry` and ending points of the extruded form in space. Calculations are in the spatial reference system of `geometry`.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if "distance" may be a little vague here. What about something like where the 'height' indicates Z values for points at the "top" of the extruded 3D geometry?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mperry455 I corrected this accordingly to your suggestion. It is also aligned with H2GIS example http://www.h2gis.org/docs/1.5.0/ST_Extrude/

@jabhay jabhay requested a review from mperry455 April 2, 2025 20:26
@jabhay jabhay marked this pull request as draft April 2, 2025 20:28
@jabhay
Copy link
Collaborator

jabhay commented Apr 2, 2025

  • Need to consider and address the unit of measurement for height. The function without a parameter or unit in the name should use the unit of measurement of the CRS. @ar-chad is looking at what other technologies do.

@ar-chad
Copy link
Collaborator Author

ar-chad commented Apr 22, 2025

  • Need to consider and address the unit of measurement for height. The function without a parameter or unit in the name should use the unit of measurement of the CRS. @ar-chad is looking at what other technologies do.

The only implementation in open source databases I found was H2GIS, which adds the height to the Z coordinates. http://www.h2gis.org/docs/1.5.0/ST_Extrude/ They provide only example of that when Z is 0, but it doesn't always have to be and it does not have to be the same value for all Z coordinates of the base geometry.

@mperry455
Copy link
Collaborator

We proposed the following naming changes during the 4/30/2025 SWG meeting to avoid misunderstanding of function behavior based on function name. extrudeToHeight() could be confusing if initial Z coordinate values were non-zero.

  1. extrude() -> extrudeByLine()
  2. extrudeToHeight() -> extrude()
  3. extrudeInTime() -> no change

@ar-chad
Copy link
Collaborator Author

ar-chad commented May 5, 2025

We proposed the following naming changes during the 4/30/2025 SWG meeting to avoid misunderstanding of function behavior based on function name. extrudeToHeight() could be confusing if initial Z coordinate values were non-zero.

✅ 1. extrude() -> extrudeByLine()
✅ 2. extrudeToHeight() -> extrude()
✅ 3. extrudeInTime() -> no change

Done and ready for review a19e1fa

Copy link
Collaborator

@situx situx left a comment

Choose a reason for hiding this comment

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

Looks good to me

@situx situx marked this pull request as ready for review May 5, 2025 14:37
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.

4 participants