Move font_paths files from aseprite to laf #123
Conversation
|
|
||
| namespace os { | ||
|
|
||
| std::string find_font(const std::string& firstDir, const std::string& filename) |
There was a problem hiding this comment.
warning: no header providing "std::string" is directly included [misc-include-cleaner]
os/font_path.cpp:6:
- #ifdef HAVE_CONFIG_H
+ #include <string>
+ #ifdef HAVE_CONFIG_H| if (base::is_file(fn)) | ||
| return fn; | ||
|
|
||
| base::paths fontDirs; |
There was a problem hiding this comment.
warning: no header providing "base::paths" is directly included [misc-include-cleaner]
os/font_path.cpp:6:
- #ifdef HAVE_CONFIG_H
+ #include "base/paths.h"
+ #ifdef HAVE_CONFIG_H| #endif | ||
|
|
||
| #include "os/font_path.h" | ||
|
|
There was a problem hiding this comment.
warning: included header font_path.h is not used directly [misc-include-cleaner]
| #include "base/string.h" | ||
|
|
||
| #include <cctype> | ||
| #include <shlobj.h> |
There was a problem hiding this comment.
warning: 'shlobj.h' file not found [clang-diagnostic-error]
#include <shlobj.h>
^|
|
||
| namespace app { | ||
|
|
||
| void get_font_dirs(base::paths& fontDirs) |
There was a problem hiding this comment.
warning: no header providing "base::paths" is directly included [misc-include-cleaner]
os/win/font_path_win.cpp:6:
- #ifdef HAVE_CONFIG_H
+ #include "base/paths.h"
+ #ifdef HAVE_CONFIG_H| #endif | ||
|
|
||
| #include "os/font_path.h" | ||
|
|
There was a problem hiding this comment.
warning: included header font_path.h is not used directly [misc-include-cleaner]
|
|
||
| namespace app { | ||
|
|
||
| base::paths g_cache; |
There was a problem hiding this comment.
warning: no header providing "base::paths" is directly included [misc-include-cleaner]
os/x11/font_path_unix.cpp:6:
- #ifdef HAVE_CONFIG_H
+ #include "base/paths.h"
+ #ifdef HAVE_CONFIG_H| return; | ||
| } | ||
|
|
||
| std::queue<std::string> q; |
There was a problem hiding this comment.
warning: no header providing "std::string" is directly included [misc-include-cleaner]
os/x11/font_path_unix.cpp:6:
- #ifdef HAVE_CONFIG_H
+ #include <string>
+ #ifdef HAVE_CONFIG_H| q.push("/usr/share/fonts"); | ||
|
|
||
| while (!q.empty()) { | ||
| std::string fontDir = q.front(); |
There was a problem hiding this comment.
warning: variable 'fontDir' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]
| std::string fontDir = q.front(); | |
| std::string const fontDir = q.front(); |
| fontDirs.push_back(fontDir); | ||
|
|
||
| for (const auto& file : base::list_files(fontDir, base::ItemType::Directories)) { | ||
| std::string fullpath = base::join_path(fontDir, file); |
There was a problem hiding this comment.
warning: variable 'fullpath' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]
| std::string fullpath = base::join_path(fontDir, file); | |
| std::string const fullpath = base::join_path(fontDir, file); |
|
Just in case, any font/text related refactor should go to beta, because there is a new laf-text library and the main branch is deprecated for this kind of stuff. Probably this can be done in the beta branch, but I'm not sure if it's needed there (you should try to do the example app with the beta branch). |
|
I actually needed to use the main branch because the fix is planned for the next release of the main branch. But sure, these changes can go in beta branch. I will first see if they are still needed for beta. |
|
Seems that in beta, in lines: fontFilename = app::find_font(xmlDir, platformFileStr);
if (fileStr && fontFilename.empty())
fontFilename = app::find_font(xmlDir, fileStr);The app::find_font() function is still being used as in the main branch. Which means that if I want to create an example with a custom Theme using just the ui module I cannot use the find_font function because it is part of Aseprite. So, I believe that the refactor still applies for beta. Will create a new PR. |
|
Probably we can move the find_font() function to the ui-lib in that case. In other case, we could refactor the functionality to something inside FontMgr implementation. |
|
Sorry, but in case you want to use a custom font using the ui module, I think the current FontMgr can solve that: Line 27 in 20d13cf |
|
Okay, so, does the current FontMgr have a function that returns all the system directories that are used to look for font files? |
|
Or a function to get all the font files available on the system? |
Yes, check the We want to deprecate and finally remove the |
I see, then does this means that we will change how the fonts.xml works in Aseprite? So, instead of naming the font file, we would just name the family name? |
|
I'm closing this because it doesn't make sense any more, since the function will be deprecated. However, I'm still curious about the future of the processing of fonts.xml. |
Moves the font_path* files from Aseprite source into LAF's "os" namespace. I did this work because when I was trying to find the cause of aseprite/aseprite#4795 I wanted to create an isolated simple app showing just a window with a menu using the aseprite ui, without using any aseprite code. And these files were one of the reasons that made it impossible to do it, hence they had to be moved.