| | [ Return to Bugs & Features | SVN ⇄ GIT ]
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: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #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. | |
[ Return to Bugs & Features ]
|
| |
