-
Notifications
You must be signed in to change notification settings - Fork 35.7k
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
build: fix kerning issue with macOS .dmg background image #17143
Conversation
I haven't seen the kerning issue locally, so @dongcarl can chime in. However I'd suggest restricting the changes here to just what fixes the issue; I assume just the font change? Could you also use a commit message like the PR title, and remove the other text i.e master (dcc6408): PR (ce4a7b4): |
The distance between the letters in the "after" image looks strange to me. |
Thanks for working on this. IMO "Arial Black" is the wrong font. The original font that was used in most other places (like bitcoincore.org) is "Helvetica Neue" (which is not available everywhere). I guess this also not fixes #16836. I think one option would be to always convert the @2x image and then downscale the bitmap image (instead of converting the SVG with to different DPI values). |
Let's investigate the kerning issue with upstream here: https://gitlab.gnome.org/GNOME/librsvg/issues/520, it seems to be more complicated than we think. Not a huge fan of down-scaling the bitmap image if we can avoid it. I believe we should restrict this PR to just changing "Tuffy" to "Arial". One thing you might want to do is remove Tuffy from our gitian descriptor for osx. |
Thanks for the feed back @dongcarl. |
@RandyMcMillan: you have change the font to "monospace",... which is probably not what we want. I suggest you change it to "Arial" and remove "Tuffy" from the OSX gitian descriptor. |
@RandyMcMillan Let's do the following:
Let's not touch the font size or letter spacing |
@dongcarl & @jonasschnelli - Thanks for the feedback and I appreciate your guidance while I get little more familiar with the macOS build processes for local and gitian builds. I think it may be useful to summarize the issue better... Firstly, after some research I have realized that it is was a mistake to use Arial (or Helvetica) font families in my original PR. The issue is, due to their licensing, they aren't available as Debian/Ubuntu packages and cannot be dynamically added to a Gitian build process (as far as I know). Arial License I don't expect this to change and I suspect Debian/Ubuntu and other Linux distros will avoid using any "font software" the doesn't have a compatible license in their packages. I have a good candidate for future macOS builds - fonts-liberation As you can see in the screen shot - fonts-liberation provides monospace, sans, and serif type faces. IMO this is a great choice moving forward & "Liberation Sans" looks very similar to fonts-tuffy anyway. :) I am currently configuring a VM so that I can test the whole macOS build process for local and Gitian. What I thought was going to be pretty simple edit has turned into much more. @dongcarl - I did see your post about librsvg - and I am in agreement that this is a macOS issue and not a library issue. I will also research this (Fix macOS Mojave Font Rendering Issue) more as well. @jonasschnelli - I will definitely add the correct font to the gitian build once some of these things get ironed out. Thanks again for your feedback, fonts-liberation was created to fill this exact niche with Linux users that wanted a Times, Arial or Courier font face. RM |
@RandyMcMillan After looking at it more carefully, I believe fonts-liberation might be a good choice. It seems to be available in most major distros and even homebrew has |
@dongcarl |
fonts-liberation is good choice for cross platform dev and it is available in packages for Linux and macOS. font-family within the .svg is set to "Liberation Mono" and monospace. If fonts-libertation isn't present on the build machine it will default too another monospaced font. Also in the background.svg file I changed the color of the arrow to match the text color of gray. custom_dsstore.py and macdeployqtplus contained redundant setttings for the bundler. These changes allow for the fancy.plist to be the only source of the icon positions, sizes as well as windowbounds settings. This is much easier to maintain and configure. I added settings for .fseventds and the .background folders as well - if the hidden files are shown by the system, they look orderly.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
@fanquake - I am researching a more prudent fix. There has to be a better way. |
I found a proper fix. I will submit in a new PR. |
Proper fix here --> #17273 |
Fixes: #16836
The kerning issue for the .dmg background image originates in the css styling that is embedded in the background.svg file itself.
This commit also updates the font family to Arial Black which is a common font on Macs.
This commit was tested on macOS Mojave 10.14.6 (18G103). The change uses standard CSS styling. The change enables normal font kerning but the fix is actually accomplish with the letter-spacing attribute.