Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> No it is not a recursion issue, but about knowing who asked for update. I

Then I did not get the underlying problem correctly, anyway proceed as you wish 
;)
P

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 2 novembre 2017 11:24:19 GMT+01:00, Pavel Sanda  a écrit :
>Jean-Marc Lasgouttes wrote:
>> Le 31 octobre 2017 13:36:47 GMT+01:00, Jean-Marc Lasgouttes
> a écrit :
>> >Yes 2 is good for fixing the symptom, but it is very fragile...
>> 
>> We could also try
>> 4. Note when we have requested a repaint and exit early from
>paintEvent() when we did not ask for this repaint. (Or even boldlier do
>a full metrics+draw).
>
>like this?
>
>int process=0; //0 - not updating; 1 - just in update; >1 update
>requested while in update
>fun update(){
>  //not a thread safe
>  if (process) {process++; return;}
>  process++;
>
>  do_updates();
>
>  process--;
>  if (process) {process=0; assynchronously_trigger_new_update();}
>  
>}

No it is not a recursion issue, but about knowing who asked for update. I would 
set a Boolean request_redraw in requestRedraw() and reset it to false after 
redraw is done.

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Le 31 octobre 2017 13:36:47 GMT+01:00, Jean-Marc Lasgouttes 
>  a écrit :
> >Yes 2 is good for fixing the symptom, but it is very fragile...
> 
> We could also try
> 4. Note when we have requested a repaint and exit early from paintEvent() 
> when we did not ask for this repaint. (Or even boldlier do a full 
> metrics+draw).

like this?

int process=0; //0 - not updating; 1 - just in update; >1 update requested 
while in update
fun update(){
  //not a thread safe
  if (process) {process++; return;}
  process++;

  do_updates();

  process--;
  if (process) {process=0; assynchronously_trigger_new_update();}
  
}

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 31 octobre 2017 13:36:47 GMT+01:00, Jean-Marc Lasgouttes 
 a écrit :
>Yes 2 is good for fixing the symptom, but it is very fragile...

We could also try
4. Note when we have requested a repaint and exit early from paintEvent() when 
we did not ask for this repaint. (Or even boldlier do a full metrics+draw).

That might work.

JMarc

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 31 octobre 2017 11:52:57 GMT+01:00, Pavel Sanda  a écrit :
>Jean-Marc Lasgouttes wrote:
>> Yes, this is needed in some cases. I see 3 types of fixes
>> 1. Convince qt that there is no need to update a whole window when
>the status bar changes
>> 2. Audit the uses of Buffer::message and use something else when
>possible.
>> 3. Change row painting code so that it does not have to use the
>actual paragraph contents. This was already on my to-do list for other
>reasons.
>
>I would probably end up with 2, but I am not really competent to give
>advices about the painting code...


Yes 2 is good for fixing the symptom, but it is very fragile...

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Yes, this is needed in some cases. I see 3 types of fixed
> 1. Convince qt that there is no need to update a whole window when the status 
> bar changes
> 2. Audit the uses of Buffer::message and use something else when possible.
> 3. Change row painting code so that it does not have to use the actual 
> paragraph contents. This was already on my to-do list for other reasons.

I would probably end up with 2, but I am not really competent to give advices 
about the painting code...

Pavel

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 30 octobre 2017 16:00:13 GMT+01:00, Pavel Sanda  a écrit :
>Jean-Marc Lasgouttes wrote:
>My guess is that if we do it generally for all cases some messaging
>gets
>broken (like the updates for longer lfuns - e.g. reconfiguration or
>compilation).

Yes, this is needed in some cases. I see 3 types of fixed
1. Convince qt that there is no need to update a whole window when the status 
bar changes
2. Audit the uses of Buffer::message and use something else when possible.
3. Change row painting code so that it does not have to use the actual 
paragraph contents. This was already on my to-do list for other reasons.

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> The patch does the right thing by using Cursor::message instead (so that 

I can confirm that the crash is fixed.

> where this may happen. I'd be grateful for ideas that would make 
> buffer::message and BufferView::message safer. It might be enough to just 
> delay the setting of the message in GuiView.

My guess is that if we do it generally for all cases some messaging gets
broken (like the updates for longer lfuns - e.g. reconfiguration or 
compilation).

Pavel

Re: Initial view of document (master)

[Kornel Benko]

Am Mittwoch, 25. Oktober 2017 um 15:24:33, schrieb Richard Heck 
> On 10/25/2017 02:35 PM, Kornel Benko wrote:
> > Am Dienstag, 24. Oktober 2017 um 19:51:41, schrieb Jean-Marc Lasgouttes 
> > 
> >> Le 23/10/2017 à 22:26, Pavel Sanda a écrit :
> >>> And I have reproducible crash:
> >>> 1. start new document
> >>> 2. write "ambititious ", spellcheck correctly underlies text.
> >>> 3. try to fix spelling via context menu, choose "ambitious"
> >>> 4. kaboom
> >> I can't reproduce. Qt4 or Qt5?
> >>
> >> JMarc
> > I can reproduce with continuous spellcheck enabled. Qt5.
> >
> > It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return 
> > false);
> >  at src/Paragraph.cpp:612
> > (gdb) p start
> > $1 = 0
> > (gdb) p end
> > $2 = 11
> > (gdb) p size()
> > $1 = 9
> >
> > Without this assert, everything works OK. Richard, why is it there needed?
> 
> "git blame" will blame me, but I only added the "return false". That
> way, in release mode, we just say "no change" and continue, hopefully
> safely. The assertion itself goes back to d53d4a5c, in 2006.
> 
> Presumably, the end variable does not really make sense here. Position
> 11 is past the end of the paragraph (and past the end of paragraph
> marker at pos=10). We're lucky, because Changes::isChanged isn't
> affected by this. Something is definitely wrong, though, and that's what
> the assertion is telling us.
> 
> Richard

The same assertion in 2.3.x does not trigger here.

(gdb) p start
$1 = 0
(gdb) p end
$2 = 10
(gdb) p size()
$3 = 9
.
(gdb) up
#1  0x00dfaae6 in lyx::RowPainter::paintChangeBar (this=0x7fffb290)
at /usr2/src/lyx/lyx-2.0.x-git/src/RowPainter.cpp:259
259 if (start == end || !par_.isChanged(start, end))
(gdb) p end
$4 = 10
(gdb) p par_.size()
$5 = 9


In lyx2.4, it is totally impossible to debug. One has to wait for crash first.

(gdb) up
#6  0x00dd94dc in lyx::Paragraph::isChanged (this=0x27ec370, start=0, 
end=11)
at /usr2/src/lyx/lyx-git/src/Paragraph.cpp:612
612 LASSERT(end > start && end <= size() + 1, return false);
(gdb) up
#7  0x00dfde10 in lyx::RowPainter::paintChangeBar (this=0x7fff5330)
at /usr2/src/lyx/lyx-git/src/RowPainter.cpp:259
259 if (start == end || !par_.isChanged(start, end))
(gdb) p par_.size()
$1 = 9
(gdb) p end
$2 = 11
(gdb) p row_.endpos()
$3 = 11

So the endpos() is different here.

Kornel
Kornel



signature.asc
Description: This is a digitally signed message part.

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 23/10/2017 à 22:26, Pavel Sanda a écrit :

And I have reproducible crash:
1. start new document
2. write "ambititious ", spellcheck correctly underlies text.
3. try to fix spelling via context menu, choose "ambitious"
4. kaboom


Here is a patch, that raises as many question as is solves. The issue is 
that lyxreplace uses Buffer::update, which does an update in the middle 
of a dispatch operation (it is necessary to look at the full backtrace 
to see what happens). This is of course terribly dangerous.


The patch does the right thing by using Cursor::message instead (so that 
the message is set at the end), but I am sure that there are other 
places where this may happen. I'd be grateful for ideas that would make 
buffer::message and BufferView::message safer. It might be enough to 
just delay the setting of the message in GuiView.


JMarc

PS: I will be in vacation for the next week, so don't expect more than 
replying to some mail during this time.
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 4e2428d7d7..c6c0cf17f7 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -82,6 +82,7 @@
 #include "support/lassert.h"
 #include "support/lstrings.h"
 #include "support/Package.h"
+#include "support/RefChanger.h"
 #include "support/types.h"
 
 #include 
@@ -2763,6 +2764,7 @@ void BufferView::updatePosCache()
 	// this is the "nodraw" drawing stage: only set the positions of the
 	// insets in metrics cache.
 	frontend::NullPainter np;
+	Changer chg = make_change(d->update_strategy_, FullScreenUpdate);
 	draw(np, false);
 }
 
diff --git a/src/Cursor.h b/src/Cursor.h
index 6c5e5e6783..955c702655 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -274,9 +274,9 @@ public:
 	bool getStatus(FuncRequest const & cmd, FuncStatus & flag) const;
 	/// dispatch from innermost inset upwards
 	void dispatch(FuncRequest const & cmd);
-	/// display a message
+	/// set a message to display after getStatus/dispatch
 	void message(docstring const & msg) const;
-	/// display an error message
+	/// set an error  message to display after getStatus/dispatch
 	void errorMessage(docstring const & msg) const;
 	/// get the resut of the last dispatch
 	DispatchResult const & result() const;
diff --git a/src/lyxfind.cpp b/src/lyxfind.cpp
index 4b8da77ac0..b5b3b7d921 100644
--- a/src/lyxfind.cpp
+++ b/src/lyxfind.cpp
@@ -391,19 +391,19 @@ bool lyxreplace(BufferView * bv,
 			replace_count = rv.second;
 		}
 
-		Buffer const & buf = bv->buffer();
+		Cursor & cur = bv->cursor();
 		if (!update) {
 			// emit message signal.
-			buf.message(_("String not found."));
+			cur.message(_("String not found."));
 		} else {
 			if (replace_count == 0) {
-buf.message(_("String found."));
+cur.message(_("String found."));
 			} else if (replace_count == 1) {
-buf.message(_("String has been replaced."));
+cur.message(_("String has been replaced."));
 			} else {
 docstring const str =
 	bformat(_("%1$d strings have been replaced."), replace_count);
-buf.message(str);
+cur.message(str);
 			}
 		}
 	} else if (findnext) {
@@ -412,7 +412,7 @@ bool lyxreplace(BufferView * bv,
 		if (findOne(bv, search, casesensitive, matchword, forward, true, findnext))
 			update = true;
 		else
-			bv->message(_("String not found."));
+			bv->cursor().message(_("String not found."));
 	}
 	return update;
 }

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 27/10/2017 à 15:14, Kornel Benko a écrit :

BTW, I get the same crash also with QT4.
Every time the word to correct is 2 chars longer as the wrong spelled.
For instance using 'tesest' and correct to 'test'. I used Hunspell,
but using Enchant or Aspell made no difference.


2 characters, of course ! I was typing "ambititous" instead.

I see it now.

JMarc

Re: Initial view of document (master)

[Kornel Benko]

Am Freitag, 27. Oktober 2017 um 14:37:08, schrieb Kornel Benko 
> > > I can reproduce with continuous spellcheck enabled. Qt5.
> > > 
> > > It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return 
> > > false);
> > >   at src/Paragraph.cpp:612
> > 
> > I still can't reproduce (with qt5). Can I have a backtrace? (I doubt 
> > that it will help, to be frank).
> > 
> > JMarc
> 

BTW, I get the same crash also with QT4.
Every time the word to correct is 2 chars longer as the wrong spelled.
For instance using 'tesest' and correct to 'test'. I used Hunspell,
but using Enchant or Aspell made no difference.

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: Initial view of document (master)

[Kornel Benko]

Am Freitag, 27. Oktober 2017 um 14:07:35, schrieb Jean-Marc Lasgouttes 

> Le 25/10/2017 à 20:35, Kornel Benko a écrit :
> > Am Dienstag, 24. Oktober 2017 um 19:51:41, schrieb Jean-Marc Lasgouttes 
> > 
> >> Le 23/10/2017 à 22:26, Pavel Sanda a écrit :
> >>> And I have reproducible crash:
> >>> 1. start new document
> >>> 2. write "ambititious ", spellcheck correctly underlies text.
> >>> 3. try to fix spelling via context menu, choose "ambitious"
> >>> 4. kaboom
> >>
> >> I can't reproduce. Qt4 or Qt5?
> > 
> > I can reproduce with continuous spellcheck enabled. Qt5.
> > 
> > It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return 
> > false);
> >   at src/Paragraph.cpp:612
> 
> I still can't reproduce (with qt5). Can I have a backtrace? (I doubt 
> that it will help, to be frank).
> 
> JMarc

Here it comes:
Assertion triggered in void lyx::doAssertWithCallstack(bool) by failing check 
"false" in file /usr2/src/lyx/lyx-git/src/support/lassert.cpp:44
 /home/kornel/newfile1.lyx.emergency

Program received signal SIGABRT, Aborted.
0x75295c37 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
56  ../nptl/sysdeps/unix/sysv/linux/raise.c: Adresár alebo súbor neexistuje.
(gdb) bt
#0  0x75295c37 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x75299028 in __GI_abort () at abort.c:89
#2  0x00d9ff9c in lyx::lyx_exit (exit_code=1) at 
/usr2/src/lyx/lyx-git/src/LyX.cpp:273
#3  0x00e96d81 in boost::assertion_failed (expr=0x19927ae "false", 
function=0x1992940  
"void lyx::doAssertWithCallstack(bool)", 
file=0x1992780 "/usr2/src/lyx/lyx-git/src/support/lassert.cpp", line=44) at 
/usr2/src/lyx/lyx-git/src/boost.cpp:47
#4  0x013dee0d in lyx::doAssertWithCallstack (value=false) at 
/usr2/src/lyx/lyx-git/src/support/lassert.cpp:44
#5  0x013deee9 in lyx::doAssert (expr=0x1460538 "end > start && end <= 
size() + 1", file=0x14604f0 "/usr2/src/lyx/lyx-git/src/Paragraph.cpp", 
line=612) at /usr2/src/lyx/lyx-git/src/support/lassert.cpp:53
#6  0x00dd950e in lyx::Paragraph::isChanged (this=0x2430550, start=0, 
end=11) at /usr2/src/lyx/lyx-git/src/Paragraph.cpp:612
#7  0x00dfde42 in lyx::RowPainter::paintChangeBar (this=0x7fff5320) 
at /usr2/src/lyx/lyx-git/src/RowPainter.cpp:259
#8  0x00e5d8fd in lyx::TextMetrics::drawParagraph (this=0x21552c8, 
pi=..., pit=0, x=0, y=49) at /usr2/src/lyx/lyx-git/src/TextMetrics.cpp:1948
#9  0x00e5cd76 in lyx::TextMetrics::draw (this=0x21552c8, pi=..., x=0, 
y=49) at /usr2/src/lyx/lyx-git/src/TextMetrics.cpp:1806
#10 0x00c8e4e8 in lyx::BufferView::draw (this=0x26d68e0, pain=..., 
paint_caret=true) at /usr2/src/lyx/lyx-git/src/BufferView.cpp:3125
#11 0x01147238 in lyx::frontend::GuiWorkArea::paintEvent 
(this=0x26f4450, ev=0x7fff5d70) at 
/usr2/src/lyx/lyx-git/src/frontends/qt4/GuiWorkArea.cpp:1240
#12 0x76ceef87 in QWidget::event(QEvent*) () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#13 0x76dcf58e in QFrame::event(QEvent*) () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#14 0x7634d181 in 
QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () 
from /usr/BUILD/BuildQtRoot/lib/libQt5Core.so.5
#15 0x76cab295 in QApplicationPrivate::notify_helper(QObject*, QEvent*) 
() from /usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#16 0x76cb2210 in QApplication::notify(QObject*, QEvent*) () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#17 0x010e2a5d in lyx::frontend::GuiApplication::notify 
(this=0x1f54530, receiver=0x2521bb0, event=0x7fff5d70) at 
/usr2/src/lyx/lyx-git/src/frontends/qt4/GuiApplication.cpp:2703
#18 0x7634d2b5 in QCoreApplication::notifyInternal2(QObject*, QEvent*) 
() from /usr/BUILD/BuildQtRoot/lib/libQt5Core.so.5
#19 0x76ce7dba in QWidgetPrivate::sendPaintEvent(QRegion const&) () 
from /usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#20 0x76ce83aa in QWidgetPrivate::drawWidget(QPaintDevice*, QRegion 
const&, QPoint const&, int, QPainter*, QWidgetBackingStore*) () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#21 0x76cb9d91 in ?? () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#22 0x76cbafe9 in ?? () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#23 0x76cda0df in QWidgetPrivate::syncBackingStore() () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#24 0x76ceebcd in QWidget::event(QEvent*) () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#25 0x76de4ecb in QMainWindow::event(QEvent*) () from 
/usr/BUILD/BuildQtRoot/lib/libQt5Widgets.so.5
#26 0x011196b9 in lyx::frontend::GuiView::event (this=0x28a3c60, 
e=0x7fff67d0) at /usr2/src/lyx/lyx-git/src/frontends/qt4/GuiView.cpp:1361
#27 0x76cab2bc in 

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 25/10/2017 à 20:35, Kornel Benko a écrit :

Am Dienstag, 24. Oktober 2017 um 19:51:41, schrieb Jean-Marc Lasgouttes 


Le 23/10/2017 à 22:26, Pavel Sanda a écrit :

And I have reproducible crash:
1. start new document
2. write "ambititious ", spellcheck correctly underlies text.
3. try to fix spelling via context menu, choose "ambitious"
4. kaboom


I can't reproduce. Qt4 or Qt5?


I can reproduce with continuous spellcheck enabled. Qt5.

It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return false);
  at src/Paragraph.cpp:612


I still can't reproduce (with qt5). Can I have a backtrace? (I doubt 
that it will help, to be frank).


JMarc

Re: Initial view of document (master)

[Richard Heck]

On 10/25/2017 02:35 PM, Kornel Benko wrote:
> Am Dienstag, 24. Oktober 2017 um 19:51:41, schrieb Jean-Marc Lasgouttes 
> 
>> Le 23/10/2017 à 22:26, Pavel Sanda a écrit :
>>> And I have reproducible crash:
>>> 1. start new document
>>> 2. write "ambititious ", spellcheck correctly underlies text.
>>> 3. try to fix spelling via context menu, choose "ambitious"
>>> 4. kaboom
>> I can't reproduce. Qt4 or Qt5?
>>
>> JMarc
> I can reproduce with continuous spellcheck enabled. Qt5.
>
> It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return false);
>  at src/Paragraph.cpp:612
> (gdb) p start
> $1 = 0
> (gdb) p end
> $2 = 11
> (gdb) p size()
> $1 = 9
>
> Without this assert, everything works OK. Richard, why is it there needed?

"git blame" will blame me, but I only added the "return false". That
way, in release mode, we just say "no change" and continue, hopefully
safely. The assertion itself goes back to d53d4a5c, in 2006.

Presumably, the end variable does not really make sense here. Position
11 is past the end of the paragraph (and past the end of paragraph
marker at pos=10). We're lucky, because Changes::isChanged isn't
affected by this. Something is definitely wrong, though, and that's what
the assertion is telling us.

Richard




Richard

Re: Initial view of document (master)

[Kornel Benko]

Am Dienstag, 24. Oktober 2017 um 19:51:41, schrieb Jean-Marc Lasgouttes 

> Le 23/10/2017 à 22:26, Pavel Sanda a écrit :
> > And I have reproducible crash:
> > 1. start new document
> > 2. write "ambititious ", spellcheck correctly underlies text.
> > 3. try to fix spelling via context menu, choose "ambitious"
> > 4. kaboom
> 
> I can't reproduce. Qt4 or Qt5?
> 
> JMarc

I can reproduce with continuous spellcheck enabled. Qt5.

It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return false);
 at src/Paragraph.cpp:612
(gdb) p start
$1 = 0
(gdb) p end
$2 = 11
(gdb) p size()
$1 = 9


Without this assert, everything works OK. Richard, why is it there needed?

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Le 23/10/2017 ?? 22:21, Pavel Sanda a écrit :
>> Another cursor issue (I have vague memory this was already discussed):
>> while editing paragraph of text go to the end of last word.
>> Hit spacebar - space is inserted, but cursor stays at the end of word
>> and shows only after inserting the next non whitespace character.
>
> This one? http://www.lyx.org/trac/ticket/10117

Yep. Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Le 23/10/2017 ?? 22:26, Pavel Sanda a écrit :
>> And I have reproducible crash:
>> 1. start new document
>> 2. write "ambititious ", spellcheck correctly underlies text.
>> 3. try to fix spelling via context menu, choose "ambitious"
>> 4. kaboom
>
> I can't reproduce. Qt4 or Qt5?

IIRC I got it home with qt4, I just got assertion on my work box with qt5 as 
well:
lassert.cpp (51): ASSERTION end > start && end <= size() + 1 VIOLATED IN 
Paragraph.cpp:612

Pavel

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 23/10/2017 à 22:26, Pavel Sanda a écrit :

And I have reproducible crash:
1. start new document
2. write "ambititious ", spellcheck correctly underlies text.
3. try to fix spelling via context menu, choose "ambitious"
4. kaboom


I can't reproduce. Qt4 or Qt5?

JMarc

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 23/10/2017 à 22:21, Pavel Sanda a écrit :

Another cursor issue (I have vague memory this was already discussed):
while editing paragraph of text go to the end of last word.
Hit spacebar - space is inserted, but cursor stays at the end of word
and shows only after inserting the next non whitespace character.


This one? http://www.lyx.org/trac/ticket/10117

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
>> After some testing:
>> - initial thread report present
>
> Which one?

https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg202482.html

>> - a is present
>
> I think it is gone now

Confirmed, this issue is gone.

>> - c mostly fixed, ghost c3*** still remains

See the attached file. Put caret into note.
Left arrow //you get out of note
Left arrow //you get in front of 'f', this is place where ghost remains
Left arrow //caret is correctly moved but ghost remains after 'f'

Can you see it now?

Pavel


pok.lyx
Description: application/lyx

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

I added an early return for Update::None that got forgotten.

Le 20/10/2017 à 16:56, Pavel Sanda a écrit :

After some testing:
- initial thread report present


Which one?


- a is present


I think it is gone now


- b is fixed
- c mostly fixed, ghost c3*** still remains


Cannot reproduce, but I am not sure of what it is.


Given that none of the remaining is critical I'd say go and commit,
I'll start new threads for new issues.


It is in now.

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Pavel Sanda wrote:
> Pavel Sanda wrote:
> > Jean-Marc Lasgouttes wrote:
> > > Le 19/10/2017 ?? 15:05, Pavel Sanda a écrit :
> > >> Pavel Sanda wrote:
> > >>> d) Editing problems
> > >>> Don't see how to produce solid recipy but something is wrong with 
> > >>> editing 
> > >>> as well.
> > >> Try couple times this one:
> > >> 1. open user guid, uncheck disable editing
> > >> 2. go to in front of "i" in "What is LyX"
> > >> 3. hit delete
> > >> 4. in most cases the character does not disappear although deleted once 
> > >> you move caret
> > >
> > > This (and the insert note crash) are fixed in this latest version. I send 
> > > it because I will not have much time on it until next Tuesday.
> > 
> > After some testing:
> > - initial thread report present
> > - a is present
> > - b is fixed
> > - c mostly fixed, ghost c3*** still remains
> 
> Another cursor issue (I have vague memory this was already discussed):
> while editing paragraph of text go to the end of last word.
> Hit spacebar - space is inserted, but cursor stays at the end of word
> and shows only after inserting the next non whitespace character.

And I have reproducible crash:
1. start new document
2. write "ambititious ", spellcheck correctly underlies text.
3. try to fix spelling via context menu, choose "ambitious"
4. kaboom

> Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Pavel Sanda wrote:
> Jean-Marc Lasgouttes wrote:
> > Le 19/10/2017 ?? 15:05, Pavel Sanda a écrit :
> >> Pavel Sanda wrote:
> >>> d) Editing problems
> >>> Don't see how to produce solid recipy but something is wrong with editing 
> >>> as well.
> >> Try couple times this one:
> >> 1. open user guid, uncheck disable editing
> >> 2. go to in front of "i" in "What is LyX"
> >> 3. hit delete
> >> 4. in most cases the character does not disappear although deleted once 
> >> you move caret
> >
> > This (and the insert note crash) are fixed in this latest version. I send 
> > it because I will not have much time on it until next Tuesday.
> 
> After some testing:
> - initial thread report present
> - a is present
> - b is fixed
> - c mostly fixed, ghost c3*** still remains

Another cursor issue (I have vague memory this was already discussed):
while editing paragraph of text go to the end of last word.
Hit spacebar - space is inserted, but cursor stays at the end of word
and shows only after inserting the next non whitespace character.

Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Le 19/10/2017 ?? 15:05, Pavel Sanda a écrit :
>> Pavel Sanda wrote:
>>> d) Editing problems
>>> Don't see how to produce solid recipy but something is wrong with editing 
>>> as well.
>> Try couple times this one:
>> 1. open user guid, uncheck disable editing
>> 2. go to in front of "i" in "What is LyX"
>> 3. hit delete
>> 4. in most cases the character does not disappear although deleted once 
>> you move caret
>
> This (and the insert note crash) are fixed in this latest version. I send 
> it because I will not have much time on it until next Tuesday.

After some testing:
- initial thread report present
- a is present
- b is fixed
- c mostly fixed, ghost c3*** still remains

Given that none of the remaining is critical I'd say go and commit,
I'll start new threads for new issues.
Pavel

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 19/10/2017 à 15:05, Pavel Sanda a écrit :

Pavel Sanda wrote:

d) Editing problems
Don't see how to produce solid recipy but something is wrong with editing as 
well.


Try couple times this one:
1. open user guid, uncheck disable editing
2. go to in front of "i" in "What is LyX"
3. hit delete
4. in most cases the character does not disappear although deleted once you 
move caret


This (and the insert note crash) are fixed in this latest version. I 
send it because I will not have much time on it until next Tuesday.


I do know that the "save after mouse wheel" bug is still present, but I 
have not manage to fix it yet. I did not have time to look at the other 
bugs.


JMarc
From 29d3eb6e0f67829774ff91dd5f06d0063c11d841 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes 
Date: Wed, 11 Oct 2017 18:00:48 +0200
Subject: [PATCH] Allow multiple calls to processUpdateFlags before redraw

The goal of this commit is to ensure that a processUpdateFlags call
that requires no redraw will not override a previous one that did
require a redraw.

To this end, the semantics of the flag argument is now different: its
value is now OR'ed with a private update_flags_ variable. This
variable is only reset after the buffer view has actually been
redrawn.

A new Update::ForceRedraw flag has been added. It requires a full
redraw but no metrics computation. It is not used in the main code
(yet), but avoids to compute metrics repeatedly in consecutive
processUpdateFlags calls.

The process is now as follows:
- the Force flag is honored (full metrics computation) and replaced
  with ForceDraw.
- the FitCursor flag is honored and removed from the flags.
- the SinglePar update is added if ForceDraw is not in flags and only
  the current par has been modified.

The remaining flags are only then added to the BufferView update
flags, and the update strategy is computed for the next paint event.

Finally the dubious call to updateMacros in updateMetrics has been
removed for performance reasons.
---
 development/PAINTING_ANALYSIS |  21 +++--
 src/BufferView.cpp| 176 ++
 src/BufferView.h  |   9 ++-
 src/TextMetrics.cpp   |   7 +-
 src/update_flags.h|  14 +++-
 5 files changed, 125 insertions(+), 102 deletions(-)

diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
index f734edb..ec3566e 100644
--- a/development/PAINTING_ANALYSIS
+++ b/development/PAINTING_ANALYSIS
@@ -60,12 +60,6 @@ cursor.
 
 * Clean-up of drawing code
 
-The goal is to make painting with drawing disable fast enough that it
-can be used after every metrics computation. Then we can separate real
-drawing from metrics.
-
-Other changes are only clean-ups.
-
 ** When a paragraph ends with a newline, compute correctly the height of the extra row.
 ** Merging bv::updateMetrics and tm::metrics
 
@@ -76,7 +70,7 @@ insets. We should re-use the bv::updateMetrics logic:
  + transfer all the logic of bv::updateMetrics to tm.
  + Main InsetText should not be special.
 
-The difficuly for a tall table cell for example, is that it may be
+The difficulty for a tall table cell for example, is that it may be
 necessary to break the whole contents to know the width of the cell.
 
 
@@ -113,11 +107,19 @@ DecorationUpdate). It triggers a recomputation of the metrics when either:
existing metrics. Note that the Update::SinglePar flag is *never*
taken into account.
 
+If a computation of metrics has taken place, Force is removed from the
+flags and ForceDraw is added instead.
+
+It is OK to call processUptateFlags several times before an update. In
+this case, the effects are cumulative.processUpdateFlags execute the
+metrics-related actions, but defers the actual drawing to the next
+paint event.
+
 The screen is drawn (with appropriate update strategy), except when
 update flag is Update::None.
 
 
-** Metrics computation
+** Metrics computation (and nodraw drawing phase)
 
 This is triggered by bv::updateMetrics, which calls tm::redoParagraph for
 all visible paragraphs. Some Paragraphs above or below the screen (needed
@@ -127,6 +129,9 @@ tm::redoParagraph will call Inset::metrics for each inset. In the case
 of text insets, this will invoke recursively tm::metrics, which redoes
 all the paragraphs of the inset.
 
+At the end of the function, bv::updatePosCache is called. It triggers
+a repaint of the document with a NullPainter (a painter that does
+nothing). This has the effect of caching all insets positions.
 
 ** Drawing the work area.
 
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 53d374f..153ab2d 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -228,7 +228,8 @@ enum ScreenUpdateStrategy {
 
 struct BufferView::Private
 {
-	Private(BufferView & bv) : update_strategy_(NoScreenUpdate),
+	Private(BufferView & bv) : update_strategy_(FullScreenUpdate),
+		update_flags_(Update::Force),
 		wh_(0), cursor_(bv),
 		

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 19/10/2017 à 14:59, Pavel Sanda a écrit :

Pavel Sanda wrote:

While doing the above steps in c) and jumping to different tabs or undoing I 
randomly get LyX crashing:
Assertion false violated in Coordcache.cpp:31.


Recipy is rather simple:
1. open new document
2. insert LyX note
3. kaboom


I got this one fixed.

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Thanks for testing. I complain, but I appreciate (time to find a shrink?)

I appreciate. Until the new painting stuff is merged into stable I need to stay 
with lyx 2.0...
Pavel

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 19/10/2017 à 14:38, Pavel Sanda a écrit :

Jean-Marc Lasgouttes wrote:

Could you have a go at this updated patch? I had to do now a rewrite of
processUpdateFlags that I planned to do later.


Indeed, the reported issues seem to be solved (qt5 test).

But I see new ones (all of these with applied patch, but they might apply to 
2.3 as well):


OK, OK, I have to look at these too :( The question is probably whether 
it is due to this patch or not. I'll see.


Thanks for testing. I complain, but I appreciate (time to find a shrink?)

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Pavel Sanda wrote:
> d) Editing problems
> Don't see how to produce solid recipy but something is wrong with editing as 
> well.

Try couple times this one:
1. open user guid, uncheck disable editing
2. go to in front of "i" in "What is LyX"
3. hit delete
4. in most cases the character does not disappear although deleted once you 
move caret

Re: Initial view of document (master)

[Pavel Sanda]

Pavel Sanda wrote:
> > While doing the above steps in c) and jumping to different tabs or undoing 
> > I randomly get LyX crashing:
> > Assertion false violated in Coordcache.cpp:31.

Recipy is rather simple:
1. open new document
2. insert LyX note
3. kaboom

Re: Initial view of document (master)

[Pavel Sanda]

Pavel Sanda wrote:
> But I see new ones (all of these with applied patch, but they might apply to 
> 2.3 as well):
> a)
> 1. edit document
> 2. scroll to different page via mouse wheel
> 3. hit ctrl+s to save
> 4. caret suddenly appears on screen on totally wrong position
>unrelated to current
> Reproducible?
> 
> b) Selection problems.
> 1. open some document, copy uncollapsed note inset
> 2. open new document, paste note inset
> 3. hit backscpace, the selection is done but _not_ painted.
>when playing with caret, the same sometimes happens together with delete,
>sometimes not, the pattern is not clear.
>I first discovered it when trying to delete note inset in c3.
> Reproducible?
> 
> c) Caret movement
> There is bunch of weird effects with note insets inside figure float.
> 1. insert figure float, paragraph of text into caption and then insert
>note inset at the very end. Do not put new line before inset.
> 2.   Put another paragraph of text into inset.
> 3. Now editing last line above inset:
>*) remove some word inside the line, the line shrinks instead of remaining 
> spanning across the whole row
>Reproducible?
>**) remove last character before the inset, the character gets deleted but 
> _stays_ on screen
>Reproducible?
>***) Moving caret inside inset and back outside few characters into 
> previous paragraph. Ghost caret
> stays on screen on the very end of line
>Reproducible?
>) Going into inset takes and out is not symmetric, sometimes caret 
> ends in front of painted 
>  note label sometimes it can make just to the last position of the 
> previous line and directly
>jumps into inset, depending on direction where you come from.
>Reproducible?
>  
> While doing the above steps in c) and jumping to different tabs or undoing I 
> randomly get LyX crashing:
> Assertion false violated in Coordcache.cpp:31.

d) Editing problems
Don't see how to produce solid recipy but something is wrong with editing as 
well.
Sometimes caret just jumps as I type at the end of line but no character is 
painted.
After typing few characters all of them suddenly appear. Similar with delete at 
the end of line.
This might actualy explain problem in c3 without need of insets to be present...

> Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Could you have a go at this updated patch? I had to do now a rewrite of 
> processUpdateFlags that I planned to do later.

Indeed, the reported issues seem to be solved (qt5 test).

But I see new ones (all of these with applied patch, but they might apply to 
2.3 as well):
a)
1. edit document
2. scroll to different page via mouse wheel
3. hit ctrl+s to save
4. caret suddenly appears on screen on totally wrong position
   unrelated to current
Reproducible?

b) Selection problems.
1. open some document, copy uncollapsed note inset
2. open new document, paste note inset
3. hit backscpace, the selection is done but _not_ painted.
   when playing with caret, the same sometimes happens together with delete,
   sometimes not, the pattern is not clear.
   I first discovered it when trying to delete note inset in c3.
Reproducible?

c) Caret movement
There is bunch of weird effects with note insets inside figure float.
1. insert figure float, paragraph of text into caption and then insert
   note inset at the very end. Do not put new line before inset.
2.   Put another paragraph of text into inset.
3. Now editing last line above inset:
   *) remove some word inside the line, the line shrinks instead of remaining 
spanning across the whole row
   Reproducible?
   **) remove last character before the inset, the character gets deleted but 
_stays_ on screen
   Reproducible?
   ***) Moving caret inside inset and back outside few characters into previous 
paragraph. Ghost caret
stays on screen on the very end of line
   Reproducible?
   ) Going into inset takes and out is not symmetric, sometimes caret ends 
in front of painted 
 note label sometimes it can make just to the last position of the 
previous line and directly
 jumps into inset, depending on direction where you come from.
   Reproducible?
 
While doing the above steps in c) and jumping to different tabs or undoing I 
randomly get LyX crashing:
Assertion false violated in Coordcache.cpp:31.

Pavel

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 18/10/2017 à 01:59, Pavel Sanda a écrit :

1. Mouse-wheel scrolling broken, scrolling via the scrollbar at right broken
2. Keys navigation broken (eg open manual and just hold down arrow up/down
for a while when it hits start and end), the same for many pg-up/downs.
3. Go to the end of document, try resize of lyx window (this one is not 100%
regular and depends on another unknown variable, but once you get there
pretty much any resize is broken; you need to play for a while).

There might be additional points, but difficult to distinguish them, too many
things broken at the same time...


Could you have a go at this updated patch? I had to do now a rewrite of 
processUpdateFlags that I planned to do later.


FWIW, the bugs listed above seems to be gone.

JMarc

From a02bfba3f56307fa08deaa2a4456092507f91d83 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes 
Date: Wed, 11 Oct 2017 18:00:48 +0200
Subject: [PATCH] Allow multiple calls to processUpdateFlags before redraw

The goal of this commit is to ensure that a processUpdateFlags call
that requires no redraw will not override a previous one that did
require a redraw.

To this end, the semantics of the flag argument is now different: its
value is now OR'ed with a private update_flags_ variable. This
variable is only reset after the buffer view has actually been
redrawn.

A new Update::ForceRedraw flag has been added. It requires a full
redraw but no metrics computation. It is not used in the main code
(yet), but avoids to compute metrics repeatedly in consecutive
processUpdateFlags calls.

The process is now as follows:
- the FitCursor flag is honored and removed from the flags
- the Force flag is nhonored (full metrics computation) and replaced
  with ForceDraw.

The remaining flags are only then added to the BufferView update
flags, and the update strategy is computed for the next paint event.

Finally the dubious call to updateMacros in updateMetrics has been
removed for performance reasons.
---
 development/PAINTING_ANALYSIS |  21 +++---
 src/BufferView.cpp| 148 +-
 src/BufferView.h  |   7 +-
 src/TextMetrics.cpp   |   7 +-
 src/update_flags.h|  14 +++-
 5 files changed, 104 insertions(+), 93 deletions(-)

diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
index f734edb..ec3566e 100644
--- a/development/PAINTING_ANALYSIS
+++ b/development/PAINTING_ANALYSIS
@@ -60,12 +60,6 @@ cursor.
 
 * Clean-up of drawing code
 
-The goal is to make painting with drawing disable fast enough that it
-can be used after every metrics computation. Then we can separate real
-drawing from metrics.
-
-Other changes are only clean-ups.
-
 ** When a paragraph ends with a newline, compute correctly the height of the extra row.
 ** Merging bv::updateMetrics and tm::metrics
 
@@ -76,7 +70,7 @@ insets. We should re-use the bv::updateMetrics logic:
  + transfer all the logic of bv::updateMetrics to tm.
  + Main InsetText should not be special.
 
-The difficuly for a tall table cell for example, is that it may be
+The difficulty for a tall table cell for example, is that it may be
 necessary to break the whole contents to know the width of the cell.
 
 
@@ -113,11 +107,19 @@ DecorationUpdate). It triggers a recomputation of the metrics when either:
existing metrics. Note that the Update::SinglePar flag is *never*
taken into account.
 
+If a computation of metrics has taken place, Force is removed from the
+flags and ForceDraw is added instead.
+
+It is OK to call processUptateFlags several times before an update. In
+this case, the effects are cumulative.processUpdateFlags execute the
+metrics-related actions, but defers the actual drawing to the next
+paint event.
+
 The screen is drawn (with appropriate update strategy), except when
 update flag is Update::None.
 
 
-** Metrics computation
+** Metrics computation (and nodraw drawing phase)
 
 This is triggered by bv::updateMetrics, which calls tm::redoParagraph for
 all visible paragraphs. Some Paragraphs above or below the screen (needed
@@ -127,6 +129,9 @@ tm::redoParagraph will call Inset::metrics for each inset. In the case
 of text insets, this will invoke recursively tm::metrics, which redoes
 all the paragraphs of the inset.
 
+At the end of the function, bv::updatePosCache is called. It triggers
+a repaint of the document with a NullPainter (a painter that does
+nothing). This has the effect of caching all insets positions.
 
 ** Drawing the work area.
 
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 53d374f..737f184 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -228,7 +228,8 @@ enum ScreenUpdateStrategy {
 
 struct BufferView::Private
 {
-	Private(BufferView & bv) : update_strategy_(NoScreenUpdate),
+	Private(BufferView & bv) : update_strategy_(FullScreenUpdate),
+		update_flags_(Update::Force),
 		wh_(0), cursor_(bv),
 		anchor_pit_(0), 

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> It was fixing the math completion thing, and other things that have not been 
> identified yet...

Ok I stay tuned for the next merge :) P

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 18 octobre 2017 02:05:12 GMT+02:00, Pavel Sanda  a écrit :
>Jean-Marc Lasgouttes wrote:
>> OK, I reverted it. 
>
>Was this commit fixing some issue or is it refactoring type of work
>to make things better in the future?
>
>Pavel

It was fixing the math completion thing, and other things that have not been 
identified yet...

JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> OK, I reverted it. 

Was this commit fixing some issue or is it refactoring type of work
to make things better in the future?

Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Le 17/10/2017 ?? 18:46, Pavel Sanda a écrit :
>> Bisect leads to c19c54dd5b85.
>> JMarc, this commit renders master editing/scrolling unusable.
>> (Qt5/debian stable).
>
> OK, I reverted it. Before you update master, I'd appreciate a list of other 
> buggy situations, so that I can do some damage control.

1. Mouse-wheel scrolling broken, scrolling via the scrollbar at right broken
2. Keys navigation broken (eg open manual and just hold down arrow up/down
   for a while when it hits start and end), the same for many pg-up/downs.
3. Go to the end of document, try resize of lyx window (this one is not 100%
   regular and depends on another unknown variable, but once you get there
   pretty much any resize is broken; you need to play for a while).

There might be additional points, but difficult to distinguish them, too many
things broken at the same time...
Pavel

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 17/10/2017 à 18:46, Pavel Sanda a écrit :

Bisect leads to c19c54dd5b85.
JMarc, this commit renders master editing/scrolling unusable.
(Qt5/debian stable).


OK, I reverted it. Before you update master, I'd appreciate a list of 
other buggy situations, so that I can do some damage control.


JMarc

Re: Initial view of document (master)

[Pavel Sanda]

Pavel Sanda wrote:
> Anyway you are right. Any editing in master seems to be seriously
> broken now, combo of painting issues and cursor shadows reported
> previously. After recent update it became so bad that I don't
> even know what to report first :)

Bisect leads to c19c54dd5b85.
JMarc, this commit renders master editing/scrolling unusable.
(Qt5/debian stable).

The original bug reported in the begining of this thread is
unrelated and was there even before c19c54dd5b85.

Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Pavel Sanda wrote:
> This is unrelated and likely connected to the problem JMarc is
> solving with Stephan on Mac. (But you run linux, right?)

Anyway you are right. Any editing in master seems to be seriously
broken now, combo of painting issues and cursor shadows reported
previously. After recent update it became so bad that I don't
even know what to report first :)

Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Kornel Benko wrote:
> I see something different:
> 1. Open document than spans more than single screen.
> 2. Resize the lyx window ==> black window

This is unrelated and likely connected to the problem JMarc is
solving with Stephan on Mac. (But you run linux, right?)

My problem is rather scrolling issue, not painting problem...

Pavel

Re: Initial view of document (master)

[Kornel Benko]

Am Dienstag, 17. Oktober 2017 um 17:55:41, schrieb Pavel Sanda 
> Hi,
> 
> I see regression wrt LyX 2.0.
> 1. Open document than spans more than single screen.
> 2. Set cursor at the very end of the document
> 3. Close LyX
> 4. Launch LyX and reopen the same document
> 5. The cursor position is set correcly, but the scrolling/view
>is wrong, i.e. I see on top last couple rows of text only, the
>rest of screen is unused.
> 
> Qt 5. I see it consistently, but different documents show
> different size of unused space. Random manual showed only
> half page (not that bad).
> This worked OK in 2.0.
> 
> Anyone can confirm?
> 
> Pavel

I see something different:
1. Open document than spans more than single screen.
2. Resize the lyx window ==> black window
3. Click into the window ==> nothing happens,
4. position with the raider, click in window ==> text reappears
5. position with the raider, ==> nothing happens,

Looks like the screen update is not often enough.

Kornel

signature.asc
Description: This is a digitally signed message part.