STR #3364

GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

Return to Bugs & Features | Post Text | Post File | Prev | Next ]

STR #3364

Application:FLTK Library
Status:5 - New
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:Unassigned
Fix Version:Unassigned
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:

Post File ]
Name/Time/Date Filename/Size top right image
 
#1 chris
00:03 Feb 06, 2017
editor_wrong_text_width.png
27k
 
 
#2 chris
00:04 Feb 06, 2017
text_display_text_width.diff
0k
 
 
#3 chris
09:34 Jan 04, 2018
STR_3364.diff
8k
 
bottom left image   bottom right image

Trouble Report Comments:

Post Text ]
Name/Time/Date Text top right image
 
#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).  
bottom left image   bottom right image

Return to Bugs & Features | Post Text | Post File ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2021 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.