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

New default legend symbol dimensions when no details are provided #6165

Merged
merged 11 commits into from
Dec 31, 2021

Conversation

PaulWessel
Copy link
Member

Description of proposed changes

While legend will plot exactly what you tell it to, when we now start to rely on the auto legend feature in -l there were several shortcomings. Once was the lack of a sense of the annotation font in placing the text (#6163) but also the lack of default parameters for most symbol when none is provided. And the few defaults that were there needed some adjustments as well. Before we accept these changes as is we should make sure they are adequate. Here is a plot of all the symbols that have auto-settings (historically, we had one for front but there really is no good way to know what kind of front (here squares) so it should probably be removed as auto, just like we cannot guess what quoted and decorated lines might look like:

autosize

What I mean by "no details" is simulated here by passing record like this (for circle):

S - A - red 0.5p - Star

[In -l, pen, colors, symbol type and label comes from the calling module, while size, dz1, and dx2 may be unset]

Here are issues to tackle or make decisions on:

  1. For ellipse, the rectangles, the wedge and vector sthere are named parameters at the top of pslegend.c that can be tinkered with (and recompiled). Are these the finalized settings, you think?
  2. Given the new equation for dx2 = size + W (symbol size plus letter width), most tests using -l will have slight failures
  3. Two old examples (03 and 07) had very small symbols relative to the label so they have been updated (and then fail)
  4. Any legend-related test script showing many types of symbols will fail due to the adjustments of parameters.

I think the proposed PR is vastly superior to what is in master, but some tinkering can happen, and I think for consistency we should remove the auto-square setting from front so it is like the other line-embellishment symbols that need more than "-" to do anything useful.

Once this PR can be merged I can start tackling the job of adding -l to grdvector and maybe velo.

Note: No PostScript updates are presented here so you can run the tests and compare the results to see the changes.

@PaulWessel PaulWessel added the enhancement Improving an existing feature label Dec 29, 2021
@PaulWessel PaulWessel added this to the 6.4.0 milestone Dec 29, 2021
@PaulWessel PaulWessel self-assigned this Dec 29, 2021
@maxrjones
Copy link
Member

Generally looks good, just a couple issues: the symbols do not appear in the region_u_i.sh or autolegendcolor.sh tests using this branch. I don't think that the rotated ellipses look better in leg3d.sh, legend3d.sh and legend.sh.

@PaulWessel
Copy link
Member Author

Darn it, I must had buggered something, the region_u_i.sh and autolegendcolor.sh was fine yesterday but now I see the problems. I will fix whatever that is but forced to take a break and go swimming. Grrrr.

@PaulWessel
Copy link
Member Author

Fixed.

@maxrjones
Copy link
Member

Thanks.

I think the ellipses could be improved. Consider the example below, where the ellipse conditions set with -S differ from the legend result:

gmt begin test png
    gmt plot @DSDP.txt -R100/160/-20/30 -JM5c -Se280/0.2/0.1 -Ggreen -Bafg -l"ellipse"
    gmt legend -DJCB+w2c+o0c/0.5c
gmt end show

test

@PaulWessel
Copy link
Member Author

Hm, it should have passed those args in - will have to debug.

@PaulWessel
Copy link
Member Author

Thanks, the latest revisions returns the ellipse to zero angle and does a better job parsing incoming parameters for the multi-arg symbols. Your ellipse case now works better, for instance. I think this is pretty good?

Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

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

Looks good. My preference would be to leave the failures as is for now in case there are adjustments made in the next legend PR(s).

@PaulWessel
Copy link
Member Author

OK, we will leave the failures for now.

@PaulWessel PaulWessel changed the title WIP New default legend symbol dimensions when no details are provided New default legend symbol dimensions when no details are provided Dec 31, 2021
@PaulWessel PaulWessel merged commit b5df9d8 into master Dec 31, 2021
@PaulWessel PaulWessel deleted the new-legend-spacings branch December 31, 2021 18:21
@maxrjones maxrjones added the add-changelog Add PR to the changelog label Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add-changelog Add PR to the changelog enhancement Improving an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants