Performance issue when used from ImGui as a SVG font loader #150
Comments
|
@pthom After conducting a thorough investigation into the issue, I discovered that a large SVG file (approximately 12MB) from https://github.com/cppfw/svgren/blob/master/tests/unit/samples_data/back.svg takes about 2.5 seconds to load and render using LunaSvg alone. Interestingly, it loads faster than smaller files like glyphs in the repro profile analysis. This suggests that the problem might be associated with FreeType, ImGui, or other components in the reproduction process. Your ongoing collaboration is crucial for improving this library. Thank you for your support!
Thanks for the New Year wishes! No bother at all, because clearly, dealing with GitHub issues is the best way to kick off the year. 😜 |
|
You are probably right, there is something sniffy in the various components chain (ImGui => FreeType => LunaSvg). Here is how I came to this conclusion: I saw that for example, the glyph number 18 is extremely slow to load.
// Write document->svg_document to a file for debugging
std::ofstream file("svg_document.svg");
file.write((const char*)document->svg_document, document->svg_document_length);And then I looked at the saved svg document for the glyph number 18.
|
|
@ocornut: what do you think? Is is related to Freetype? Or maybe to ImGui? |
|
I have a suspect inside Freetype, namely ttsvg.c. More info: If we look at the evolution of the svg document sizes per glyph, we have: So, I investigated what happens when glyph_i=16, and somewhere inside the long callstack, we have: external/freetype/src/sfnt/ttsvg.c, line 283: FT_LOCAL_DEF( FT_Error )
tt_face_load_svg_doc( FT_GlyphSlot glyph,
FT_UInt glyph_index )
{
FT_Error error = FT_Err_Ok;
TT_Face face = (TT_Face)glyph->face;
FT_Memory memory = face->root.memory;
Svg* svg = (Svg*)face->svg;
FT_Byte* doc_list;
FT_ULong doc_limit;
FT_Byte* doc;
FT_ULong doc_offset;
FT_ULong doc_length;
FT_UShort doc_start_glyph_id;
FT_UShort doc_end_glyph_id;
FT_SVG_Document svg_document = (FT_SVG_Document)glyph->other;
FT_ASSERT( !( svg == NULL ) );
doc_list = svg->svg_doc_list;
error = find_doc( doc_list + 2, svg->num_entries, glyph_index,
&doc_offset, &doc_length,
&doc_start_glyph_id, &doc_end_glyph_id );And svg = {Svg *} 0x6000004171c0
version = {FT_UShort} 0
num_entries = {FT_UShort} 674
glyph_index = {FT_UInt} 6
doc_offset = {FT_ULong} 9365
doc_length = {FT_ULong} 14516311 // ARGH.... 14.5MB
doc_start_glyph_id = {FT_UShort} 4. // Glyph number 4...
doc_end_glyph_id = {FT_UShort} 2808. // ... to 2808... Exactly what I saw inside InkscapeI'm using Freetype VER-2-13-2 (i.e. the latest tag) |
|
And finally, there is a probably related issue for freetype about the same font (NotoColorEmoji): Freetype does not support 'COLR' v1 tables (whatever that means), but as it appears, it still tries to load fonts with such a format, instead of giving an error. See https://gitlab.freedesktop.org/freetype/freetype-demos/-/issues/35
|
|
I posted the issue to Freetype: https://gitlab.freedesktop.org/freetype/freetype/-/issues/1265 |
|
@pthom Well done 👍 |
|
Summary of the current situation: |
|
I can confirm the issue is in Freetype only, since I created a repro that depends only on it. |
|
If a deep dive into FreeType internals interests you, you can look at https://gitlab.freedesktop.org/freetype/freetype/-/issues/1265#note_2228459 |
|
@pthom Hi buddy, Thanks for reaching out. After extensive research, I discovered that FreeType is working as expected. The performance issue seems to originate from the following functions in the ImGui FreeType implementation:
It appears that the glyph ID is omitted, which causes LunaSvg to render the entire document, leading to the performance drop you observed. |
|
Where would the glyph id be used/useful? |
|
@ocornut @pthom According to the FreeType rsvg demo (see source lines 326 to 336):
In the code snippet: if (start_glyph_id < end_glyph_id) {
// Render only the element with its ID equal to `glyph<ID>`.
sprintf(str, "#glyph%u", slot->glyph_index);
id = str;
} else {
// If NULL, render the whole document.
id = NULL;
}
|
|
Thank you for kindly investigating! Much appreciable. |
|
@omar, @sammycage I will also need to re-investigate if the issue I found when using Freetype only (when not using neither ImGui, nor LunaSvg) still holds. As a reminder, I had instrumented the Freetype code and found this: i.e. when using Freetype in a standalone program with a SVG font (see code), some glyphs are using the full document size. Anyway, I'm going to dive into it and let you know. |
|
@pthom I have rendered glyph index 16 of the NotoColorEmoji-Regular.ttf font using plutosvg. Rendering with
|
|
@sammycage, I might need a little more of your time. I'm trying to find a strategy on how to apply your advices. I'm considering the two functions you mentioned "ImGuiLunasvgPortPresetSlot" and "ImGuiLunasvgPortRender". They are both called by Freetype, in the order: The solution might be separated in three steps:
I have several questions (I'm giving you below more details with my current status in the code):
LunasvgPortStateFirst I think we should add an information whether we should render the whole document or just a specific glyph. struct LunasvgPortState
{
FT_Error err = FT_Err_Ok;
lunasvg::Matrix matrix;
std::unique_ptr<lunasvg::Document> svg = nullptr;
char glyph_id[32]; // <==== added this
};ImGuiLunasvgPortPresetSlotImGuiLunasvgPortPresetSlot would be responsible for filling glyph_id. See the "WIP" section below. static FT_Error ImGuiLunasvgPortPresetSlot(FT_GlyphSlot slot, FT_Bool cache, FT_Pointer* _state)
{
FT_SVG_Document document = (FT_SVG_Document)slot->other;
LunasvgPortState* state = *(LunasvgPortState**)_state;
FT_Size_Metrics& metrics = document->metrics;
// This function is called twice, once in the FT_Load_Glyph() and another right before ImGuiLunasvgPortRender().
// If it's the latter, don't do anything because it's // already done in the former.
if (cache)
return state->err;
// WIP: I have one question for you Samuel:
// When reading your message I understand that parsing will be a bit slower, but it's not that big of a deal. Am I right?
state->svg = lunasvg::Document::loadFromData((const char*)document->svg_document, document->svg_document_length);
if (state->svg == nullptr)
{
state->err = FT_Err_Invalid_SVG_Document;
return state->err;
}
// WIP
{
// If I instrument the code such as in rsvg_port.c (as you hinted)
// I can see that we have a strong slowdown when start_glyph_id < end_glyph_id
// which is a hint that you were right.
FT_UShort end_glyph_id = document->end_glyph_id;
FT_UShort start_glyph_id = document->start_glyph_id;
if ( start_glyph_id < end_glyph_id )
{
snprintf(state->glyph_id, sizeof(state->glyph_id), "glyph%u", slot->glyph_index);
printf("ImGuiLunasvgPortPresetSlot glyph_index=%i / Should Render only the element with its ID equal to `glyph%i`\n", slot->glyph_index);
}
else
{
state->glyph_id[0] = '\0';
printf("ImGuiLunasvgPortPresetSlot glyph_index=%i / Should Render the whole document\n", slot->glyph_index);
}
}
lunasvg::Box box = state->svg->box();
double scale = std::min(metrics.x_ppem / box.w, metrics.y_ppem / box.h);
double xx = (double)document->transform.xx / (1 << 16);
double xy = -(double)document->transform.xy / (1 << 16);
double yx = -(double)document->transform.yx / (1 << 16);
double yy = (double)document->transform.yy / (1 << 16);
double x0 = (double)document->delta.x / 64 * box.w / metrics.x_ppem;
double y0 = -(double)document->delta.y / 64 * box.h / metrics.y_ppem;
// Scale and transform, we don't translate the svg yet
state->matrix.identity();
state->matrix.scale(scale, scale);
state->matrix.transform(xx, xy, yx, yy, x0, y0);
state->svg->setMatrix(state->matrix);
// Pre-translate the matrix for the rendering step
state->matrix.translate(-box.x, -box.y);
// Get the box again after the transformation
box = state->svg->box();
// Calculate the bitmap size
slot->bitmap_left = FT_Int(box.x);
slot->bitmap_top = FT_Int(-box.y);
slot->bitmap.rows = (unsigned int)(ImCeil((float)box.h));
slot->bitmap.width = (unsigned int)(ImCeil((float)box.w));
slot->bitmap.pitch = slot->bitmap.width * 4;
slot->bitmap.pixel_mode = FT_PIXEL_MODE_BGRA;
// Compute all the bearings and set them correctly. The outline is scaled already, we just need to use the bounding box.
double metrics_width = box.w;
double metrics_height = box.h;
double horiBearingX = box.x;
double horiBearingY = -box.y;
double vertBearingX = slot->metrics.horiBearingX / 64.0 - slot->metrics.horiAdvance / 64.0 / 2.0;
double vertBearingY = (slot->metrics.vertAdvance / 64.0 - slot->metrics.height / 64.0) / 2.0;
slot->metrics.width = FT_Pos(IM_ROUND(metrics_width * 64.0)); // Using IM_ROUND() assume width and height are positive
slot->metrics.height = FT_Pos(IM_ROUND(metrics_height * 64.0));
slot->metrics.horiBearingX = FT_Pos(horiBearingX * 64);
slot->metrics.horiBearingY = FT_Pos(horiBearingY * 64);
slot->metrics.vertBearingX = FT_Pos(vertBearingX * 64);
slot->metrics.vertBearingY = FT_Pos(vertBearingY * 64);
if (slot->metrics.vertAdvance == 0)
slot->metrics.vertAdvance = FT_Pos(metrics_height * 1.2 * 64.0);
state->err = FT_Err_Ok;
return state->err;
}ImGuiLunasvgPortRenderThis function should be able to render only the desired ID. How can this be achieved? static FT_Error ImGuiLunasvgPortRender(FT_GlyphSlot slot, FT_Pointer* _state)
{
printf("ImGuiLunasvgPortRender glyph_index=%i\n", slot->glyph_index);
LunasvgPortState* state = *(LunasvgPortState**)_state;
// If there was an error while loading the svg in ImGuiLunasvgPortPresetSlot(), the renderer hook still get called, so just returns the error.
if (state->err != FT_Err_Ok)
return state->err;
// rows is height, pitch (or stride) equals to width * sizeof(int32)
lunasvg::Bitmap bitmap((uint8_t*)slot->bitmap.buffer, slot->bitmap.width, slot->bitmap.rows, slot->bitmap.pitch);
state->svg->setMatrix(state->svg->matrix().identity()); // Reset the svg matrix to the default value
// WIP:
// Another question for you, Samuel:
// How could we limit the rendering to one specific ID (i.e. state->glyph_id) instead of the whole document?
state->svg->render(bitmap, state->matrix); // state->matrix is already scaled and translated
state->err = FT_Err_Ok;
return state->err;
}Anyway, thanks again for your efforts. It is a big help! |
Yes, parsing generally much faster than rendering, so the impact is minimal.
Limiting the rendering to one specific ID with a specific matrix isn't possible with the current version. However, I am currently working on this feature. |
|
@pthom In the meantime, I'd love to hear your thoughts on plutosvg. PlutoSVG is specifically designed for rendering SVG documents embedded in OpenType fonts. It uses block allocation to minimize memory fragmentation and is several times faster. Additionally, it supports Color Palette Table and image rendering. It implements all the elements in the specifications except for clipPath. |
|
@sammycage
As far as using plutosvg inside Dear ImGui, It is not a decision I can make by myself. We will have to discuss this with Omar. There are potential consequences to consider in the decision , especially for Dear ImGui users who have already set up a build system using LunaSvg. Thanks! |
I believe it would be best to leave it open until the issue is completely resolved.
I understand if this is the case.
I will update you when I am done. Thank you |
|
Give me a few days, I'll give a try to plutosvg, so that Omar can have a look at it and see whether the switch might be interesting (However, I think that adding this to LunaSVG might be interesting as well) |
I'll post an update anyhow, to inform them. It seems important, since Werner Lemberg seems to be looking for feedback about it. 
Update: here is the message I posted: |
|
test plutosvg, current status: here are some first results after a first quick try Plutosvg: I could get it to compile and to replacer the Freetype hooks However, the rendering is now faster, but still too slow (see below for profile analysis). It takes about 1 minute to load the font "NotoColorEmoji-Regular". Compile and link with ImGuiThis is slightly more complex that lunasvg, since it requires plutosvg + plutovg. There is no vcpkg support for the moment (as opposed to lunasvg). Also, note that plutovg comes with its own version of stb_truetype.h, which is distinct from the one provided with ImGui (imstb_truetype.h). Both versions are almost similar (except for some warning correction on ImGui side) When using CMake, the difference boils down to: if (USE_PLUTO_INSTEAD_OF_LUNA_SVG)
#
# Add plutovg and plutosvg
#
# plutovg
file(GLOB plutovg_sources external/plutovg/source/*.c)
add_library(plutovg STATIC ${plutovg_sources})
target_include_directories(plutovg PUBLIC external/plutovg/include)
target_include_directories(plutovg PRIVATE external/plutovg/stb) # Note: two versions of stb_truetype.h (one in plutovg, one in imgui)
# plutosvg
add_library(plutosvg STATIC external/plutosvg/source/plutosvg.c)
target_include_directories(plutosvg PUBLIC external/plutosvg/source)
target_compile_definitions(plutosvg PUBLIC PLUTOSVG_HAS_FREETYPE)
target_link_libraries(plutosvg PUBLIC plutovg freetype)
else()
# Add lunasvg
add_subdirectory(external/lunasvg)
endif()This difference in setup might interest @ocornut in deciding whether or not the switch is worth it. Integration with ImGuiThe integration is simple at the moment, see modifications inside ImGui: pthom/imgui@be7fbdf StatusHowever this first commit is not enough, since the rendering is still slow Repro codeSee the branch plutosvg in the repro repository: Profiling resultIt seems that we spend too much time inside plutosvg_load_document_from_data 
I do not think there is much to do on the ImGui side to make it faster, since the hooks are now handled by plutosvg. |
@pthom With the latest commit of plutosvg, it can now load and render all the glyphs in less than 4 seconds.
In the next major release of lunasvg, lunasvg will be using plutovg as a dependency #110.
Once I am done with the next major release, I will talk to vcpkg to add plutovg and plutosvg. |
|
I confirm that using the latest commit will reduce the font loading time down to 3 seconds (with FontHeight=60, 1428 glyphs); which is OK I think. 
I now have a nice running gui in the ImGui window: 
Next steps
@ocornut: A decision will likely have to be made about the next steps. I'm open to work on it, but need more guidance. Let's clarify what we know in order to be able to decide with accuracy:
However, there are several points to consider: #if !((FREETYPE_MAJOR >= 2) && (FREETYPE_MINOR >= 12))
#error IMGUI_ENABLE_FREETYPE_LUNASVG requires FreeType version >= 2.12
#endif#ifdef IMGUI_ENABLE_FREETYPE_LUNASVG
IM_ASSERT(slot->format == FT_GLYPH_FORMAT_OUTLINE || slot->format == FT_GLYPH_FORMAT_BITMAP || slot->format == FT_GLYPH_FORMAT_SVG); |
3 seconds? Isn't that cool 😎. librsvg, the one used in the FreeType demo, takes like infinity!
Thank you for the suggestion. At this time, we do not plan to add a CMakeLists file alongside the Meson build file.
Sorry, but the Meson options should handle that for you. I recommend checking the Meson build configuration for details on the available options. |
Not everyone will use meson. I did not, and will not in this particular case (where another build system was in place) :-) Anyway, this is not that important.
1.5 seconds in release builds. Nice! |
…native to lunasvg (fix ocornut#7187) <SVG Fonts include a set of SVG documents. As per the [OpenType specification](https://learn.microsoft.com/en-us/typography/opentype/spec/svg#glyph-identifiers), some SVG fonts (such as NotoColorEmoji) may group several glyphs in a common svg document (by selecting a subset of the elements in this document). LunaSvg does support fonts where each glyph is associated to a distinct document. Unfortunately, it is not able to render a subset of a svg document, and will likely not be able to do so in the future. Its cousin project plutosvg (by the same author), is able to do it, and provides ready to use freetype hooks. Example: sammycage/lunasvg#150 shows an example where a single svg document included in the font may contains thousands of glyphs (each glyph is a subset of the svg document). Pros and Cons ------------- Since this commit adds some complexity in the code here is a study of the pros and cons: Pros: - plutosvg is faster than lunasvg - freetype hooks are provided by plutosvg (no need to provide them in imgui_freetype.cpp) Cons: - the compilation setup for plutosvg is a bit more complex than for lunasvg (requires plutovg + plutosvg) - no offical release is available yet for plutosvg. No vcpkg package available yet. - having two competing compilation flags is not ideal (IMGUI_ENABLE_FREETYPE_LUNASVG vs IMGUI_ENABLE_FREETYPE_PLUTOSVG) (it may be possible to remove IMGUI_ENABLE_FREETYPE_LUNASVG in the future, at the cost of breaking some users build upon upgrade) Compilation hints for plutovg/plutosvg -------------------------------------- _Compilation hints for plutovg_ - Compile all source files in `plutovg/source/*.c` - Add include directory: `plutovg/include` + `plutovg/stb` _Compilation hints for plutosvg_ - Compile `plutosvg/source/plutosvg.c` - Add include directory: `plutosvg/source` - Add define: `PLUTOSVG_HAS_FREETYPE` - Link with: plutovg, freetype Demonstration repository ------------------------- https://github.com/pthom/lunasvg_perf_issue
|
See ImGui new PR where I propose the integration of plutosvg. @sammycage : It seems that you did some more performance improvement, since the application now starts in 0.45 seconds (this includes font loading + all application setup). |
|
Hey @pthom, how's it going? I wanted to let you know that element subsetting is fully supported in version 3.0.0 of LunaSVG! Although PlutoSVG is several times faster than LunaSVG, as you mentioned, there are indeed potential consequences to consider, especially for Dear ImGui users who have already set up a build system using LunaSVG. If you decide to stick with LunaSVG, here’s some code that might help with the fix: #include <cstdio>
#include <cmath>
#include <ft2build.h>
#include FT_FREETYPE_H
#include FT_MODULE_H
#include FT_OTSVG_H
#include <lunasvg.h>
struct ImGuiLunasvgPortEntry
{
ImGuiLunasvgPortEntry(FT_UShort start_glyph_id, FT_UShort end_glyph_id, std::unique_ptr<lunasvg::Document> document);
FT_UShort start_glyph_id;
FT_UShort end_glyph_id;
std::unique_ptr<lunasvg::Document> document;
};
inline ImGuiLunasvgPortEntry::ImGuiLunasvgPortEntry(FT_UShort start_glyph_id, FT_UShort end_glyph_id, std::unique_ptr<lunasvg::Document> document)
: start_glyph_id(start_glyph_id), end_glyph_id(end_glyph_id), document(std::move(document))
{
}
using ImGuiLunasvgPortEntries = std::vector<ImGuiLunasvgPortEntry>;
struct LunasvgPortState
{
lunasvg::Matrix matrix;
lunasvg::Box extents;
lunasvg::Element element;
ImGuiLunasvgPortEntries entries;
};
static FT_Error ImGuiLunasvgPortInit(FT_Pointer* _state)
{
*_state = new LunasvgPortState;
return FT_Err_Ok;
}
static void ImGuiLunasvgPortFree(FT_Pointer* _state)
{
delete (*(LunasvgPortState**)_state);
}
static FT_Error ImGuiLunasvgPortRender(FT_GlyphSlot slot, FT_Pointer* _state)
{
LunasvgPortState* state = *(LunasvgPortState**)_state;
if(state->element.isNull()) {
return FT_Err_Invalid_SVG_Document;
}
lunasvg::Bitmap bitmap((uint8_t*)slot->bitmap.buffer, slot->bitmap.width, slot->bitmap.rows, slot->bitmap.pitch);
state->element.render(bitmap, lunasvg::Matrix::translated(-state->extents.x, -state->extents.y) * state->matrix);
slot->bitmap.pixel_mode = FT_PIXEL_MODE_BGRA;
slot->bitmap.num_grays = 256;
slot->format = FT_GLYPH_FORMAT_BITMAP;
return FT_Err_Ok;
}
static lunasvg::Document* ImGuiLunasvgPortLoad(LunasvgPortState* state, FT_SVG_Document ft_document, FT_UInt index)
{
if(ft_document->start_glyph_id < ft_document->end_glyph_id) {
for(const auto& entry : state->entries) {
if(index >= entry.start_glyph_id && index <= entry.end_glyph_id) {
return entry.document.get();
}
}
}
auto document = lunasvg::Document::loadFromData(reinterpret_cast<const char*>(ft_document->svg_document), static_cast<size_t>(ft_document->svg_document_length));
if(document == nullptr)
return nullptr;
state->entries.emplace_back(ft_document->start_glyph_id, ft_document->end_glyph_id, std::move(document));
return state->entries.back().document.get();
}
static FT_Error ImGuiLunasvgPortPresetSlot(FT_GlyphSlot slot, FT_Bool cache, FT_Pointer* _state)
{
FT_SVG_Document ft_document = (FT_SVG_Document)slot->other;
FT_Size_Metrics& ft_metrics = ft_document->metrics;
LunasvgPortState* state = *(LunasvgPortState**)_state;
state->element = lunasvg::Element();
state->matrix = lunasvg::Matrix();
state->extents = lunasvg::Box();
auto document = ImGuiLunasvgPortLoad(state, ft_document, slot->glyph_index);
if(document == nullptr) {
return FT_Err_Invalid_SVG_Document;
}
lunasvg::Element element;
if(ft_document->start_glyph_id < ft_document->end_glyph_id) {
char id[64];
std::sprintf(id, "glyph%u", slot->glyph_index);
element = document->getElementById(id);
} else {
element = document->documentElement();
}
if(element.isNull()) {
return FT_Err_Invalid_SVG_Document;
}
auto extents = element.getLocalBoundingBox();
auto scale = std::min(ft_metrics.x_ppem / extents.w, ft_metrics.y_ppem / extents.h);
auto matrix = lunasvg::Matrix::scaled(scale, scale);
matrix *= lunasvg::Matrix {
(float)ft_document->transform.xx / (1 << 16),
-(float)ft_document->transform.xy / (1 << 16),
-(float)ft_document->transform.yx / (1 << 16),
(float)ft_document->transform.yy / (1 << 16),
(float)ft_document->delta.x / 64 * extents.w / ft_metrics.x_ppem,
-(float)ft_document->delta.y / 64 * extents.h / ft_metrics.y_ppem
};
extents.transform(matrix);
slot->bitmap_left = (FT_Int)extents.x;
slot->bitmap_top = (FT_Int)-extents.y;
slot->bitmap.rows = (unsigned int)ceilf(extents.h);
slot->bitmap.width = (unsigned int)ceilf(extents.w);
slot->bitmap.pitch = (int)slot->bitmap.width * 4;
slot->bitmap.pixel_mode = FT_PIXEL_MODE_BGRA;
float metrics_width = extents.w;
float metrics_height = extents.h;
float horiBearingX = extents.x;
float horiBearingY = -extents.y;
float vertBearingX = slot->metrics.horiBearingX / 64.f - slot->metrics.horiAdvance / 64.f / 2;
float vertBearingY = (slot->metrics.vertAdvance / 64.f - slot->metrics.height / 64.f) / 2;
slot->metrics.width = (FT_Pos)roundf(metrics_width * 64);
slot->metrics.height = (FT_Pos)roundf(metrics_height * 64);
slot->metrics.horiBearingX = (FT_Pos)(horiBearingX * 64);
slot->metrics.horiBearingY = (FT_Pos)(horiBearingY * 64);
slot->metrics.vertBearingX = (FT_Pos)(vertBearingX * 64);
slot->metrics.vertBearingY = (FT_Pos)(vertBearingY * 64);
if(slot->metrics.vertAdvance == 0)
slot->metrics.vertAdvance = (FT_Pos)(metrics_height * 1.2f * 64);
if(cache) {
state->element = element;
state->matrix = matrix;
state->extents = extents;
}
return FT_Err_Ok;
}
SVG_RendererHooks hooks = { ImGuiLunasvgPortInit, ImGuiLunasvgPortFree, ImGuiLunasvgPortRender, ImGuiLunasvgPortPresetSlot };Let me know if you have any other questions or need further assistance! |
This is a follow up to these related issues: - ocornut#7187 (which may need to be reopened, since there is a real rendering issue in ImGui) - sammycage/lunasvg#150 ---- SVG Fonts include a set of SVG documents. As per the [OpenType specification](https://learn.microsoft.com/en-us/typography/opentype/spec/svg#glyph-identifiers), some SVG fonts (such as NotoColorEmoji) may group several glyphs in a common svg document (by selecting a subset of the elements in this document). LunaSvg did not originally support fonts where each glyph is associated to a distinct document. LunaSvg 3.0.0 now supports this feature This PR thus adds support for LunaSvg (3.0.0), and its author (@sammycage) was kind enough to provide a patch for ImGui: sammycage/lunasvg#150 (comment) Notes ----- - WARNING: the API of LunaSVG has changed inside the latest version (3.0.0): the current code inside imgui_freetype.cpp (without this PR) is not compatible with it! A test for this was added in imgui_freetype.cpp: ```cpp #ifndef LUNASVG_VERSION_MAJOR #error IMGUI_ENABLE_FREETYPE_LUNASVG requires LunaSvg version >= 3.0 #endif #if !(LUNASVG_VERSION_MAJOR >= 3) #error IMGUI_ENABLE_FREETYPE_LUNASVG requires LunaSvg version >= 3.0 #endif ``` - Performance: NotoColorEmoji-Regular is now correctly loaded in approx 1 second (versus > 1 hour with current ImGui) - Alternative PR, using PlutoSvg instead of LunaSvg: There is an alternative PR that would replace LunaSvg by PlutoSvg (ocornut#7927). We will likely have to choose between those two. The present PR does not propose to replace LunaSvg by PlutoSvg, but to update LunaSvg to the latest version. - Demonstration repository: https://github.com/pthom/lunasvg_perf_issue / branch "lunasvg_patch_oct24"
|
Hello Sammy, I looked at your code which uses LunaSvg, and used it "as is" inside imgui_freetype.cpp. I think it is possible to open an alternative PR in the ImGui repo, this time using the updated LunaSvg. https://github.com/ocornut/imgui/compare/master...pthom:imgui:update_lunasvg?expand=1 Notes:
Please keep me posted. Cheers! Below is the commit message I wrote: Title: Fix OpenType SVG fonts rendering with LunaSVG 3.0 (by @sammycage) This is a follow up to these related issues:
SVG Fonts include a set of SVG documents. As per the OpenType specification, some SVG fonts (such as NotoColorEmoji) may group several glyphs in a common svg document (by selecting a subset of the elements in this document). LunaSvg did not originally support fonts where each glyph is associated to a distinct document. LunaSvg 3.0.0 now supports this feature This PR thus adds support for LunaSvg (3.0.0), and its author (@sammycage) was kind enough to provide a patch for ImGui: #150 (comment) Notes
#ifndef LUNASVG_VERSION_MAJOR
#error IMGUI_ENABLE_FREETYPE_LUNASVG requires LunaSvg version >= 3.0
#endif
#if !(LUNASVG_VERSION_MAJOR >= 3)
#error IMGUI_ENABLE_FREETYPE_LUNASVG requires LunaSvg version >= 3.0
#endif
|
|
I appreciate your mention in the commit. Regarding the code, I would like to clarify that I expect it not to be used as-is in the final PR. ImGui uses custom macros and functions, such as I'm happy to help review and refine this further before it’s finalized. |
|
Hello Sammy, Omar did merge the PR that added support for plutosvg. Posting a PR that adds support for the LunaSVG 3.0 is less urgent now that plutosvg support is included (and perhaps not needed after all). However, the fact that the code currently in imgui_freetype.cpp is not compatible with LunaSVG 3.0 is a possible issue in the future, as it seems parts of the API has changed in V3.0. Here is a list of the errors raised when compiling imgui_freeetype.cpp (as of now) with LunaSVG 3.0: Given this, several solutions can be explored:
At the moment, my bet would be on 2. or 3. @sammycage: do you think option 2 would be feasible? @ocornut: if you happen to read this, do you have an opinion? Thanks |
|
@pthom After considering the options, I believe going with Option 1 would be the best approach for now, and I personally don't think Option 2 is feasible, at least for me. Trying to maintain backward compatibility with older APIs might introduce unnecessary complexity and maintenance overhead. Additionally, for the sake of code quality and simplicity, maintaining two libraries that essentially do the same thing doesn't seem like the best idea. Streamlining by sticking with PlutoSVG, now that support has been merged, could avoid potential conflicts and redundancies in the future. |
|
@pthom does vcpkg support dependencies in the same way that conda(-forge) does? edit: it seems reasonable to say "ok this new version of imgui now requires lunasvg > 3.0" |
|
@hmaarrfk : I will answer in your PR at pthom/imgui_bundle#284 |
Hello,
I saw some performance issues with ImGui when it uses LunaSvg in order to load some Svg fonts:
It can load some fonts with a very good performance. However, with some others, the performance drops and it takes about 2 seconds per glyph to load.
In order to facilitate the analysis I prepared a repro repository, here: https://github.com/pthom/lunasvg_perf_issue
{ // will load 1408 colored glyphs, with lunasvg. Fast! std::string fontFile = ThisDir() + "/fonts/noto-untouchedsvg.ttf"; gEmojiFont = ImGui::GetIO().Fonts->AddFontFromFileTTF(fontFile.c_str(), 30.0f, &cfg, ranges); } { // will load 1428 glyphs, with lunasvg. Slow. About 2 seconds per glyph std::string fontFile = ThisDir() + "/fonts/NotoColorEmoji-Regular.ttf"; gEmojiFont = ImGui::GetIO().Fonts->AddFontFromFileTTF(fontFile.c_str(), 30.0f, &cfg, ranges); }As explained is the repro repository, a quick analysis with a profiler shows that time seems to be spent in std::map::find + std::vector::emplace_back.
PS: Happy new year! I'm sorry to bother you on Jan 2nd!
The text was updated successfully, but these errors were encountered: