| | [ Return to Bugs & Features | SVN ⇄ GIT ]
STR #3364
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 3 - Moderate, e.g. unable to compile the software |
|---|
| Scope: | 3 - Applies to all machines and operating systems |
|---|
| Subsystem: | Core Library |
|---|
| Summary: | Fl_Text_Display: wrong text width calculation |
|---|
| Version: | 1.4-current |
|---|
| Created By: | chris |
|---|
| Assigned To: | matt |
|---|
| Fix Version: | 1.4.0 |
|---|
| Fix Commit: | cdb51045dda7076be33147cc8e9138035ad9b97e |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #1 | chris
00:03 Feb 06, 2017 |
| There is something wrong with text width calculation in Fl_Text_Display.
Best seen with italic fonts (e.g. styled comments in test/editor when editing .cxx files).
How to reproduce:
- Use test/editor demo
- Better seen when changing the default textsize TS (line 62 in editor.cxx) to a bigger font e.g. 100
- edit a .cxx file with it e.g ./editor editor.cxx
- look at the '/' characters at line ends => they miss a part on the right
I attach a screenshot of this scenario.
I hunted that down to an fl_width() statement in Fl_Text_Display.cxx. Don't know if there is generally something wrong with fl_width() or if this is not suitable here in that context.
Anyway, replacing it with an fl_text_extents() seems to get the right widths.
Patch attached. | |
| |
| #2 | chris
00:05 Feb 06, 2017 |
| Forgot to mention: this is on Ubuntu 14.04. | |
| |
| #3 | chris
04:35 Nov 19, 2017 |
| There has been a remark about this STR in http://www.fltk.org/str.php?L3320 (#70 by Albrecht).
Citing here the relevant passage:
---
".. Another observation in the same image is that the beginning '//' on otherwise "empty" lines are truncated on the upper right side. ISTR that this was already reported (maybe by Chris?), but I couldn't find it right now.
Both observations may be related (text measurement). IIRC someone (Chris?) reported that replacing fl_measure() with fl_text_extents() helped to fix at least the 2nd (i.e. '//') issue. But I wonder if calling fl_text_extents() in Fl_Text_Display/Editor might be too expensive and slow text rendering down."
---
First a correction: The patch replaces *fl_width* with fl_text_extents.
If the performance doubts are the only reason for not accepting this patch, then I can assure that in the current code (at least for Xlib/Xft) there is practically no difference between fl_width() and fl_text_extents(). See the code of Fl_Xlib_Graphics_driver_font_xft.cxx:
--- snip ---
double Fl_Xlib_Graphics_Driver::width_unscaled(const char* str, int n) {
if (!font_descriptor()) return -1.0;
XGlyphInfo i;
utf8extents(font_descriptor(), str, n, &i);
return i.xOff;
}
static double fl_xft_width(Fl_Font_Descriptor *desc, FcChar32 *str, int n) {
if (!desc) return -1.0;
XGlyphInfo i;
XftTextExtents32(fl_display, desc->font, str, n, &i);
return i.xOff;
}
double Fl_Xlib_Graphics_Driver::width_unscaled(unsigned int c) {
if (!font_descriptor()) return -1.0;
return fl_xft_width(font_descriptor(), (FcChar32 *)(&c), 1);
}
void Fl_Xlib_Graphics_Driver::text_extents_unscaled(const char *c, int n, int &dx, int &dy, int &w, int &h) {
if (!font_descriptor()) {
w = h = 0;
dx = dy = 0;
return;
}
XGlyphInfo gi;
utf8extents(font_descriptor(), c, n, &gi);
w = gi.width;
h = gi.height;
dx = -gi.x + line_delta_;
dy = -gi.y + line_delta_;
correct_extents(scale_, dx, dy, w, h);
}
-- snip --- | |
| |
| #4 | AlbrechtS
07:46 Dec 26, 2017 |
| Thanks for the report and the xft code analysis. I tried to confirm (or disprove) my speed concerns, but unfortunately it is even worse than I could imagine.
WRT Linux you are correct. There are speed differences, but they are small, so we could probably use fl_text_extents() instead of fl_width().
However, under Windows, fl_text_extents() is incredibly slow, and this results in really sluggish behavior in Fl_Text_Editor. A simple test would be to use your patch, then open misc/lorem_ipsum.txt or another long text file with (preferably) long text lines, and scroll up and down. It gets even worse if you enable word wrapping (menu: Edit/Preferences/Word Wrap). I used a special test program that measures text thousands of times, and the differences under Linux were some percent, whereas the loss of speed under Windows could be measured in factors of several hundred (500 - 1500).
An additional bad behavior I experienced was that after the test described in the previous paragraph, the program doesn't exit instantly when the editor window is closed. It takes several seconds (up to minutes!) until the program finally terminates. This is caused by some reformatting of strings when the editor buffer is deleted (in the modify_callback). IMHO this should not be done because it is useless anyway, but I couldn't find an easy way to suppress it. This is an entirely different subject that is only *visible* if fl_text_extents() is used instead of fl_width() under Windows, but it is there with fl_width() as well.
I tried hard to speed things up, but for now I gave up. I spent way too much time on it. Maybe we should open another STR with these fl_text_extents() speed issues under Windows, but *for now* I can't see how we could accept your patch w/o intolerable speed deficiencies under Windows although it seems to be correct. | |
| |
| #5 | chris
02:37 Dec 27, 2017 |
| That's unfortunate. Sorry - not beeing a Windows user/developer, I could not test the patch under Windows.
Had some deeper look at the problem now.
What the method Fl_Text_Display::string_width() tries to compute, is the 'bounding width' of a text string. But this cannot be done with fl_width(), because fl_width() does only return the 'advance width' of a character/string (i.e. the width the cursor has to be advanced to step over the character/string), which in case of a bold or italic font is less than the bounding width. fl_text_extents() would probably be correct, but.. (see previous post from Albrecht).
Doing some research (for Windows), this seems to a problem many have faced already. AFAIS the generally accepted way to do it, would be to use the GetCharABCWidths functions to determine the underhang/overhang of the first/last character of the string and adjust the width calculated by GetCharWidth32 accordingly.
Maybe all this could be implemented in a new method, like fl_bwidth() or even fl_text_extents() could be reimplemented (and made faster thereby under Windows).
X11/Xft implementation can just be taken from fl_text_extents().
However it will not be me, who can do this (for Windows).
Some links that might get someone started:
The MSDN GetCharABCWidths documentation:
https://msdn.microsoft.com/en-us/library/vs/alm/dd183418(v=vs.85).aspx
Deep link into wxWidgets github repo text measure functions, which uses this approach (just search for 'abc'):
https://github.com/wxWidgets/wxWidgets/blob/master/src/msw/textmeasure.cpp | |
| |
| #6 | chris
09:33 Jan 04, 2018 |
| Attaching a patch - STR_3364.diff - that adresses the speed issue for the WIN32 platform and also fixes some side effects of the change in Fl_Text_Display (width of space character, wobbling of selected proportional text, cursor positioning in proportional text).
It uses the above mentioned GetCharABCWidths method, that is hopefully faster then the method used in the current fl_text_extents.
I could not just change the current fl_text_extents method for WIN32, because IMHO there is no other way than the current (slow) one to do the height measurements too. So I made it a variant of fl_text_extents, with parameters for width only. The non WIN32 platforms have a default implementation that calls the current fl_text_extents method as they have no speed issue with it.
I was able to test this patch with Windows XP in a VM and the speed is ok.
Tested also with Linux. | |
| |
| #7 | AlbrechtS
07:58 Jan 06, 2018 |
| Hi Chris, many thanks for the research and the patch. I'll take a look at it... | |
| |
| #8 | AlbrechtS
04:48 Mar 07, 2020 |
| FTR: Changed from RFE to Prio 3 (Moderate) and removed '?' from the title (aka summary). | |
| |
| #9 | AlbrechtS
14:22 Jan 26, 2022 |
| See related GitHub Issue comment:
https://github.com/fltk/fltk/pull/378#issuecomment-1022657695 | |
| |
| #10 | matt
15:03 Jan 27, 2022 |
| Fixed in Git repository.
The implementation is a bit different. The text size calculations are correct, but for proportional text, letters can overlap, and for italics, letters can leak out of their bounding box.
This solution draws all backgrounds of a line first, and then renders the text, so that nothing is clipped. Text can still leak, but it's not usually visible (unless there is a partial line update). | |
[ Return to Bugs & Features ]
|
| |
