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

Polygon holes filling is tricky #3135

Closed
joa-quim opened this issue Apr 19, 2020 · 6 comments · Fixed by #3139
Closed

Polygon holes filling is tricky #3135

joa-quim opened this issue Apr 19, 2020 · 6 comments · Fixed by #3139

Comments

@joa-quim
Copy link
Member

It seems that adding -Ph to headers is not enough. It needs that the hole polygon has an orientation contrary to its parent. But that's not all, if origin is not at LowerLeft (for rectangles) wholes are painted too regardless if its orientation.

cat << EOF > quads.dat
> -Gred CCW
0  0
5  0
5  10
0  10
0  0
> -Ph  CW
2  1
2  3
4  3
4  1
2  1
> -Ph  CCW
2  4
4  4
4  6
2  6
2  4
> -Ph CCW origin TopLeft
2  9
2  7
4  7
4  9
2  9
> CW -Gblue
5  0
5  10
10 10
10 0
5  0
> -Ph  CW
7  1
7  3
9  3
9  1
7  1
> -Ph  CCW
7  4
9  4
9  6
7  6
7  4
EOF
gmt plot quads.dat -JX12c -Ba2f1WSen -R-0.5/10.5/-0.5/10.5 -Gred -W1p -png map

map

@PaulWessel
Copy link
Member

Yes, I think that is true. Software that writes polygons with holes will ensure the orientation is consistent (e.g., inside is to the left as you walk around the perimeter). I am pretty sure shapefiles do this correctly. We do not reverse holes as we assume they are placed correctly. if you ensure this orientation, does it then work?

@joa-quim
Copy link
Member Author

Curiously there was a thread in GDAL list about holes and Even said that GDAL does not care about the parent-child orietation. And PostGIS doesn't care either (and plots holes correctly).

But aside from the orientation there seems to be another issue that looks like a bug. Third square on reds should be transparent bit it's not because origin is not the same (TopLeft instead of BottomLeft).

@PaulWessel
Copy link
Member

The 3rd square is CCW like the parent so no hole.

@PaulWessel
Copy link
Member

If we must, we have code in gmtspatial to determine handedness and could reverse holes etc.

@joa-quim
Copy link
Member Author

True about 3rd. I got lost with so many looks at the Clock.
I thought on that gmtspatial option. Worth trying (at polygon plotting time) though I'm curious in what will happen from Julia. The thing is, for those cases, from Julia I don't make a copy of data coming from GMT, just a direct memory access. Don't remember what happens when same data is sent back to GMT and it changes it because of bad handedness (kind of 1D transpose).

@PaulWessel
Copy link
Member

It seems we can bury something deep in gmt_geo_polygons. It plots the parent polygon (the usual case is to plot a single polygon), but then we loop over all the holds that might follow. I think we can (a) detect that there are holes, (b) then determine the handedness of the parent, and (c) get handedness of each hold and reverse if matches the parent. I will give that a try.

PaulWessel added a commit that referenced this issue Apr 21, 2020
Do not trust the incoming data.  For polygons with holes that need to be filled, we ensure the holes have a different handedness than the parent, reversing the child if needed.  Closes #3135.
PaulWessel added a commit that referenced this issue Apr 21, 2020
Do not trust the incoming data.  For polygons with holes that need to be filled, we ensure the holes have a different handedness than the parent, reversing the child if needed.  Closes #3135.
PaulWessel added a commit that referenced this issue Apr 23, 2020
* doc: Fix references to GMT defaults (#3132)

* Auto-generate gmt_moduleinfo.h

* Fix new bug in filterwidth parsing for median filter (#3137)

I recently introduced this bug.

* Better check if no barwidth given in -D (#3138)

With nothing we are thrown into the deprecated parsing and it did not check if nothing was given.  Closes #3136.

* Ensure hoels of polygons have correct handedness (#3139)

Do not trust the incoming data.  For polygons with holes that need to be filled, we ensure the holes have a different handedness than the parent, reversing the child if needed.  Closes #3135.

* Make info go to GMT_MSG_INFORMATION

* Fix extending the grid in grdcut (#3142)

* Fix extending the grid in grdcut

When grdcut -N is used we can extend the grid beyond its domain and fill in the new areas with NaNs.  However, if the new region was not kosher and changed the domain by an integer number of pixels our algorithm for filling in NaNs got tricked and the result was a new grid with a bunch of zero nodes.  This PR checks for this case and adds a sanity check as well.

* Update grdcut.c

* Remember to also flip curves with -N with -A is used (#3145)

Needed to flip the x and y coordinates when plotting the pdf curves when -A is used.  Closes #3141.

* Fix wrong string comparison when converting berween GMT and GDAL ellipsoid names (#3143)

* Let set_tbl_minmax make hole connections (#3146)

Since datasets may come to us via external interfaces we need to look for polygons with holes and hook things up.

* Major revision of grd2kml (#3144)

* Update grd2kml.c

Fix the quadtree.

* Update grdvector_common.rst_

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* FInalize module

* Remove junk update docs

* Update grd2kml.c

* Add -S option

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Allow JPGs vs PNGs

* Eliminate -Q

* Handle grdiline grids

* Fix both grdcut and grd2kml use of it.

* Fix contours

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Allow variable pen widths

It is best to scale down pen widthd for lower levels as otherwise they are just too thick.

* Let pen scale/limit be set via modifier +s

* Same -A as gmt2kml

* Update grdcontour.c

* Fix my bug in grdcontour (#3149)

I forgot to set the id of the pen before using it...

* Limit -J to x|X for histogram (#3148)

It currently prints out all the map projections.  Closes #3140.

* Relegate no contour message back to information (#3150)

This was elevated back when -V was required to get any information at all.  Since warnings are now default, we move this to information.  I also added the same information message to pscontour.

* Add the missing entries

Co-authored-by: Paul Wessel <[email protected]>
Co-authored-by: Joaquim Luis <[email protected]>
PaulWessel added a commit that referenced this issue Apr 25, 2020
* Eliminate duplications

* Add title

* Update gmt_core_module.c

* Redo everything

* Update CMakeLists.txt

* names...

* Update gmt_api.c

* Update src/CMakeLists.txt

Co-Authored-By: Dongdong Tian <[email protected]>

* Update CMakeLists.txt

* Update gmt_modules.h

* Only shared libraries supported

* Eliminate function pointer in struct

* Change common_*.[ch] to gmt_*.[ch]

* Auto-generate gmt_moduleinfo.h (#3134)

* doc: Fix references to GMT defaults (#3132)

* Auto-generate gmt_moduleinfo.h

* Fix new bug in filterwidth parsing for median filter (#3137)

I recently introduced this bug.

* Better check if no barwidth given in -D (#3138)

With nothing we are thrown into the deprecated parsing and it did not check if nothing was given.  Closes #3136.

* Ensure hoels of polygons have correct handedness (#3139)

Do not trust the incoming data.  For polygons with holes that need to be filled, we ensure the holes have a different handedness than the parent, reversing the child if needed.  Closes #3135.

* Make info go to GMT_MSG_INFORMATION

* Fix extending the grid in grdcut (#3142)

* Fix extending the grid in grdcut

When grdcut -N is used we can extend the grid beyond its domain and fill in the new areas with NaNs.  However, if the new region was not kosher and changed the domain by an integer number of pixels our algorithm for filling in NaNs got tricked and the result was a new grid with a bunch of zero nodes.  This PR checks for this case and adds a sanity check as well.

* Update grdcut.c

* Remember to also flip curves with -N with -A is used (#3145)

Needed to flip the x and y coordinates when plotting the pdf curves when -A is used.  Closes #3141.

* Fix wrong string comparison when converting berween GMT and GDAL ellipsoid names (#3143)

* Let set_tbl_minmax make hole connections (#3146)

Since datasets may come to us via external interfaces we need to look for polygons with holes and hook things up.

* Major revision of grd2kml (#3144)

* Update grd2kml.c

Fix the quadtree.

* Update grdvector_common.rst_

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* FInalize module

* Remove junk update docs

* Update grd2kml.c

* Add -S option

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Allow JPGs vs PNGs

* Eliminate -Q

* Handle grdiline grids

* Fix both grdcut and grd2kml use of it.

* Fix contours

* Update grd2kml.c

* Update grd2kml.c

* Update grd2kml.c

* Allow variable pen widths

It is best to scale down pen widthd for lower levels as otherwise they are just too thick.

* Let pen scale/limit be set via modifier +s

* Same -A as gmt2kml

* Update grdcontour.c

* Fix my bug in grdcontour (#3149)

I forgot to set the id of the pen before using it...

* Limit -J to x|X for histogram (#3148)

It currently prints out all the map projections.  Closes #3140.

* Relegate no contour message back to information (#3150)

This was elevated back when -V was required to get any information at all.  Since warnings are now default, we move this to information.  I also added the same information message to pscontour.

* Add the missing entries

Co-authored-by: Paul Wessel <[email protected]>
Co-authored-by: Joaquim Luis <[email protected]>

* cmake: Simplify the regex for building gmt_moduleinfo.h (#3153)

* Eliminate old gmt_supplements_module.[ch]

Made gmt_suppl_glue.[ch].in after gmt_glue.[ch] and gmt_suppl_modules.h after gmt_modules.h and the contents of the old gmt_supplements_module.h.  I include gmt_suppl_modules.h from gmt_dev.h and gmt_suppl_glue.h from gmt_api.c.  I updated the CMakefile to configure the two new files.  All compiles but currently the gmt_suppl_glue.[ch] file says "core" instead of "suppl" in the function names.

* Update src/CMakeLists.txt

Co-Authored-By: Dongdong Tian <[email protected]>

* Update CMakeLists.txt

* Build gmt_suppl_moduleinfo.h (#3154)

* Eliminate gmt_modules.h and gmt_suppl_modules.h

* Update CMakeLists.txt

Need correct name

* Ignore modules.h files

* Add EXTERN_MSC to all module functions

* Build gmt_suppl_moduleinfo.h

* Finalize new api build via Cmake

Local tests run fine on macOS

* Use GMT_SUPPL_LIB_NAME Cmake variable instead of hardwired "supplements"

* Update cmake/ConfigDefault.cmake

Co-Authored-By: Dongdong Tian <[email protected]>

* Eliminate glue include files (#3159)

* Eliminate glue include files

Since the glue functions are not called as is anywhere, their names are only assembled via strings and obtained via a pointer from dlsym, there is no purpose to gmt_glue.h and gmt_suppl_glue.h.  Just added EXTERN_MSC to the functions in gmt_*glue.c instead.

* Eliminate suppl_glue in repo

* Update comments for BUILD_SHARED_LIBRARIES

* remove unused file and macro

* Update CMakeLists.txt

* Delete gmt_make_module_src.sh

* Update CMakeLists.txt

* Update MB-system files

* CI: Remove gmt_make_module_src.sh from CI testing (#3160)

gmt_make_module_src.sh is no longer used and was removed in branch new-api-lookup.

* Update gmt_glue.c.in

* Update gmt_glue.c.in

* Rename Gmt_libinfo struct type to GMT_LIBINFO

Stick with our conventions

* cmake: Extract gmtread and gmtwrite module information from source codes directly (#3163)

* Rename vars (#3164)

* Rename SUPPL_* toSHARED_LIB_*

* Update gmt_glue.c.in

* Update src/CMakeLists.txt

Co-Authored-By: Dongdong Tian <[email protected]>

* More documentation

* Move settings closer to library building

Co-authored-by: Dongdong Tian <[email protected]>

* GMT_SUPPL_LIB_NAME was defined on Windows only (#3166)

* cmake: Make the CMake file more readable (#3167)

Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Joaquim Luis <[email protected]>
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 a pull request may close this issue.

2 participants