Details

Reviewers

Julian Eisel (Severin)
Campbell Barton (campbellbarton)

Group Reviewers

User Interface

Commits

rB8aa1c0a326a8: Fix T75028: Improved Font Names in File Manager

Summary

I'm not sure how this is on other platforms, but on Windows when viewing a list of font names - either in a list in a program or in Windows Explorer - we see the font's display name, not the file system name. This can be quite confusing. For example I will see a font name as "Book Antiqua Regular" all over the place, but it is shown as "BKANT" in blender. I might be looking for "Bodini MT Black" but I have to guess this is actually "BOD_BLAR".

This patch makes it so that we see the full family and style names in the file browser. The names sort with display name of course. And when selecting, the proper file name is still selected. And this is done in the correct place for these to be cached with loading directories.

The following shows a portion of my current list on the left, after patch applied on the right:

This patch also uses the same name when loading vfonts (used for 3d font objects).

Diff Detail

Repository: rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Jul 23 2021, 10:25 PM

Harley Acheson (harley) created this revision.

Ray Molenkamp (LazyDodo) awarded a token.Jul 23 2021, 10:37 PM

Aaron Carlisle (Blendify) awarded a token.Jul 23 2021, 11:13 PM

Duarte Farrajota Ramos (duarteframos) awarded a token.Jul 24 2021, 12:11 AM

Vitalii Vlasov (Fracture128) awarded a token.Jul 24 2021, 11:03 AM

Evan Wilson (EAW) awarded a token.Jul 25 2021, 3:25 PM

Julian Eisel (Severin) requested changes to this revision.Jul 27 2021, 4:51 PM

Julian Eisel (Severin) added a subscriber: Julian Eisel (Severin).

Julian Eisel (Severin) added inline comments.

source/blender/blenfont/intern/blf.c
919	This API usually acts on font-IDs. I'd suggest having an internal `blf_display_name(FontBLF *font)` and turning this into `BLF_display_name_from_file(...)` that calls the former.
source/blender/editors/space_file/filelist.c
1836	This is a flag, should use bitwise `&`.

This revision now requires changes to proceed.Jul 27 2021, 4:51 PM

Julian Eisel (Severin) awarded a token.Jul 27 2021, 4:51 PM

Campbell Barton (campbellbarton) requested changes to this revision.Jul 27 2021, 5:30 PM

Campbell Barton (campbellbarton) added a subscriber: Campbell Barton (campbellbarton).

Campbell Barton (campbellbarton) added inline comments.

source/blender/blenfont/intern/blf.c
40	Prefer individual includes (only `BLI_string.h` should be needed in this case).
source/blender/editors/space_file/filelist.c
1839	This will leak memory, as this pointer isn't intended to be allocated.

Updated to incorporate all suggested review changes.

BLF_display_name_from_file() & blf_display_name()
bitwise check for FILE_TYPE_FTFONT
changes to includes
no more double allocation of display string

Initialize displayname with zeros.
Handle failure to get displayname.

Campbell Barton (campbellbarton) requested changes to this revision.Jul 28 2021, 2:40 AM

Campbell Barton (campbellbarton) added inline comments.

source/blender/blenfont/intern/blf.c
918	prefer `name`, `name_len`. `r_` prefix is typically for cases when passing a pointer to a value (although there are some exceptions to that).
source/blender/editors/space_file/filelist.c
1841	This returns stack memory which goes out of scope, use `buff` instead (which should really have a length argument too).

This revision now requires changes to proceed.Jul 28 2021, 2:40 AM

Pratik Borhade (PratikPB2123) awarded a token.Jul 28 2021, 2:57 AM

Simpler version.

@Campbell Barton (campbellbarton) - This returns stack memory which goes out of scope...

Actually backtracked a bit. Ray prodded me into going back to allocating that displayname with BLI_sprintfN and just moving around the BLI_strdup for the other code paths of fileentry_uiname.

This is much simpler and reads well.

@Campbell Barton (campbellbarton) - use buff instead (which should really have a length argument too)....

Off topic but that buff looks weird. We pass in a dir and call it buff here. Then we use only if FILE_TYPE_BLENDERLIB and pass it as r_dir argument to BLO_library_path_explode.

Seems very obscure that the assignment of entry->name could alter the value of dir under narrow circumstances.

Campbell Barton (campbellbarton) accepted this revision.Jul 28 2021, 10:40 AM

Great!

This touches only the name display in the File Browser though. Would be nice to also show the font name elsewhere in the UI, not the file name. BKE_vfont_load() sets the name of the font data-block, I think it should be as simple as changing that.

This revision is now accepted and ready to land.Jul 28 2021, 11:16 AM