Page MenuHome
Differential D12802

UI: Default Fonts Folder for Mac and Linux
ClosedPublic
Actions

Authored by Harley Acheson (harley) on Oct 8 2021, 11:06 PM.

Details

Reviewers
Yevgeny Makarov (jenkm)
Campbell Barton (campbellbarton)
Group Reviewers
User Interface
Commits
rB02a9377da0da: UI: Default Fonts Folder for Mac and Linux
Summary

Initial defaults for userdef->fontdir for Mac and Linux.

The following report - https://developer.blender.org/T91919 - shows a Linux user clicking a "browse" button to select a font but is shown their root drive. This is quite confusing for new users who have not set a custom Fonts path (Preferences / File Paths).

Once we know that users at least start with some default path we can then start opening the "select font" browser there in thumbnail view.

Note this is only used for new installations and never overwrites a user-set value. A good way to test this is just to "Load Factory Preferences".

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Oct 8 2021, 11:06 PM
Harley Acheson (harley) created this revision.
Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).Oct 9 2021, 2:51 AM
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/blenkernel/intern/appdir.c
235

No need for the "else", since there's a return in the "if"

Richard Antalik (ISS) mentioned this in T91919: Fonts not showing up in VSE 3.0.0-alpha build.Oct 15 2021, 12:07 AM
Harley Acheson (harley) updated this revision to Diff 44046.Oct 27 2021, 10:10 PM

@Hans Goudey (HooglyBoogly) - No need for the "else", since there's a return in the "if"

Yikes, you're right of course. Fixed.

Yevgeny Makarov (jenkm) added a subscriber: Yevgeny Makarov (jenkm).EditedOct 27 2021, 10:59 PM

On Mac, the default system fonts are in /System/Library/Fonts/.

User-installed fonts in:

const char *home = BLI_getenv("HOME");
BLI_snprintf(dir, FILE_MAXDIR, "%s/Library/Fonts/", home);

At least on my Mac. But "/Library/Fonts/" also exists.

UPD: there is a preferences to install fonts for User or All Users.
So it's ~/Library/Fonts/ or /Library/Fonts/ respectively.

Harley Acheson (harley) added a comment.Oct 27 2021, 11:39 PM

@Yevgeny Makarov (jenkm) wrote:- On Mac, the default system fonts are in /System/Library/Fonts/

So if you had to pick a single folder to be the default that would be it?

I realize that we can have multiple folders in the File Browser system list, but this is to make sure users don't start with an empty entry at Preferences / File Paths.

It can be any path you prefer but should probably be where the most are and is most likely to exist.

Yevgeny Makarov (jenkm) added a comment.Oct 28 2021, 12:04 AM

/System/Library/Fonts/ - will definitely have standard fonts.
~/Library/Fonts/ - is initially empty, but by default custom fonts will be added here.
/Library/Fonts/ - the least likely scenario.

Probably the second one. It may be empty, but... uh...
and the system looks like this:

Harley Acheson (harley) updated this revision to Diff 44049.Oct 28 2021, 12:40 AM
Harley Acheson (harley) added a reviewer: Yevgeny Makarov (jenkm).

@Yevgeny Makarov (jenkm) - Ah... so the system folder is meant just for fonts used by the system itself and user fonts go into ~/Library/Fonts/

I updated the patch for this path instead. And added you as a reviewer so you can be the official tester on Mac. ;-)

and the system looks like this:

Hmm... I'm a little curious about how https://developer.blender.org/D12032 looks with that folder. I think some like Arabic, Braille, Emoji will show better with that patch installed.

Yevgeny Makarov (jenkm) added a comment.Oct 28 2021, 12:50 AM

It should be this way:

const char *home = BLI_getenv("HOME");
BLI_snprintf(dir, FILE_MAXDIR, "%s/Library/Fonts/", home);
Yevgeny Makarov (jenkm) added a comment.Oct 28 2021, 12:52 AM

And for linux, too, probably, look in fsmenu.c / fsmenu_read_system.

Campbell Barton (campbellbarton) added a subscriber: Campbell Barton (campbellbarton).Oct 28 2021, 4:01 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/blenkernel/intern/appdir.c
275–280

Success should only be returned if the directory exists.

Campbell Barton (campbellbarton) requested changes to this revision.Oct 28 2021, 4:08 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/blenkernel/intern/appdir.c
276

Using ~ only works in the file selector (where it's expanded to HOME), this function shouldn't rely on the caller expanding the value and use HOME literal instead.

This revision now requires changes to proceed.Oct 28 2021, 4:08 AM
Harley Acheson (harley) updated this revision to Diff 44057.Oct 28 2021, 4:47 AM

@Campbell Barton (campbellbarton)

  • returning success only if the directory exists
  • using HOME environment string

Note that I am not able to compile for either Mac or Linux.

Yevgeny Makarov (jenkm) added a comment.Oct 28 2021, 8:27 AM
  • Tested on macOS.
Campbell Barton (campbellbarton) added inline comments.Oct 28 2021, 1:38 PM
source/blender/blenkernel/intern/appdir.c
281–282

This check should only run if dir is initialized. It may not be for WIN32.

Harley Acheson (harley) updated this revision to Diff 44098.Oct 29 2021, 5:35 AM

Returning result better on Win32.

Campbell Barton (campbellbarton) accepted this revision.Oct 29 2021, 6:07 AM
This revision is now accepted and ready to land.Oct 29 2021, 6:07 AM
Campbell Barton (campbellbarton) added inline comments.Oct 29 2021, 6:09 AM
source/blender/blenkernel/intern/appdir.c
263–264

This comment can be removed.

Harley Acheson (harley) updated this revision to Diff 44131.Oct 30 2021, 2:07 AM

Removal of unnecessary comment.

Closed by commit rB02a9377da0da: UI: Default Fonts Folder for Mac and Linux (authored by Harley Acheson (harley)). · Explain WhyOct 30 2021, 2:16 AM
This revision was automatically updated to reflect the committed changes.
Harley Acheson (harley) added a commit: rB02a9377da0da: UI: Default Fonts Folder for Mac and Linux.
Ankit Meel (ankitm) added a subscriber: Ankit Meel (ankitm).EditedOct 31 2021, 10:48 AM

Don't agree with making this environment dependent on Mac. Alternatives:

Case where HOME is empty/ unset is not checked either. Caller will attempt to write to system directory which needs root password.

Harley Acheson (harley) added a comment.Oct 31 2021, 3:45 PM

@Ankit Meel (ankitm) - Don't agree with making this environment dependent on Mac. Alternatives…

It would be awesome to improve this on Mac. But this patch is committed and I don’t develop on Mac. So just submit a new patch that makes your proposed changes.

Campbell Barton (campbellbarton) added a comment.Nov 1 2021, 3:47 AM
In D12802#344219, @Ankit Meel (ankitm) wrote:

Case where HOME is empty/ unset is not checked either. Caller will attempt to write to system directory which needs root password.

This is an exceptional situation, checked over the code and other crashes existed here too.

Committed:

Ankit Meel (ankitm) mentioned this in D13212: macOS: add tilde based path expander.Nov 13 2021, 1:00 PM
Ankit Meel (ankitm) added a comment.Nov 13 2021, 1:07 PM

Posted D13213: Cleanup: reuse BKE function, fix comments (which depends on two more patches though)

Ankit Meel (ankitm) mentioned this in rB9df13fba69c3: macOS: add tilde-based path expander.Dec 11 2021, 6:00 PM
Harley Acheson (harley) mentioned this in T94030: Font Changes (Parent Task).Dec 13 2021, 6:12 PM

Revision Contents
Changeset List

PathSize
source/
blender/
blenkernel/
intern/
15 lines

Diff 44049

View Options

source/blender/blenkernel/intern/appdir.c

Loading...