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

mathtext: Increase space after spaced symbols #4872

Closed
wants to merge 3 commits into from

Conversation

zblz
Copy link
Member

@zblz zblz commented Aug 6, 2015

Currently, most spaced symbols (=,+,\times, etc) appear to have a bigger space in front of them, so it looks weird, specially in scientific notation of numbers (e.g., $2\times10^4$, which shows as '2 x10^4' rather than '2 x 10^4').

This PR adds 0.2 to the space afters these spaced symbols to solve this situation. IMO, the look of test images is improved, but I don't really understand why the previous setup (0.2 space on either side) resulted in asymmetrical spacing.

@mdboom
Copy link
Member

mdboom commented Aug 6, 2015

Thanks for the contribution -- it's always nice to have more people getting their hands dirty in the mathtext code. Indeed, it seems that the test images are improved for the most part. One place where it actually gets worse, though, is in subscripts, such as in mathtext_cm_21.png. There the space after the plus has increased to where it is no longer centered.

This got me looking at the root cause of the problem, and I think this may have been introduced by my recent changes to handling of advance width (i.e. the space after characters). When a "spaced symbol" is created, do_kern=False is passed. And do_kern, despite its name, not only handles kerning between characters but also adding the advance width to the "physical" width of the character. Since this extra space wasn't being added after the symbol, it created the appearance of the symbol not being centered.

I think the fix here is to just remove the do_kern=False. This will add the appropriate amount of space depending on the specific symbol, and seems to work well in both the regular and subscript case. I'm leaving on vacation tomorrow, so don't have time right now to verify all of the changed images and make a pull request etc. My hunch may be wrong, since I've only spot-checked some of the results. You can try it yourself, or I can do so when I get back.

@tacaswell tacaswell added this to the Color overhaul milestone Aug 6, 2015
@zblz
Copy link
Member Author

zblz commented Aug 6, 2015

As I mentioned above, I suspected I did not fully understand why the extra 0.2 was needed. Thanks for the do_kern=False suggestion, I will implement it and check the resulting images.

@zblz
Copy link
Member Author

zblz commented Aug 6, 2015

@mdboom Indeed, setting do_kern=True makes all spaced symbols look more symmetric. The only exception is after sub/superscripts, where it still seems that there is too much space after the sup/super, but I think that is a problem with the sub/super block, as it also shows a bit too much space when followed by a regular character.

@zblz
Copy link
Member Author

zblz commented Aug 27, 2015

@tacaswell, @mdboom: This is now conflicting with master because of the baseline images modified in #4873. In addition, some of the baseline images will conflict between this PR and #4887, so we should decide to merge one first and modify the other accordingly so we don't have to redo the baseline images several times.

@tacaswell
Copy link
Member

I think #4887 (which now also needs a rebase) should go in first as it is fixing a more glaring issue.

Another option is to combine them.

@zblz
Copy link
Member Author

zblz commented Aug 27, 2015

I agree on either #4887 going first and I'll rebase this, or combining them, whatever you prefer.

@zblz
Copy link
Member Author

zblz commented Aug 27, 2015

Actually, it might be better if you combine this into #4887: the changes here are very small and we avoid multiple versions of baseline images in master.

@mdboom
Copy link
Member

mdboom commented Aug 27, 2015

No objections from me to just combine this into #4887.

@zblz
Copy link
Member Author

zblz commented Aug 29, 2015

Combined into #4887, closing this PR.

@zblz zblz closed this Aug 29, 2015
@tacaswell tacaswell modified the milestones: Color overhaul, next major release (2.0) Oct 26, 2015
@QuLogic QuLogic modified the milestones: v1.5.0, 2.0 (style change major release) Oct 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants