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 bufferWithStyle #55

Merged
merged 7 commits into from
Oct 18, 2018
Merged

add bufferWithStyle #55

merged 7 commits into from
Oct 18, 2018

Conversation

jaakkor2
Copy link
Contributor

Wrap GEOSBufferWithStyle as bufferWithStyle similarly like GEOSBuffer as buffer.

I set default for mitreLimit 5.0, same as in the library
/~https://github.com/libgeos/geos/blob/8a9600bb2e617c788fdcdad8eb9e9045925bbd23/src/operation/buffer/BufferParameters.cpp#L35

Defaults for endCapStyle 1 (CAP_ROUND) and joinStyle 1 (JOIN_ROUND).

@@ -324,6 +324,9 @@ end
buffer(ptr::GEOSGeom, width::Real, quadsegs::Integer=8, context::GEOSContext = _context) =
GEOSBuffer_r(context.ptr, ptr, width, Int32(quadsegs))

bufferWithStyle(ptr::GEOSGeom, width::Real, quadsegs::Integer=8, endCapStyle::Integer=1, joinStyle::Integer=1, mitreLimit::Real=5.0, context::GEOSContext = _context) =
GEOSBufferWithStyle_r(context.ptr, ptr, width, Int32(quadsegs), Int32(endCapStyle), Int32(joinStyle), mitreLimit)

# enum GEOSBufCapStyles
# enum GEOSBufJoinStyles
Copy link
Member

Choose a reason for hiding this comment

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

Could you perhaps add these as well, and use them instead of integers as input argument type for bufferWithStyle?
That way it will be easier for users to find out about the different possible styles.

See for instance this example:
/~https://github.com/JuliaGeo/GDAL.jl/blob/38ccd2d37664673b81b1a86e9947bd5fe9a2f6d2/src/common.jl#L102
/~https://github.com/JuliaGeo/GDAL.jl/blob/017bf6b8492dcd2186ced297076b283c3591d798/src/gdal_h.jl#L14

Copy link
Member

@visr visr left a comment

Choose a reason for hiding this comment

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

Thanks, a welcome addition! Could you also add a test, to show it is working?

@jaakkor2
Copy link
Contributor Author

Sure, will add tests later today.

@jaakkor2
Copy link
Contributor Author

Tests added. Checks are passing. Should be ready to go. A matter of taste if CAP_* and JOIN_* should be exported, now they are not.

@yeesian
Copy link
Member

yeesian commented Oct 18, 2018

I'm okay with not exporting the enums for now. Thanks!

@yeesian yeesian merged commit 1485489 into JuliaGeo:master Oct 18, 2018
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.

3 participants