GenericMappingTools / gmt

The Generic Mapping Tools
https://www.generic-mapping-tools.org
Other
843 stars 352 forks source link

Potential issues or breaking changes with the GMT modern theme? #4955

Closed seisman closed 3 years ago

seisman commented 3 years ago

Running this command:

gmt basemap -R0/1/0/1 -JX5c -B+t"ABC" -png map

GMT v6.1.1 doesn't plot any axes, only the title, but GMT master branch plots the 4 axes.

v6.1.1 master
image image

The changes are a big surprise to me and look like a bug.

seisman commented 3 years ago
gmt basemap -Rg -JW15c -B -png map
v6.1.1 master
image image

Global maps using most projections all have the similar issue.

seisman commented 3 years ago
gmt begin colorbar png
gmt makecpt -Cbatlow -T0/10/1
gmt basemap -R20/30/-10/10 -B
gmt colorbar
gmt end show

I know the east and north annotations and ticks are not plotted by default, but the color bar is not annotated, too.

v6.1.1 master
image image
seisman commented 3 years ago

The automatic annotation and frame intervals may not as good as v6.1.1.

gmt basemap -JM6c -R-125/-121/47/48 -B -png map
v6.1.1 master
image image
seisman commented 3 years ago

This one is pretty bad:

gmt coast -Rd -JKf12c -Bafg -Ggreen -png map

image

seisman commented 3 years ago
gmt basemap -R0/360/0/1 -JP5c -Baf -pdf map
basemap [NOTICE]: Given polar projection flip = 0, set MAP_FRAME_AXES = WrbNZ
gmt basemap -R0/90/0/1 -JP5c+a+t45 -Baf -pdf map
basemap [NOTICE]: Given polar projection flip = 0, set MAP_FRAME_AXES = WrbNZ
basemap [WARNING]: 2 annotations along the right border were skipped due to crowding.
basemap [WARNING]: Crowding decisions is controlled by MAP_ANNOT_MIN_SPACING, currently set to 23.0363p.
basemap [WARNING]: Decrease or increase MAP_ANNOT_MIN_SPACING to see more or fewer annotations, with 0 showing all annotations.
maxrjones commented 3 years ago

I will look into the colorbar issue

maxrjones commented 3 years ago

The colorbar issue is related to current.map.frame.draw in GMT being set to true inside gmt_set_undefined_axes. This is probably also the case with the first example (gmt basemap -R0/1/0/1 -JX5c -B+t"ABC" -png map). I am not sure how to solve this, however.

maxrjones commented 3 years ago

Reopening because #4957 only addressed https://github.com/GenericMappingTools/gmt/issues/4955#issuecomment-797863444 and https://github.com/GenericMappingTools/gmt/issues/4955#issue-830797657

maxrjones commented 3 years ago

I am wondering if it is possible to use a gentler scaling for MAP_ANNOT_MIN_SPACING if a defined annotation spacing is given. For example, while the warnings are helpful for knowing the reason for the missing annotations, it seems a bit problematic to require setting MAP_ANNOT_MIN_SPACING for a command like this: gmt basemap -Rg -JV12c -Ba15fg -png map or gmt coast -Rd -JN12c -Bxa15g -Bya30g -Dc -A10000 -Ggoldenrod -Ssnow2 -pdf GMT_robinson

PaulWessel commented 3 years ago

Yes, probably. I think these examples show a few issues:

  1. The JV example is missing many annotations while the JN is not so bad I think (all lats are and longs are too crowded)
  2. The JV example makes we wonder if the user sets a specific annotation interval we should not auto-compute tick and gridlines to exceed that spacing. It looks odd with the coarser gridlines than annotations, I think we usually see the opposite (denser gridlines, more spaced annotations.
  3. The -JN shows that ugly bump on the alignment for the 0 meridian. I guess I thought we had set the extend ticks here so they would all be on the same horizontal line?
maxrjones commented 3 years ago

Yes, probably. I think these examples show a few issues:

1. The JV example is missing many annotations while the JN is not so bad I think (all lats are and longs are too crowded)

2. The JV example makes we wonder if the user sets a specific annotation interval we should not auto-compute tick and gridlines to exceed that spacing.  It looks odd with the coarser gridlines than annotations, I think we usually see the opposite (denser gridlines, more spaced annotations.

This seems like a bug. From the cookbook:

In the case of automatic spacing, when the stride argument is omitted after g, the grid line spacing is chosen the same as the minor tick spacing; unless g is used in consort with a, then the grid lines are spaced the same as the annotations.

Edit: Perhaps I misunderstood that section. Regardless, I think if g is automatic with a or f custom, then it would be good to set g as equal to a or f (whichever is set).

3. The -JN shows that ugly bump on the alignment for the 0 meridian.  I guess I thought we had set the extend ticks here so they would all be on the same horizontal line?

The annotations are stepped, it is most obvious for the 0 annotation. I agree it looks bad.

image
PaulWessel commented 3 years ago

Adding this line to the non-oblique setups for WInkel, Robinson and thw two Eckerts:

if (!GMT->current.setting.map_annot_oblique_set) GMT->current.setting.map_annot_oblique |= GMT_OBL_ANNOT_NORMAL_TICKS;

gives nice plots. Will prep a PR soon but dinner with two actual guests (!) will interfere...

maxrjones commented 3 years ago

All of the remaining issues are related to these new lines in gmt_auto_frame_interval. I'm doing some trial and error checks to see if there is a better multiplier here, because 1.75 is too large.

#ifndef NO_THEMES
    if (GMT->current.setting.run_mode == GMT_MODERN && gmt_M_axis_is_geo (GMT, axis)) { /* Need more space for degree symbol and WESN letters not considered in the algorithm */
        if (strchr (GMT->current.setting.format_geo_map, 'F'))  /* Need more space for degree symbol and letter */
            d *= 1.75;
        else    /* Just more space for degree symbol */
            d *= 1.25;
    }
#endif
maxrjones commented 3 years ago

The automatic annotation and frame intervals may not as good as v6.1.1.

gmt basemap -JM6c -R-125/-121/47/48 -B -png map

v6.1.1 master image image

In addition to only having one western annotation, these figures show that fancy frames in 6.1.1 did not have tick marks by default, but do have tick marks after the themes branch was merged. I think the old default (no tick marks on fancy frames) should be maintained. What do you think, @PaulWessel?

PaulWessel commented 3 years ago

The tick marks were always there but drowned by that over-thick frame. So more of a mini-bug than feature in my mind. I think ticks should be shown as they are in modern. If anyone wants to maintain that "failure/feature" they can shrink their tick lengths to match the frame width.

maxrjones commented 3 years ago

The tick marks were always there but drowned by that over-thick frame. So more of a mini-bug than feature in my mind. I think ticks should be shown as they are in modern. If anyone wants to maintain that "failure/feature" they can shrink their tick lengths to match the frame width.

Thanks for the explanation. One more question regarding the maps from the last comment. gmt_auto_frame_intervals is called independently on the x- and y- axes. Then, the maximum interval from those calls is used both for the x- and y- axes. Can you explain the motivation for having the interval be identical on both axes? This causes some limitations for maps that span great longitudes but short latitudes or vice versa (e.g., the equidistant conic example: https://docs.generic-mapping-tools.org/6.1/cookbook/map-projections.html#equidistant-conic-projection-jd-jd).

Here is the example above if the auto-x and auto-y intervals are considered independently:

image

PaulWessel commented 3 years ago

I do not know - I think that was what Remko's old Fortran code did. Perhaps the joint/separate could be decided based on the differences in range? E.g r = (east-west)/{north-south); if (r < 0) r = 1.r; if (r > 2) do separate else joint?

maxrjones commented 3 years ago

I do not know - I think that was what Remko's old Fortran code did. Perhaps the joint/separate could be decided based on the differences in range? E.g r = (east-west)/{north-south); if (r < 0) r = 1.r; if (r > 2) do separate else joint?

Yes, I agree. I will open a PR with that type of solution - just wanted to check whether there was a historical reason for the current implementation before making any changes.

maxrjones commented 3 years ago

I do not know - I think that was what Remko's old Fortran code did. Perhaps the joint/separate could be decided based on the differences in range? E.g r = (east-west)/{north-south); if (r < 0) r = 1.r; if (r > 2) do separate else joint?

It turns out that adding something like this creates more problems than it solves. I simply reduced the first guess at the annotation interval in #4991 instead, which had been increased by 1.75 times in the themes branch.

maxrjones commented 3 years ago

@seisman, I think these are all handled now. Please reopen or create a separate issue if you notice more.

maxrjones commented 3 years ago

@PaulWessel, can you take a look at these examples to provide your opinion about whether the auto scaling for MAP_FRAME_PEN for inset boxes should be improved (motivated by https://github.com/GenericMappingTools/pygmt/pull/1287)? I remember discussing the pen thickness for subplots, but cannot remember if there was any discussion about insets.

Example 1:

Example 2:

PaulWessel commented 3 years ago

Just so I understand, the question here is if MAP_FRAME_PEN (which is used if there is no +ppen in -F) shall be

  1. Shrunk down based on inset size
  2. Kept at the size determined for the plot it is inserted into
  3. Escape shrinking altogether

Is this what you are getting at?

maxrjones commented 3 years ago

Just so I understand, the question here is if MAP_FRAME_PEN (which is used if there is no +ppen in -F) shall be

1. Shrunk down based on inset size

2. Kept at the size determined for the plot it is inserted into

3. Escape shrinking altogether

Is this what you are getting at?

Yes, this is my question. It seems now it is shrunk down based on inset size.

PaulWessel commented 3 years ago

The inset may plot a basemap and it certainly should be controlled by a scaled-down MAP_FRAME_PEN. However, the default for +p is a different thing taht should not worry about scalings, so that default should probably be the original unscaled one of 1.5p of whatever. You can always override with +p. So this line in _gmtgetpanel may need revision:

P->pen1 = GMT->current.setting.map_frame_pen;           /* Heavier pen for main outline */

and instead be

gmt_getpen (GMT, "thicker,black", &P->pen1 );

I think you can go ahead and test that with a WIP PR.

maxrjones commented 3 years ago

The inset may plot a basemap and it certainly should be controlled by a scaled-down MAP_FRAME_PEN. However, the default for +p is a different thing taht should not worry about scalings, so that default should probably be the original unscaled one of 1.5p of whatever. You can always override with +p. So this line in _gmtgetpanel may need revision:

P->pen1 = GMT->current.setting.map_frame_pen;         /* Heavier pen for main outline */

and instead be

gmt_getpen (GMT, "thicker,black", &P->pen1 );

I think you can go ahead and test that with a WIP PR.

OK, I will work on this.