Skip to content
New issue

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

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

Already on GitHub? # to your account

Suggestion: Generalise bboxClip to support all GeoJSON object types #2813

Open
stevage opened this issue Jan 15, 2025 · 6 comments · May be fixed by #2814
Open

Suggestion: Generalise bboxClip to support all GeoJSON object types #2813

stevage opened this issue Jan 15, 2025 · 6 comments · May be fixed by #2814
Assignees

Comments

@stevage
Copy link
Collaborator

stevage commented Jan 15, 2025

There is not really an easy way to crop a featureCollection (or other geometry type) to a polygon or bbox.

There are times when I just want to do:

const newFC = turf.crop(oldFC, [xmin, ymin, xmax, ymax])

or

const newFC = turf.crop(oldFC, cropPolygon)

It's relatively easy to use intersect to crop Polygons, and booleanPointInPolygon to filter out Points, but I'm not even sure if there's a straightforward way to crop LineStrings currently.

@stevage
Copy link
Collaborator Author

stevage commented Jan 15, 2025

Ah, I have eventually discovered that bboxClip does some, but not all of this:

  • only handles bbox, not arbitrary polygons
  • does not handle Point/MultiPoint (easy to implement)
  • does not handle FeatureCollection (easy to implement)
  • has an ominous and vague warning: "May result in degenerate edges when clipping Polygons."

But I suspect clip is the better term rather than crop.

@stevage stevage changed the title Suggestion: crop Suggestion: Generalise bboxClip to support all GeoJSON object types Jan 15, 2025
@stevage stevage self-assigned this Jan 15, 2025
@smallsaucepan
Copy link
Member

Thanks @stevage. Would mask and difference do what you're hoping? mask can take a legit polygon.

Alternatively, would we be better off adding a new general purpose clip function (that takes whatever you throw at it) and deprecate bboxClip? I'm not sure the history behind only cropping with a bbox, so maybe there's a particular use case for that.

Your example would only need a minor mod:

const newFC = turf.clip(oldFC, turf.bboxPolygon([xmin, ymin, xmax, ymax]))

@stevage
Copy link
Collaborator Author

stevage commented Jan 16, 2025

I don't think mask and difference would achieve the same thing.

In any case, operations using a general polygon (as opposed to a bbox) are orders of magnitude slower, and mostly what I (and probably others) need is the bbox clip.

So I've gone and implemented this in #2814.

@smallsaucepan
Copy link
Member

Ah ok. From the initial post you mentioned there being no easy way to "crop ... to a polygon or bbox" so thought you wanted to add clipping with a polygon as well. Will take a look at #2814.

@stevage
Copy link
Collaborator Author

stevage commented Jan 17, 2025

Yeah, well, I did. But on further reflection, a general clipping function and a bbox clipping function are probably best kept as two separate concepts.

@stevage
Copy link
Collaborator Author

stevage commented Jan 31, 2025

Well, this is bizarre, I've just discovered issue #1565.

Apparently I made exactly the same suggestion in January 2019. I have no memory of this.

That issue got a bit derailed by the topic of spatial indexing.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants