STR #3395

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 ]

STR #3395

Application:FLTK Library
Status:1 - Closed w/Resolution
Priority:4 - High, e.g. key functionality not working
Scope:3 - Applies to all machines and operating systems
Subsystem:Core Library
Summary:Fl_Text_Display and Fl_Text_Editor has significant clipping on redraw when font sizes are changed.
Version:1.4-feature
Created By:deech
Assigned To:greg.ercolano
Fix Version:1.4.0 (SVN: v12530)
Fix Commit:2a3e86921094748dc1188e1bfe073c6fbdfec996
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:


Name/Time/Date Filename/Size top right image
 
#1 deech
06:36 Aug 11, 2017
Fl_Text_Display_Bug.tar.gz
0k
 
 
#2 greg.ercolano
11:02 Oct 25, 2017
fonts.cxx
5k
 
 
#3 greg.ercolano
11:03 Oct 25, 2017
str3395_screencapture.webm
165k
 
 
#4 greg.ercolano
09:11 Oct 26, 2017
str3395.patch
3k
 
bottom left image   bottom right image

Trouble Report Comments:


Name/Time/Date Text top right image
 
#1 deech
06:36 Aug 11, 2017
I was playing around with using Fl_Text_{Display, Editor} and found some issues with how the widget redraws itself; as the font size changes there is significant clipping and the spaces between the lines increases. To demonstrate I've attached an archive with a version of the "fonts" demo which uses a Fl_Text_Display instead of a custom Fl_Widget to display the text and a gif demonstrating the issue.

A workaround discovered by Greg Ercolano was not to use 'textobj->redraw()' but to use 'textobj->resize(textobj->x(),textobj->y(),textobj->w(), textobj->h());'
 
 
#2 greg.ercolano
11:02 Oct 25, 2017
Thanks -- I'm looking into this.

Your Fl_Text_Display_Bug.tar.gz is pretty large (~8MB), the original contents:

-rw-r--r-- deech/wheel 9059481 2017-08-10 06:04 fonts-text-display.gif
-rw-r--r-- deech/wheel    5214 2017-08-10 05:28 fonts.cxx

The gif file is a screen capture showing the problem, but is quite large (9MB).

For the sake of disk space, I'm going to replace the tar file's contents
with a single README file, and will make two separate attachments to replace it:

[EDIT: At first I tried a .mov, but later realized a .webm is better -erco]

    str3395_screencapture.webm -- 168k version of your .gif as an .webm
    fonts.cxx                  -- your example code

FWIW, the .webm file was created with this command on a linux machine:

    ffmpeg -f x11grab -r 25 -s 562x396  -i :0.0+0,24 -vcodec libvpx -b 512k str3395_screencapture.webm
 
 
#3 greg.ercolano
09:10 Oct 26, 2017
Fixed in Subversion repository.

OK, applied a fix to fltk 1.4.x r12529.
Pretty sure that solves the problem.

Attached as a patch too, to show the approach used.
 
 
#4 chris
09:42 Oct 27, 2017
This patch has problems. The font.cxx demo from the attachments crashes with it, as does the unittest program.

The culprits are the added calls to 'recalc_display()' from the setter methods of textfont() and textsize(). These are also called from the constructor of Fl_Text_Display, when not all values are initialised, leading to a crash.

IMHO the solution would be to remove these calls from the setters and put a call to 'recalc_display()' in the draw() method at the point where the damage(FL_DAMAGE_ALL) from Fl_Widget::redraw() is handled:

--- FL/Fl_Text_Display.H (Revision 12529)
+++ FL/Fl_Text_Display.H (Arbeitskopie)
@@ -293,7 +293,7 @@
    Sets the default font used when drawing text in the widget.
    \param s default text font face
    */
-  void textfont(Fl_Font s) {textfont_ = s; mColumnScale = 0; recalc_display(); }
+  void textfont(Fl_Font s) {textfont_ = s; mColumnScale = 0; }
   
   /**
    Gets the default size of text in the widget.
@@ -305,7 +305,7 @@
    Sets the default size of text in the widget.
    \param s new text size
    */
-  void textsize(Fl_Fontsize s) {textsize_ = s; mColumnScale = 0; recalc_display(); }
+  void textsize(Fl_Fontsize s) {textsize_ = s; mColumnScale = 0; }
   
   /**
    Gets the default color of text in the widget.
Index: src/Fl_Text_Display.cxx
===================================================================
--- src/Fl_Text_Display.cxx (Revision 12529)
+++ src/Fl_Text_Display.cxx (Arbeitskopie)
@@ -3663,6 +3663,7 @@
 
   // draw the non-text, non-scrollbar areas.
   if (damage() & FL_DAMAGE_ALL) {
+    recalc_display();
     //    printf("drawing all (box = %d)\n", box());
     if (Fl_Surface_Device::surface() != Fl_Display_Device::display_device()) {
       // if to printer, draw the background
 
 
#5 greg.ercolano
10:19 Oct 27, 2017
Confirmed unittest crash (though fonts.cxx ran OK for me in my tests on linux).
Committed your suggestion as r12530.

Investigated the unittest crash with the previous patch.
seems recalc_display() was being called before the scrollbars had been created
by the ctor, as the textsize() was being set before they'd been created.

#0  0x0000000000414ad4 in Fl_Widget::visible (this=0x1f9) at ../FL/Fl_Widget.H:657 <-- INVALID 'this' (scrollbar)
#1  0x000000000045a6be in Fl_Text_Display::recalc_display (this=0x7dbc60) at Fl_Text_Display.cxx:470
#2  0x000000000040b1b7 in Fl_Text_Display::textsize (this=0x7dbc60, s=14) at ../FL/Fl_Text_Display.H:308 <-- CTOR CALLS textsize()
#3  0x0000000000459c72 in Fl_Text_Display::Fl_Text_Display (this=0x7dbc60, X=180, Y=230, W=125, H=55, l=0x0) at Fl_Text_Display.cxx:115 <-- CTOR
#4  0x000000000040bf50 in SchemesTest::SchemesTest (this=0x7d9580, X=170, Y=25, W=520, H=365) at unittest_schemes.cxx:188
#5  0x000000000040b247 in SchemesTest::create () at unittest_schemes.cxx:59
#6  0x0000000000406c64 in UnitTest::create (this=0x6e7490) at unittests.cxx:70
#7  0x000000000040cf0a in main (argc=1, argv=0x7fffffffe348) at unittests.cxx:188

I should probably move the creation of the scrollbars higher in the ctor
so that their values aren't wild during the code that precedes them.
 
 
#6 greg.ercolano
10:24 Oct 27, 2017
Hmm, not sure I like r12530 either; recalc_display() can call redraw(),
so draw() -> recalc -> redraw() is probably a bad thing that could lead
to a possible recursion.

Investigating alternatives..
 
 
#7 chris
11:09 Oct 27, 2017
Yes, that looks dangerous, but I *think* there will be no recursion, because the program flow should not get to the redraw() condition on the second invocation..  
 
#8 greg.ercolano
12:51 Oct 27, 2017
Yes; I investigated alternatives, such as initializing the scrollbars
to zero at the top of the ctor, and having recalc_display() check if
they're NULL at the top.

But it really depends on the overall philosophy of Fl_Text_Display's
operation; should the internal values calculated by recalc_display()
be valid even before the next redraw? It seems like it should, so that
doing something like textsize(newvalue) followed by an operation that
checks to see how many lines are visible in the display will get the
right value, and not have to wait for the next draw(). We have that
problem in some widgets, and were able to solve by separating out the
recalc from draw().

But Fl_Text_Display is a large beast, and there is no developer document
describing its overall implementation philosophy, and it's a bitch to
get one's head around.

I guess I'll stick with the r12530 and let's see how that goes..
leaving this open in case I have more time to dig into it.
 
 
#9 AlbrechtS
04:22 Mar 07, 2020
FTR: Git commit of svn r12530: 2a3e86921094748dc1188e1bfe073c6fbdfec996  
 
#10 AlbrechtS
04:26 Mar 07, 2020
This STR has been resolved more than two years ago. I tested the behavior of the posted file #2 (fonts.cxx) with FLTK 1.4-current (Git) and it worked well (didn't show the reported issue).

Closing this STR. If anybody sees any issues please reopen or open a new STR (or GitHub issue).

Closed w/Resolution.
 
bottom left image   bottom right image

Return to Bugs & Features ]

 
 

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'.