| | [ Return to Bugs & Features | Roadmap 1.1 | SVN ⇄ GIT ]
STR #1655
| 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: possible endless loop when switching buffers |
|---|
| Version: | 1.1-current |
|---|
| Created By: | AlbrechtS |
|---|
| Assigned To: | matt |
|---|
| Fix Version: | 1.1-current (SVN: v5793) |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
No files
Trouble Report Comments:
| |
| #1 | AlbrechtS
07:53 Apr 12, 2007 |
| Problem description: If there is a buffer associated with Fl_Text_Display, and this buffer holds more lines than fit into the current view, and then another buffer with fewer lines than the visible number of lines is attached to the same Fl_Text_Display, then Fl_Text_Display::resize() goes into an endless loop.
Version: FLTK 1.1, r5750, tested on Windows and Linux
This happens, because Fl_Text_Display::buffer() calls
buffer_modified_cb( 0, 0, mBuffer->length(), 0, 0, this );
without providing "deletedText" (param. #5).
buffer_modified_cb then uses deletedText:
linesDeleted = nDeleted == 0 ? 0 : countlines( deletedText );
to count the # of deleted lines, which results in linesDeleted == 0,
which is wrong.
That's why mNBufferLines gets out of sync, and later, when resize()
is called, the wrong number of lines leads to an endless loop in
resize() at line #490ff ...
// everything will fit in the viewport
if (mNBufferLines < mNVisibleLines || mBuffer == NULL || mBuffer->length() == 0)
scroll_(1, mHorizOffset);
/* if empty lines become visible, there may be an opportunity to
display more text by scrolling down */
else while (mLineStarts[mNVisibleLines-2] == -1)
scroll_(mTopLineNum-1, mHorizOffset);
... because mNBufferLines > mNVisibleLines, but scrolling does not help (see the last two lines of the source above.
The attached diff fixes this issue, but IMHO it wouldn't be useful to
(1) copy the complete text buffer,
(2) count the lines,
(3) subtract the # of lines (result: always 0),
(4) free the previously copied text again.
Maybe, NBufferLines should simply be set to 0 after calling buffer_modified_cb, or buffer_modified_cb should not be called at all, because at this point, the old buffer is being removed, and thus it might be enough to reset some variables to a known state ...
Albrecht
Attached is the working patch:
$ diff -u /xxx/fltk-1.1/src/Fl_Text_Display.cxx .
--- /xxx/fltk-1.1/src/Fl_Text_Display.cxx 2006-11-24 13:05:48.000000000 +0100
+++ ./Fl_Text_Display.cxx 2007-04-12 15:54:56.000000000 +0200
@@ -173,9 +173,15 @@
of the display and remove our callback from it */
if ( buf == mBuffer) return;
if ( mBuffer != 0 ) {
- buffer_modified_cb( 0, 0, mBuffer->length(), 0, 0, this );
+ char * del_txt = mBuffer->text();
+ buffer_modified_cb( 0, 0, mBuffer->length(), 0, del_txt, this );
+ free (del_txt);
mBuffer->remove_modify_callback( buffer_modified_cb, this );
mBuffer->remove_predelete_callback( buffer_predelete_cb, this );
+#ifdef DEBUG
+ printf ("*** Fl_Text_Display::buffer -01- mBuffer cleared, mNBufferLines=%d\n",
+ mNBufferLines);
+#endif // DEBUG
}
/* Add the buffer to the display, and attach a callback to the buffer for | |
| |
| #2 | AlbrechtS
09:51 Apr 12, 2007 |
| Short form of patch (setting only mNBufferLines), works for me, too:
$ svn diff Fl_Text_Display.cxx
Index: Fl_Text_Display.cxx
===================================================================
--- Fl_Text_Display.cxx (Revision 5750)
+++ Fl_Text_Display.cxx (Arbeitskopie)
@@ -174,6 +174,7 @@
if ( buf == mBuffer) return;
if ( mBuffer != 0 ) {
buffer_modified_cb( 0, 0, mBuffer->length(), 0, 0, this );
+ mNBufferLines = 0; // fix missing deletedText param., buffer removed
mBuffer->remove_modify_callback( buffer_modified_cb, this );
mBuffer->remove_predelete_callback( buffer_predelete_cb, this );
} | |
| |
| #3 | AlbrechtS
10:09 Apr 13, 2007 |
| Here is an extended patch (diff) for Fl_Text_Display.cxx. The second part (at line 1155 ff.) fixes a scrolling bug, if wrap mode is enabled and the last newline character is being deleted from the text buffer. This could also go to an endless loop (same scrolling problem).
The new (complete) patch is:
$ svn diff Fl_Text_Display.cxx
Index: Fl_Text_Display.cxx
===================================================================
--- Fl_Text_Display.cxx (revision 5750)
+++ Fl_Text_Display.cxx (working copy)
@@ -174,6 +174,7 @@
if ( buf == mBuffer) return;
if ( mBuffer != 0 ) {
buffer_modified_cb( 0, 0, mBuffer->length(), 0, 0, this );
+ mNBufferLines = 0; // fix missing deletedText param., buffer removed
mBuffer->remove_modify_callback( buffer_modified_cb, this );
mBuffer->remove_predelete_callback( buffer_predelete_cb, this );
}
@@ -1150,7 +1151,7 @@
while (true) {
lineStart = buf->line_start(pos);
wrapped_line_counter(buf, lineStart, pos, INT_MAX,
- true, 0, &retPos, &retLines, &retLineStart, &retLineEnd);
+ true, 0, &retPos, &retLines, &retLineStart, &retLineEnd, false);
if (retLines > nLines)
return skip_lines(lineStart, retLines-nLines,
true);
The missing "false" (for parameter countLastLineMissingNewLine, which defaults to true) counted a faked newline, and therefore the line counting did not correctly skip backwards.
Tested, as before, on windows an linux.
Albrecht | |
| |
| #4 | matt
07:13 May 01, 2007 |
| Um, do I have to apply all three patches, or only the last two-liner? | |
| |
| #5 | matt
14:09 May 01, 2007 |
| Fixed in Subversion repository.
He who can read has a clear advantage. Never mind, I got it ;-) | |
[ Return to Bugs & Features ]
|
| |
