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

Adding plot margin via psconvertoptions fails in modern mode #5582

Closed
PaulWessel opened this issue Aug 2, 2021 · 13 comments
Closed

Adding plot margin via psconvertoptions fails in modern mode #5582

PaulWessel opened this issue Aug 2, 2021 · 13 comments
Assignees
Labels
bug Something isn't working

Comments

@PaulWessel
Copy link
Member

This example shows the problem:

#!/bin/bash
gmt begin
	gmt set PS_MEDIA 4ix4i
	gmt figure map1 jpg A+m0.2i
	echo 0 0 | gmt plot -R-2/2/-2/2 -JX4i -SC1i -Gred -B0 -X0 -Y0 --MAP_FRAME_PEN=faint
	gmt figure map2 eps
	echo 0 0 | gmt plot -R-2/2/-2/2 -JX4i -SC1i -Gred -B0 -X0 -Y0 --MAP_FRAME_PEN=faint
gmt end
gmt psconvert map2.eps -Tj -A+m0.2i -P
file map1.jpg
file map2.jpg

Here, map1.jpg does not have the 0.2i margin added despite giving A+m0.2i to the figure command, whereas map2.jpg gets it by running psconvert separately. So the problem must lie somewhere in how the figure argument is eventually passed to psconvert via gmt end. It is the same with gmt begin.

PS. This continues my CostCo saga for metal prints of large maps: After receiving a flawed print and talking to CostCo support I learned that they have an undocumented "bleed" of 0.125" on all sides, hence if you wish to defeat their auto-cropping you must add the bleed up front, so that a 36x24 inch image at 300 dpi actually should be submitted as 10875x7275 pixels (with 75 pixels, nicely divisible by 2 (not) is cropped). Hence, I would like to add A+mbleed to my script but alas that fails.

@PaulWessel PaulWessel added the bug Something isn't working label Aug 2, 2021
@PaulWessel PaulWessel self-assigned this Aug 2, 2021
PaulWessel added a commit that referenced this issue Aug 3, 2021
See #5582 for background.  This minor change allows the margins and other settings such as +g, +f, etc.
@PaulWessel
Copy link
Member Author

Perhaps this is a better description of the problem. The current -A has a mix of crop-related and non-crop-related modifiers. A backwards-compatible change that would clarify matters might introduce a new option -N which takes the non-crop-related modifiers:

  1. -A[+r][+u]
  2. -N[+ffade][+gfill][+mmargins][+p[pen]][+s[m]|Swidth[/height]]

This solution eliminates the awkward -A+n where +n negates the crop implied by -A in the first place...
This clean separation of cropping in the -A means that -A controls whether or not we crop to BB or use the media size, and then -N controls any expansion of that box, scaling, and fill/outline of box.

It would be easy to detect this new format since the revised -A is a subset of the current -A and -N is brand new. The default PS_CONVERT setting in modern mode would remain a "A" (we crop to BB) and in my case of wanting to retain the media size and expand I would pass "N+m0.2i" instead of "A".

Let me know if you see any problems here, especially @joa-quim.

@maxrjones
Copy link
Member

Perhaps this is a better description of the problem. The current -A has a mix of crop-related and non-crop-related modifiers. A backwards-compatible change that would clarify matters might introduce a new option -N which takes the non-crop-related modifiers:

1. **-A**[**+r**][**+u**]

2. **-N**[**+f**_fade_][**+g**_fill_][**+m**_margins_][**+p**[_pen_]][**+s**[**m**]|**S**_width_[/_height_]]

This solution eliminates the awkward -A+n where +n negates the crop implied by -A in the first place...
This clean separation of cropping in the -A means that -A controls whether or not we crop to BB or use the media size, and then -N controls any expansion of that box, scaling, and fill/outline of box.

It would be easy to detect this new format since the revised -A is a subset of the current -A and -N is brand new. The default PS_CONVERT setting in modern mode would remain a "A" (we crop to BB) and in my case of wanting to retain the media size and expand I would pass "N+m0.2i" instead of "A".

Let me know if you see any problems here, especially @joa-quim.

This seems good to me, though it could be even cleaner to have one for crop, one for extend, and one for design. Downside is that there are not many more letters available.

  • -A[+r][+u]
  • -I[+mmargins][+s[m]|Swidth[/height]]
  • -N[+ffade][+ggill][+ppen]

@PaulWessel
Copy link
Member Author

There already is a -I option in there but it seems dated now (gs at 9.54) and could be deprecated (we can still easily detect a modifier-less -I to mean to old thing).

@PaulWessel
Copy link
Member Author

But I like the cleaner separations of functions.

@PaulWessel
Copy link
Member Author

And, it keeps each option shorter and less complicated.

@PaulWessel
Copy link
Member Author

We need some feedback from @joa-quim on the need for -I. I see it affects grayscales. Is it still relevant in 9.54 and beyond?

@maxrjones
Copy link
Member

I know that GMT's deprecation policy is backwards compatibility forever, but do warnings get added for changes such as these? I am wondering to find out how the wrappers can know about changes such as these, apart from rigorous tracking of the gmt repository.

@PaulWessel
Copy link
Member Author

We are probably not doing this in a rigorous way. We use the gmt_M_compat_check (GMT, this_version) to allow deprecated syntax to work if current GMT version equals or exceeds this_version, else we get an error of no such option or similar. We are supposed to add a message via GMT_MSG_COMPAT, e.g.,

xyz2grd.c: GMT_Report (API, GMT_MSG_COMPAT, "Option -A is deprecated; use -Az instead.\n");

but I am not sure we always do, and those are only seen if you use -Vc.

@PaulWessel
Copy link
Member Author

Perhaps we should search for those gmt_M_compat_check and make sure there is a COMPAT message, and vice versa.

@joa-quim
Copy link
Member

joa-quim commented Aug 3, 2021

We added that -I because of a user brought it out in a kind of bug report and that -I was the response to it. I have no idea if that issue can still strike, though I suspect not.

@PaulWessel
Copy link
Member Author

Would you be OK with a splitting of -A over -A, -N, -I?

@joa-quim
Copy link
Member

joa-quim commented Aug 3, 2021

Yes. Would prefer to avoid -I because it's a nearly global option but we have little choice.

PaulWessel added a commit that referenced this issue Aug 6, 2021
* Let gmt end better handle -A args

See #5582 for background.  This minor change allows the margins and other settings such as +g, +f, etc.

* Implement -A -I -N scheme

Also fixed errors is the use of GMT_Report.

* Fix typos

* Update psconvert.rst

* Update psconvert.rst

* Remove notice
@PaulWessel
Copy link
Member Author

Closed as implemented

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
Development

No branches or pull requests

3 participants