directwrite: query GDI-enumerated fonts by full name, not family name #745
Conversation
| // the requested family, but we would rather take this chance than | ||
| // add every font twice (via the family name and via the full name). | ||
| // lfFaceName can hold up to LF_FACESIZE wchars; truncate longer names. | ||
| wcsncpy(lf.lfFaceName, lpelf->elfFullName, LF_FACESIZE - 1); |
There was a problem hiding this comment.
It may be a bit cleaner to do wcsncpy_s(lf.lfFaceName, LF_FACESIZE, lpelf->elfFullName, _TRUNCATE);
There was a problem hiding this comment.
wcsncpy_s is part of C11, but currently our baseline is ISO C99.
Raising it to C11 came up before and we might do that eventually.
Tangential question: afaik all of this code currently still works/can be built for Windows XP and it has previously been mentioned that this can be useful for checking stuff against early VSFilters or so. Are there C11 compilers for Windows XP or is building for Windows XP still relevant?
There was a problem hiding this comment.
currently our baseline is ISO C99
This information doesn't seem to be written anywhere. It would be nice to have a contributing guide mentioning that.
afaik all of this code currently still works/can be built for Windows XP
Technically, ass_directwrite.c is only for Windows Vista SP2 and later, as DWrite doesn't exist before that.
Are there C11 compilers for Windows XP?
I don't know.
building for Windows XP still relevant?
VLC still support Windows XP, so it is relevant.
There was a problem hiding this comment.
It being kinda nonstandard is part of the reason I don’t use it, but to be fair, this file is Windows-specific and it’s existed on Windows for (at least) almost two decades. It’s in Microsoft’s versioned Visual C++ libraries, it’s in UCRT, and it’s also in MSVCRT (so even compiling with 32-bit MinGW-w64 GCC works, as I’ve just verified), but I don’t know when exactly it was added to MSVCRT and to MinGW headers.
There was a problem hiding this comment.
currently our baseline is ISO C99
This information doesn't seem to be written anywhere. It would be nice to have a contributing guide mentioning that.
It’s what our buildsystem sets:
Lines 3 to 6 in e517813
But admittedly this is not very prominent and buildflags also explicitly enabling GNU extensions is slightly misleading. We enable those since we want to make use of them if available but provide fallbacks if not available.
That being said, if Windows had this function for sufficiently long, using it in a Windows-specific file is absolutely fine.
There was a problem hiding this comment.
Added to MinGW-w64 headers in 2011. I’ve been unable to find the MinGW32 equivalent.
On Microsoft’s side, first appeared in Visual C++ 2005 (released, appropriately, in 2005). When they were added to MSVCRT, though, I don’t know. Could’ve been immediately or at some later point.
There was a problem hiding this comment.
Technically,
ass_directwrite.cis only for Windows Vista SP2 and later, as DWrite doesn't exist before that.
Not quite. It cannot run on Windows XP, but it can be built on Windows XP and run on Vista+, and binaries built on Vista+ can run on XP without using ass_directwrite at runtime.
There was a problem hiding this comment.
While the code with _TRUNCATE does look fairly nice, I feel I remain hesitant to use it for fear of it being absent in older/other compilers/libraries. So I’ll go ahead and merge this as is. But thanks for the suggestion; it may serve as a bit of education and inspiration for future code if/when we upgrade to newer language versions.
fwiw, I have barely any clue about Windows’ font APIs, but the explanation/motivation seems good to me and it apparently fixes a bug, so afaict LGTM |
Fixes: #744
The code is fairly clear, but I guess I want a second opinion on whether ignoring the family name entirely is Good Enough.