Skip to content

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

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

Closed
seisman opened this issue Mar 13, 2021 · 25 comments · Fixed by #4957, #4991 or #5265
Closed

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

seisman opened this issue Mar 13, 2021 · 25 comments · Fixed by #4957, #4991 or #5265
Assignees
Labels
bug Something isn't working

Comments

@seisman
Copy link
Member

seisman commented Mar 13, 2021

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 seisman added the bug Something isn't working label Mar 13, 2021
@seisman
Copy link
Member Author

seisman commented Mar 13, 2021

gmt basemap -Rg -JW15c -B -png map
v6.1.1 master
image image

Global maps using most projections all have the similar issue.

@seisman seisman changed the title Default axes in GMT modern theme? Potential issues with the GMT modern theme? Mar 13, 2021
@seisman seisman changed the title Potential issues with the GMT modern theme? Potential issues / breaking changes with the GMT modern theme? Mar 13, 2021
@seisman seisman changed the title Potential issues / breaking changes with the GMT modern theme? Potential issues or breaking changes with the GMT modern theme? Mar 13, 2021
@seisman
Copy link
Member Author

seisman commented Mar 13, 2021

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
Copy link
Member Author

seisman commented Mar 13, 2021

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
Copy link
Member Author

seisman commented Mar 13, 2021

This one is pretty bad:

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

image

@seisman
Copy link
Member Author

seisman commented Mar 13, 2021

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
Copy link
Member

I will look into the colorbar issue

@maxrjones
Copy link
Member

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
Copy link
Member

Reopening because #4957 only addressed #4955 (comment) and #4955 (comment)

@maxrjones
Copy link
Member

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
Copy link
Member

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
Copy link
Member

maxrjones commented Mar 14, 2021

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

@maxrjones
Copy link
Member

maxrjones commented May 24, 2021

@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 GenericMappingTools/pygmt#1287)? I remember discussing the pen thickness for subplots, but cannot remember if there was any discussion about insets.

Example 1:

Example 2:

@PaulWessel
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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 gmt_getpanel 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
Copy link
Member

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 gmt_getpanel 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants