Skip to content
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

Refactor fonts manager #3401

Merged
merged 6 commits into from
Sep 23, 2024
Merged

Refactor fonts manager #3401

merged 6 commits into from
Sep 23, 2024

Conversation

Grantim
Copy link
Contributor

@Grantim Grantim commented Sep 23, 2024

This PR simplifies font loading, no hardcoded offsets needed:
(separates loading in two steps: load single glyph and calculate desired offset, load all requested glyphs with calculated offset)
Also allows users to set custom font paths

{
assert( false && "Failed to load font!" );
spdlog::error( "Failed to load font from `{}`.", filename );

ImGui::GetIO().Fonts->AddFontFromMemoryCompressedTTF( droid_sans_compressed_data,
font =ImGui::GetIO().Fonts->AddFontFromMemoryCompressedTTF( droid_sans_compressed_data,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it be loaded multiple times if multiple fonts fail to load?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes in different sizes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it will be loaded for each failed font

Comment on lines 49 to 56
FontData{.uniqueFontType = UniqueFont::Regular},
FontData{.uniqueFontType = UniqueFont::Regular},
FontData{.uniqueFontType = UniqueFont::SemiBold},
FontData{.uniqueFontType = UniqueFont::Icons},
FontData{.uniqueFontType = UniqueFont::Regular},
FontData{.uniqueFontType = UniqueFont::SemiBold},
FontData{.uniqueFontType = UniqueFont::SemiBold},
FontData{.uniqueFontType = UniqueFont::Monospace}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values are unclear from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed

}
else if ( type == FontType::SemiBold )
ImGuiIO& io = ImGui::GetIO();
const ImWchar wRange[] = { 0x0057, 0x0057, 0 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source/meaning of the constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment added

@@ -25,6 +25,20 @@ class MRVIEWER_CLASS RibbonFontManager
Count
};

// Unique fonts that are used for different FontTypes
enum class UniqueFont
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ambiguous enum name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed

@Grantim Grantim merged commit ebc4c67 into master Sep 23, 2024
27 of 28 checks passed
@Grantim Grantim deleted the Refactor_Fonts_Manager branch September 23, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants