nested updates
To our local Qt professionals: Why do we need [EMAIL PROTECTED]:/usr/src/lyx/lyx-devel/src > grep -R processEve * frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QMathDialog.C:qApp->processEvents(); frontends/qt2/QMathDialog.C:qApp->processEvents(); frontends/qt2/lyx_gui.C:qApp->processEvents(); frontends/qt2/QLPopupMenu.C:qApp->processEvents(); One of these seems to be the cause of nested update() calls... Andre'
Re: nested updates
Andre Poenitz wrote: frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QDialogView.h:qApp->processEvents(); frontends/qt2/QMathDialog.C:qApp->processEvents(); frontends/qt2/QMathDialog.C:qApp->processEvents(); frontends/qt2/lyx_gui.C:qApp->processEvents(); frontends/qt2/QLPopupMenu.C:qApp->processEvents(); Take then out, and see what happens... I believe this is a fundamental problem. It does not make sense to update while updating. If it doesn't work without these processEvents, then the dispatcher should queue lyx func requests up, and handle them when the update is completely. Also, the updater can be extended to abort when the coord cache is full, before drawing, if a pending update is in the queue. Regards, Asger
Re: nested updates
Asger Ottar Alstrup wrote:
Andre Poenitz wrote:
frontends/qt2/QDialogView.h:qApp->processEvents();
frontends/qt2/QDialogView.h:qApp->processEvents();
frontends/qt2/QDialogView.h:qApp->processEvents();
frontends/qt2/QDialogView.h:qApp->processEvents();
frontends/qt2/QMathDialog.C:qApp->processEvents();
frontends/qt2/QMathDialog.C:qApp->processEvents();
frontends/qt2/lyx_gui.C:qApp->processEvents();
frontends/qt2/QLPopupMenu.C:qApp->processEvents();
If it doesn't work without these processEvents, then the dispatcher
should queue lyx func requests up, and handle them when the update is
completely.
Thinking about this a bit more: An alternative approach is to flag that
we need to postpone the processEvents() calls after the update. Replace
the calls above with a call to a function that does:
if (updating) {
needProcessEvent = true;
} else {
qApp->processEvents();
}
and at the bottom of update, do:
while (needProcessEvent) {
needProcessEvent = false;
qApp->processEvents();
}
Assuming that the nested updates come from processEvent, this approach
solves the problem by delaying the processEvent just enough to be able
to handle it. It is simpler to implement than queing of lyx funcs, and
it is also more general, since some of the problematic things might not
be lyx funcs.
Also, I think there is less of a risk of infinite loops this way.
Regards,
Asger
Re: [Patch] Table crash blues
On Sat, May 07, 2005 at 09:21:20PM +0100, Angus Leeming wrote: > Andre Poenitz wrote: > > [Actually, there seems to be some bad Qt GUI interaction involved as the > > problem shows only up when having certain dialogs open. Otherwise, plain > > full redraw seems to be fast enough...] > > You reckon it's fast enough? 14x is *much* slower than 13x when using over > an ADSL line. I haven't tried that, but local xform is certainly usably very well speed-wise. We could save some network traffic by restricting actual drawing to some horizontal strip as we did in 1.3, but this would be new code, the old logic does not fit anymore. Andre'
Re: [Patch] Table crash blues
On Sat, May 07, 2005 at 11:23:03PM +0200, Helge Hafting wrote: > > > Coarse screen updates when the cpu can't keep up is much better > > > than detailed but lagging updates. > > > > And our real problem is that our updates do not serve only the > > purpose of showing something on screen, but also to put the > > coordcache in some sensible state. This can't be skipped if we > > expect e.g. to work even while typing fast. > > I see. The coord cache must be updated, but I hope the actual drawing > can be skipped ??? when lyx knows that it is behind. That improves > performance in many cases, such as an overloaded machine or X over a > slow network. The coord cache is filled in the drawing phase (the metrics phase fills the size part of the coord cache only). We _do_ have a 'null painter' which only fills the cache, but if we did metrics + null painter painting + real painting in every case, drawing might get slow. Just a suspicion, though. Andre'
Re: [Patch] Table crash blues
Andre Poenitz wrote: I see. The coord cache must be updated, but I hope the actual drawing can be skipped ??? when lyx knows that it is behind. That improves performance in many cases, such as an overloaded machine or X over a slow network. The coord cache is filled in the drawing phase (the metrics phase fills the size part of the coord cache only). We _do_ have a 'null painter' which only fills the cache, but if we did metrics + null painter painting + real painting in every case, drawing might get slow. Just a suspicion, though. Hmmm, I would have expected that the only expensive operations is the real painting (provided that the null painter does not invoke any X/Qt methods). Is it possible to combine metrics & null painter? Without knowing the details, I think the only clean solution is to avoid recursive/reentrant updates completely. I always prefer stability over performance. Michael
Re: [Patch] Table crash blues
Andre Poenitz wrote: The coord cache is filled in the drawing phase (the metrics phase fills the size part of the coord cache only). We _do_ have a 'null painter' which only fills the cache, but if we did metrics + null painter painting + real painting in every case, drawing might get slow. Just a suspicion, though. An idea to fix the update over ADSL in a general way, in a way which does not require the kernel to know about the extend a change has on the screen: The null painting phase can be used to detect what really needs to be updated. During an update, we built a fingerprint hash of each painting operation: a hash based on the content and the operation itself. This finger-print is stored in a data-structure indexed by the rectangle the operation covers. So, each update results in a set of fingerprints, which exactly describe what was drawn at each pixel on the screen: All pixels in the affected rectangle has that hash associated. Now we need to update the screen, because something changed. First, we draw with the null-painter. For each operation, we also calculate the fingerprint and rectangles like above. So, now we have two data-structures of rectangles with their associated fingerprints. We compare the old and the new entires. All the different entries tell us what really needs to be updated. Thus, we build a set of rectangles which tell us what parts of the screen really needs an update. Then we fill these parts of the screen with the background color. Next, we start the second drawing phase with a real painter: For all requests, we check whether we intersect a rectangle which needs an update. If not, we just skip it. If we do, we do the drawing as requested. Regards, Asger
Re: unneeded update() calls
Am Samstag, 7. Mai 2005 21:35 schrieb Andre Poenitz:
>
> Could anybody please make sure that 'his' code does not contain
> unnecessary update() calls?
>
> As a first approximation I'd classify any update() with exception
> of the one in lyxfunc.C as 'unnecessary'.
>
> Most notably the LFUN_INSET_MODIFY handling seems to have sneaked
> something in...
All update() calls in src/insets are unnecessary. They are all in
do_dispatch(), and LCursor->dispatch() makes sure that update is called.
It is the other way round: We need to call cur.noUpdate() if no update is
needed.
I did also call noUpdate() instead of undispatched() in insetcommand.C. Is
that correct?
Georg
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetcommand.C lyx-1.4-cvs/src/insets/insetcommand.C
--- lyx-1.4-clean/src/insets/insetcommand.C 2005-04-23 10:15:05.0 +0200
+++ lyx-1.4-cvs/src/insets/insetcommand.C 2005-05-08 10:47:07.0 +0200
@@ -109,12 +109,10 @@ void InsetCommand::doDispatch(LCursor &
case LFUN_INSET_MODIFY: {
InsetCommandParams p;
InsetCommandMailer::string2params(mailer_name_, cmd.argument, p);
- if (p.getCmdName().empty()) {
- cur.undispatched();
- } else {
+ if (p.getCmdName().empty())
+ cur.noUpdate();
+ else
setParams(p);
- cur.bv().update();
- }
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetfloat.C lyx-1.4-cvs/src/insets/insetfloat.C
--- lyx-1.4-clean/src/insets/insetfloat.C 2005-04-23 10:15:08.0 +0200
+++ lyx-1.4-cvs/src/insets/insetfloat.C 2005-05-08 10:44:43.0 +0200
@@ -160,7 +160,6 @@ void InsetFloat::doDispatch(LCursor & cu
params_.sideways = params.sideways;
wide(params_.wide, cur.buffer().params());
sideways(params_.sideways, cur.buffer().params());
- cur.bv().update();
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetgraphics.C lyx-1.4-cvs/src/insets/insetgraphics.C
--- lyx-1.4-clean/src/insets/insetgraphics.C 2005-04-23 10:15:08.0 +0200
+++ lyx-1.4-cvs/src/insets/insetgraphics.C 2005-05-08 10:43:21.0 +0200
@@ -197,10 +206,10 @@ void InsetGraphics::doDispatch(LCursor &
Buffer const & buffer = cur.buffer();
InsetGraphicsParams p;
InsetGraphicsMailer::string2params(cmd.argument, buffer, p);
- if (!p.filename.empty()) {
+ if (!p.filename.empty())
setParams(p);
- cur.bv().update();
- }
+ else
+ cur.noUpdate();
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetinclude.C lyx-1.4-cvs/src/insets/insetinclude.C
--- lyx-1.4-clean/src/insets/insetinclude.C 2005-04-23 10:15:17.0 +0200
+++ lyx-1.4-cvs/src/insets/insetinclude.C 2005-05-08 10:43:57.0 +0200
@@ -128,10 +153,10 @@ void InsetInclude::doDispatch(LCursor &
case LFUN_INSET_MODIFY: {
InsetCommandParams p;
InsetIncludeMailer::string2params(cmd.argument, p);
- if (!p.getCmdName().empty()) {
+ if (!p.getCmdName().empty())
set(p, cur.buffer());
- cur.bv().update();
- }
+ else
+ cur.noUpdate();
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetnote.C lyx-1.4-cvs/src/insets/insetnote.C
--- lyx-1.4-clean/src/insets/insetnote.C 2005-04-23 10:15:17.0 +0200
+++ lyx-1.4-cvs/src/insets/insetnote.C 2005-05-08 10:45:43.0 +0200
@@ -192,7 +192,6 @@ void InsetNote::doDispatch(LCursor & cur
case LFUN_INSET_MODIFY:
InsetNoteMailer::string2params(cmd.argument, params_);
setButtonLabel();
- cur.bv().update();
break;
case LFUN_INSET_DIALOG_UPDATE:
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetwrap.C lyx-1.4-cvs/src/insets/insetwrap.C
--- lyx-1.4-clean/src/insets/insetwrap.C 2005-04-23 10:15:17.0 +0200
+++ lyx-1.4-cvs/src/insets/insetwrap.C 2005-05-08 10:45:51.0 +0200
@@ -83,7 +83,6 @@ void InsetWrap::doDispatch(LCursor & cur
InsetWrapMailer::string2params(cmd.argument, params);
params_.placement = params.placement;
params_.width = params.width;
- cur.bv().update();
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/updatableinset.C lyx-1.4-cvs/src/insets/updatableinset.C
--- lyx-1.4-clean/src/insets/updatableinset.C 2005-02-01 20:16:20.0 +0100
+++ lyx-1.4-cvs/src/insets/updatableinset.C 2005-05-08 10:44:22.0 +0200
@@ -99,8 +99,8 @@ void UpdatableInset::doDispatch(LCursor
scroll(cur.bv(), static_cast(convert(cmd.argument)));
else
scroll(cur.bv(), convert(cmd.argument));
- cur.bv().update();
- }
+ } else
+ cur.noUpdate();
break;
default:
Re: unneeded update() calls
Georg Baum wrote: All update() calls in src/insets are unnecessary. They are all in do_dispatch(), and LCursor->dispatch() makes sure that update is called. It is the other way round: We need to call cur.noUpdate() if no update is needed. I did also call noUpdate() instead of undispatched() in insetcommand.C. Is that correct? Looks right to me. Maybe you could add some comments about what the protocol for update/noupdate calls is? Add it to the BufferView::update method, or wherever the main update call is. In general, I think the new code should be commented some more. Maybe you do not understand exactly how it is supposed to work, but it is still better to add a comment about how you understand it than not adding something. Then, the people that understand it can post patches to the comments. Regards, Asger
Re: [Patch] Table crash blues
Asger Ottar Alstrup wrote: Andre Poenitz wrote: The coord cache is filled in the drawing phase (the metrics phase fills the size part of the coord cache only). We _do_ have a 'null painter' which only fills the cache, but if we did metrics + null painter painting + real painting in every case, drawing might get slow. Just a suspicion, though. An idea to fix the update over ADSL in a general way, in a way which does not require the kernel to know about the extend a change has on the screen: The null painting phase can be used to detect what really needs to be updated. During an update, we built a fingerprint hash of each painting operation: a hash based on the content and the operation itself. This finger-print is stored in a data-structure indexed by the rectangle the operation covers. IMHO all this falls into category "optimization". It's nice to think about it but first the real problem has to be solved. My impression - from a perspective of guy who knows nothing about the details - the additional null painter step looks reasonable. Regards, Michael
Re: [Patch] Table crash blues
Michael Schmitt wrote: IMHO all this falls into category "optimization". It's nice to think about it but first the real problem has to be solved. My impression - from a perspective of guy who knows nothing about the details - the additional null painter step looks reasonable. For sure it is an optimisation, but it might be justified since it is a severe regression compared to 1.3. Also, notice the null painter step in itself will not solve the crashes. Introducing it to avoid excessive repaints due to nested update calls is only a band-aid. A better fix is to go to the root of the cause, and in this case, as mentioned in another post, I believe the right fix is to either omit or postpone the qApps->processEvent calls during update. Of course, excessive update calls from other places, as found by George, should also be nailed. (Notice, that the optimisation as sketched out would reduce the cost of superfluous update calls to a metrics and null-paint job only - the outcome would be identical to before, and no painting on screen would be necessary). Regards, Asger
Re: [Patch] Table crash blues
Asger Ottar Alstrup wrote: A better fix is to go to the root of the cause, and in this case, as mentioned in another post, I believe the right fix is to either omit or postpone the qApps->processEvent calls during update. Of course, excessive update calls from other places, as found by George, should also be nailed. (Notice, that the optimisation as sketched out would reduce the cost of superfluous update calls to a metrics and null-paint job only - the outcome would be identical to before, and no painting on screen would be necessary). So the "best" solution would be: 1. Run metrics computation & null painter as a single non-interruptable, non-reentrant step (There should be some assertions in the code that check that update isn't executed twice simultaneously; this should be easily doable by the introduction of a static variable that is incremented when entering update and decrement when leaving update) 2. At the end of metrics computation & null painter, check whether there is a new event 3a. If yes, process the event, skip real painting 3b. If no, go on with real painting Since real painting is suppressed if the user edits text very fast, this solution should be more efficient than the current one. Michael
Re: [Patch] Table crash blues
Michael Schmitt wrote: So the "best" solution would be: 1. Run metrics computation & null painter as a single non-interruptable, non-reentrant step (There should be some assertions in the code that check that update isn't executed twice simultaneously; this should be easily doable by the introduction of a static variable that is incremented when entering update and decrement when leaving update) There already is such an assert. It is based on a bool, not a counter. There was a patch to change it to a counter, but I think that is wrong. The first time, we detect a reentrancy, we have the problem, and it should be fixed. 2. At the end of metrics computation & null painter, check whether there is a new event 3a. If yes, process the event, skip real painting 3b. If no, go on with real painting Since real painting is suppressed if the user edits text very fast, this solution should be more efficient than the current one. Yes, something like this should work better. This can be done in steps: First, just leave the excessive painting as it is, but fix the re-entrancy problem. Then, as a second step, introduce the null-painter as an optimisation for the cases where two sequentive updates are coming right after each other, such as fast typing, which allows us to skip the first paint job. The third step could be to build something like the update fingerprinting data-structure, as described earlier, which will help painting only what needs to be painted, and also help mitigate any excessive update calls there might be. In parallel to this, all excessive update calls should be hunted down, and killed without mercy. Regards, Asger
Re: [Patch] Table crash blues
Asger Ottar Alstrup wrote: (There should be some assertions in the code that check that update isn't executed twice simultaneously; this should be easily doable by the introduction of a static variable that is incremented when entering update and decrement when leaving update) There already is such an assert. It is based on a bool, not a counter. There was a patch to change it to a counter, but I think that is wrong. You are right. A boolean is enough. Are you sure that the assert is already in the CVS repository? First, just leave the excessive painting as it is, but fix the re-entrancy problem. Then, as a second step, introduce the null-painter ... The third step could be to build something like the update fingerprinting data-structure ... In parallel to this, all excessive update calls should be hunted down, and killed without mercy. This sounds like an excellent approach. Michael
Re: unneeded update() calls
Am Sonntag, 8. Mai 2005 11:26 schrieb Asger Ottar Alstrup:
> Maybe you could add some comments about what the protocol for
> update/noupdate calls is? Add it to the BufferView::update method, or
> wherever the main update call is.
There is already a comment in LCursor::dispatch. In fact I got my
knowledge from there :-) I added a more detailed one to
InsetBase::doDispatch(), because this is the place where inset developers
look.
> In general, I think the new code should be commented some more. Maybe
> you do not understand exactly how it is supposed to work, but it is
> still better to add a comment about how you understand it than not
> adding something. Then, the people that understand it can post patches
> to the comments.
I agree, and I added comments whenever I found it appropriate in the past.
I am going to commit the attached version of nobody objects.
Georg
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/ChangeLog lyx-1.4-cvs/src/insets/ChangeLog
--- lyx-1.4-clean/src/insets/ChangeLog 2005-05-08 11:59:27.0 +0200
+++ lyx-1.4-cvs/src/insets/ChangeLog 2005-05-08 12:41:25.0 +0200
@@ -1,3 +1,11 @@
+2005-05-08 Georg Baum <[EMAIL PROTECTED]>
+
+ * insetbase.h (doDispatch): document a bit more
+ * insetcommand.C, insetfloat.C, insetgraphics.C, insetinclude.C,
+ insetnote.C, insetwrap.C, updatableinset.C (doDispatch): don't call
+ cur.bv().update(), because that leads to nested updates. Call
+ cur.noUpdate() instead where approriate.
+
2005-05-07 Michael Schmitt <[EMAIL PROTECTED]>
* insetbibtex.C: change screen label
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetbase.h lyx-1.4-cvs/src/insets/insetbase.h
--- lyx-1.4-clean/src/insets/insetbase.h 2005-04-23 10:15:04.0 +0200
+++ lyx-1.4-cvs/src/insets/insetbase.h 2005-05-08 12:27:43.0 +0200
@@ -402,8 +406,17 @@ public:
protected:
InsetBase();
InsetBase(InsetBase const &);
- /// the real dispatcher.
- /// \sa getStatus
+ /** The real dispatcher.
+ * Gets normally called from LCursor::dispatch(). LCursor::dispatch()
+ * assumes the common case of 'LFUN handled, need update'.
+ * This has to be overriden by calling LCursor::undispatched() or
+ * LCursor::noUpdate() if appropriate.
+ * If you need to call the dispatch method of some inset directly
+ * you may have to explicitly request an update at that place. Don't
+ * do it in doDispatch(), since that causes nested updates when
+ * called from LCursor::dispatch(), and these can lead to crashes.
+ * \sa getStatus
+ */
virtual void doDispatch(LCursor & cur, FuncRequest & cmd);
private:
virtual std::auto_ptr doClone() const = 0;
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetcommand.C lyx-1.4-cvs/src/insets/insetcommand.C
--- lyx-1.4-clean/src/insets/insetcommand.C 2005-04-23 10:15:05.0 +0200
+++ lyx-1.4-cvs/src/insets/insetcommand.C 2005-05-08 10:47:07.0 +0200
@@ -109,12 +109,10 @@ void InsetCommand::doDispatch(LCursor &
case LFUN_INSET_MODIFY: {
InsetCommandParams p;
InsetCommandMailer::string2params(mailer_name_, cmd.argument, p);
- if (p.getCmdName().empty()) {
- cur.undispatched();
- } else {
+ if (p.getCmdName().empty())
+ cur.noUpdate();
+ else
setParams(p);
- cur.bv().update();
- }
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetfloat.C lyx-1.4-cvs/src/insets/insetfloat.C
--- lyx-1.4-clean/src/insets/insetfloat.C 2005-04-23 10:15:08.0 +0200
+++ lyx-1.4-cvs/src/insets/insetfloat.C 2005-05-08 10:44:43.0 +0200
@@ -160,7 +160,6 @@ void InsetFloat::doDispatch(LCursor & cu
params_.sideways = params.sideways;
wide(params_.wide, cur.buffer().params());
sideways(params_.sideways, cur.buffer().params());
- cur.bv().update();
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetgraphics.C lyx-1.4-cvs/src/insets/insetgraphics.C
--- lyx-1.4-clean/src/insets/insetgraphics.C 2005-04-23 10:15:08.0 +0200
+++ lyx-1.4-cvs/src/insets/insetgraphics.C 2005-05-08 10:43:21.0 +0200
@@ -197,10 +206,10 @@ void InsetGraphics::doDispatch(LCursor &
Buffer const & buffer = cur.buffer();
InsetGraphicsParams p;
InsetGraphicsMailer::string2params(cmd.argument, buffer, p);
- if (!p.filename.empty()) {
+ if (!p.filename.empty())
setParams(p);
- cur.bv().update();
- }
+ else
+ cur.noUpdate();
break;
}
diff -p -r -U 3 -X excl.tmp lyx-1.4-clean/src/insets/insetinclude.C lyx-1.4-cvs/src/insets/insetinclude.C
--- lyx-1.4-clean/src/insets/insetinclude.C 2005-04-23 10:15:17.0 +0200
+++ lyx-1.4-cvs/src/insets/insetinclude.C 2005-05-08 10:43:57.0 +0200
@@ -128,10 +153,10 @@ void InsetInclude::doDispatch(LCursor &
case LFUN_INSET_MODIFY: {
InsetCommandParams p;
InsetIncludeMailer::string2params(cmd.argument, p);
- if (!p.getCmdName().empty()) {
+ if (!p.getCmdName().empty())
set(p, cur.buffer());
- cur.bv().update();
- }
+ else
+ cur.noUpdate();
b
Re: nested updates
Andre Poenitz wrote: > frontends/qt2/QDialogView.h: qApp->processEvents(); > frontends/qt2/QDialogView.h: qApp->processEvents(); > frontends/qt2/QDialogView.h: qApp->processEvents(); > frontends/qt2/QDialogView.h: qApp->processEvents(); No idea why/if they are needed, but I cannot reproduce the Asserts when I comment them out. Jürgen
Re: [Patch] Table crash blues
Andre Poenitz wrote: > On Sat, May 07, 2005 at 09:21:20PM +0100, Angus Leeming wrote: >> Andre Poenitz wrote: >> > [Actually, there seems to be some bad Qt GUI interaction involved as >> > [the >> > problem shows only up when having certain dialogs open. Otherwise, >> > plain full redraw seems to be fast enough...] >> >> You reckon it's fast enough? 14x is *much* slower than 13x when using >> over an ADSL line. > > I haven't tried that, but local xform is certainly usably very well > speed-wise. > > We could save some network traffic by restricting actual drawing to some > horizontal strip as we did in 1.3, but this would be new code, the old > logic does not fit anymore. And then, we 1.3.x only saved redraws when typing at the bottom of the screen, not at the top, right? I don't think this is worth it, we need to make speed uniform improvements, if any (IMHO). Regards, Alfredo
Re: nested updates
Andre Poenitz wrote: > > > To our local Qt professionals: > > Why do we need > > [EMAIL PROTECTED]:/usr/src/lyx/lyx-devel/src > grep -R processEve * > frontends/qt2/QDialogView.h:qApp->processEvents(); > frontends/qt2/QDialogView.h:qApp->processEvents(); > frontends/qt2/QDialogView.h:qApp->processEvents(); > frontends/qt2/QDialogView.h:qApp->processEvents(); > frontends/qt2/QMathDialog.C:qApp->processEvents(); > frontends/qt2/QMathDialog.C:qApp->processEvents(); > frontends/qt2/lyx_gui.C:qApp->processEvents(); > frontends/qt2/QLPopupMenu.C:qApp->processEvents(); > > One of these seems to be the cause of nested update() calls... Well spotted... (and good question) I suppose that this reentrance is what was causing problems in the last scrollbar fix patch I've sent to the list a while ago. Alfredo
Re: nested updates
Alfredo Braunstein wrote:
> Andre Poenitz wrote:
>
>>
>>
>> To our local Qt professionals:
>>
>> Why do we need
>>
>> [EMAIL PROTECTED]:/usr/src/lyx/lyx-devel/src > grep -R processEve *
>> frontends/qt2/QDialogView.h:qApp->processEvents();
>> frontends/qt2/QDialogView.h:qApp->processEvents();
>> frontends/qt2/QDialogView.h:qApp->processEvents();
>> frontends/qt2/QDialogView.h:qApp->processEvents();
>> frontends/qt2/QMathDialog.C:qApp->processEvents();
>> frontends/qt2/QMathDialog.C:qApp->processEvents();
>> frontends/qt2/lyx_gui.C:qApp->processEvents();
>> frontends/qt2/QLPopupMenu.C:qApp->processEvents();
>>
>> One of these seems to be the cause of nested update() calls...
>
> Well spotted... (and good question)
>
> I suppose that this reentrance is what was causing problems in the last
> scrollbar fix patch I've sent to the list a while ago.
I'd be most suspicious about these two:
lyx_gui.C:
void sync_events()
{
qApp->processEvents();
}
QLPopupMenu.C:
void QLPopupMenu::fire(int index)
{
qApp->processEvents();
The QDialogView ones are clearly ugly but are invoked only when building a
dialog or when updating its contents. Not stuff that happens during a
redraw of the contents of the main lyx screen.
sync_events appears to be called on each and every blink of the cursor:
void LyXScreen::showCursor(BufferView & bv)
{
// You are not expected to understand this. This forces Qt
// (the problem case) to deal with its event queue. This is
// necessary when holding down a key such as 'page down' or
// just typing: without this processing of the event queue,
// the cursor gets ahead of itself without a selection or
// workarea redraw having a chance to keep up. If you think
// you can remove this, try selecting text with the mouse
// in Qt, or holding Page Down on the User's Guide.
lyx_gui::sync_events();
The processEvents in QLPopupMenu::fire has this ChangeLog entry:
2004-08-13 Jean-Marc Lasgouttes <[EMAIL PROTECTED]>
* QLPopupMenu.C (fire): make Qt process events before we
dispatch our function (toolbars still do not get repainted
correctly, though)
I'm not answering the 'why' these calls were felt to be needed, but I am
pointing out that they appear to 'solve' some specific problem.
Regards,
--
Angus
[PATCH 1.3] Postscript Interpreter: gswin32
Hello, Uwe pointed out that gswin32 is a Postscript interpreter used on the Windows platform. Though I don't have it on my machine, he's right. I have created a small patch for 1.3, that extends the check of the configure script. Angus, Jean-Marc, could you please commit it? (Please note that LyX 1.4 no longer checks for a Postscript interpreter. Therefore, there is no corresponding patch for 1.4.) Thanks in advance, Michael Index: lib/ChangeLog === RCS file: /cvs/lyx/lyx-devel/lib/ChangeLog,v retrieving revision 1.363.2.109 diff -u -p -r1.363.2.109 ChangeLog --- lib/ChangeLog 2005/05/08 14:11:55 1.363.2.109 +++ lib/ChangeLog 2005/05/08 14:58:40 @@ -1,3 +1,7 @@ +2005-05-08 Michael Schmitt <[EMAIL PROTECTED]> + + * lib/configure.m4: support gswin32 as Postscript interpreter + 2005-05-08 Angus Leeming <[EMAIL PROTECTED]> * scripts/lyxpreview2ppm.py: do not quote the name of the Index: lib/configure.m4 === RCS file: /cvs/lyx/lyx-devel/lib/configure.m4,v retrieving revision 1.60.2.18 diff -u -p -r1.60.2.18 configure.m4 --- lib/configure.m42005/04/22 16:39:04 1.60.2.18 +++ lib/configure.m42005/05/08 14:58:41 @@ -265,7 +265,7 @@ SEARCH_PROG([for Image converter],image_ test $image_command = "convert" && image_command="convert \$\$i \$\$o" # Search for a Postscript interpreter -LYXRC_PROG([for a Postscript interpreter], \ps_command, gs) +LYXRC_PROG([for a Postscript interpreter],\ps_command,gswin32 gs) # Search something to preview postscript SEARCH_PROG([for a Postscript previewer],GHOSTVIEW,gsview32 gv ghostview)
Re: [PATCH 1.3] Postscript Interpreter: gswin32
Michael Schmitt wrote: > Hello, > > Uwe pointed out that gswin32 is a Postscript interpreter used on the > Windows platform. Though I don't have it on my machine, he's right. > > I have created a small patch for 1.3, that extends the check of the > configure script. Angus, Jean-Marc, could you please commit it? Shouldn't we be using gswin32c, the command-line version that doesn't pop up a console window when it is invoked? $ ls /mnt/windowsC/Program\ Files/Ghostscript\ 8.33/gs8.33/bin gsdll32.dll gsdll32.lib gswin32c.exe gswin32.exe If I'd know about gswin32c.exe I woulnd't have spent two months writing lyxpreview2pnm.py so that it prevents that bloody konsole from popping up when it's not wanted :( Grr! -- Angus
Preamble
Hello, could anybody please enlighten me? In lib/ui/stdmenus.ui, the following entry is given for "document" Item "LaTeX Preamble...|P" "dialog-show preamble" But when I start LyX, there is no menu entry called "LaTeX Preamble" in the LyX Document menu! Even more surprising, when I enter "dialog-show preamble" in the command buffer, the "Document Settings" dialog opens. What is going on here? Michael (puzzled)
Re: Preamble
Michael Schmitt wrote: > Hello, > > could anybody please enlighten me? In lib/ui/stdmenus.ui, the following > entry is given for "document" > > Item "LaTeX Preamble...|P" "dialog-show preamble" > > But when I start LyX, there is no menu entry called "LaTeX Preamble" in > the LyX Document menu! Even more surprising, when I enter "dialog-show > preamble" in the command buffer, the "Document Settings" dialog opens. > What is going on here? > > Michael (puzzled) There is (an entry called "LaTeX Preamble") in the XForms frontend. In the Qt frontend the dialog has been merged into the Document dialog. Unfortunately, our attempts to do this to XForms foundered because the result 'felt' bad. One more reason to ditch XForms. -- Angus
Re: [PATCH 1.3] Postscript Interpreter: gswin32
Michael Schmitt wrote: >> Shouldn't we be using gswin32c, the command-line version that doesn't >> pop >> >>up a console window when it is invoked? >> >>$ ls /mnt/windowsC/Program\ Files/Ghostscript\ 8.33/gs8.33/bin >>gsdll32.dll gsdll32.lib gswin32c.exe gswin32.exe >> >>If I'd know about gswin32c.exe I woulnd't have spent two months writing >>lyxpreview2pnm.py so that it prevents that bloody konsole from popping up >>when it's not wanted :( >> >> > Is this a question or a statement? Both. You use Windows, I try not to. I learn vicariously through the experience of others. Can you find out from Uwe whether gswin32c exists on his box and whether it 'feels' nicer to use it. > Feel free to modify the patch. Sorry. It's Sunday and I'm feeling lazy. -- Angus
Re: [PATCH 1.3] Postscript Interpreter: gswin32
Angus Leeming wrote: Shouldn't we be using gswin32c, the command-line version that doesn't pop up a console window when it is invoked? $ ls /mnt/windowsC/Program\ Files/Ghostscript\ 8.33/gs8.33/bin gsdll32.dll gsdll32.lib gswin32c.exe gswin32.exe If I'd know about gswin32c.exe I woulnd't have spent two months writing lyxpreview2pnm.py so that it prevents that bloody konsole from popping up when it's not wanted :( Is this a question or a statement? Feel free to modify the patch. Michael
Re: nested updates
On Sun, May 08, 2005 at 03:50:56PM +0100, Angus Leeming wrote:
> sync_events appears to be called on each and every blink of the cursor:
> void LyXScreen::showCursor(BufferView & bv)
> {
> // You are not expected to understand this. This forces Qt
> // (the problem case) to deal with its event queue. This is
> // necessary when holding down a key such as 'page down' or
> // just typing: without this processing of the event queue,
> // the cursor gets ahead of itself without a selection or
> // workarea redraw having a chance to keep up. If you think
> // you can remove this, try selecting text with the mouse
> // in Qt, or holding Page Down on the User's Guide.
> lyx_gui::sync_events();
People have tried to take this out before, hence the comment. I can
almost guarantee it will break if you try to remove it. Sorry, I don't
know of a good solution either.
But I'm a bit confused as to why this is an issue: when I rewrote the
cursor handling, I'm pretty sure I made it so that showCursor is only
/ever/ called right at the end of an update (that is, after processing
an LFUN). So I don't see why this is nesting, unless somebody has
changed this behaviour.
The dialog ones are necessary because we want the dialogs to rebuild
their sizes etc. when we can't see the individual updates. If we had a
way to just update the dialog's events...
I have no idea why QLPopupMenu::fire() has one, but again, this should
not be a problem.
regards
john
[PATCH] reference-goto
Sorry, I forgot to fix a few more occurrences of "reference-goto". Please apply. Thanks, Michael Index: ChangeLog === RCS file: /cvs/lyx/lyx-devel/lib/ChangeLog,v retrieving revision 1.696 diff -u -p -r1.696 ChangeLog --- ChangeLog 2005/05/08 14:11:39 1.696 +++ ChangeLog 2005/05/08 16:58:17 @@ -1,3 +1,8 @@ +2005-05-08 Michael Schmitt <[EMAIL PROTECTED]> + + * bind/*.bind + * doc/*Customization: change "reference-goto" to "label-goto" + 2005-05-08 Angus Leeming <[EMAIL PROTECTED]> * scripts/lyxpreview2bitmap.py: Index: bind/aqua.bind === RCS file: /cvs/lyx/lyx-devel/lib/bind/aqua.bind,v retrieving revision 1.7 diff -u -p -r1.7 aqua.bind --- bind/aqua.bind 2005/01/15 18:36:02 1.7 +++ bind/aqua.bind 2005/05/08 16:58:17 @@ -131,7 +131,7 @@ \bind "M-~S-n e" "error-next" \bind "M-~S-n n" "note-next" -\bind "M-~S-n r" "reference-goto" +\bind "M-~S-n r" "label-goto" \bind "M-~S-n b s" "bookmark-save 1" \bind "M-~S-n b 1" "bookmark-goto 1" \bind "M-~S-n b 2" "bookmark-goto 2" Index: bind/cua.bind === RCS file: /cvs/lyx/lyx-devel/lib/bind/cua.bind,v retrieving revision 1.32 diff -u -p -r1.32 cua.bind --- bind/cua.bind 2005/01/15 18:36:02 1.32 +++ bind/cua.bind 2005/05/08 16:58:18 @@ -107,7 +107,7 @@ \bind "C-Home" "buffer-begin" \bind "C-End" "buffer-end" -\bind "C-~S-greater" "reference-goto" +\bind "C-~S-greater" "label-goto" \bind "C-~S-less" "bookmark-goto 0" Index: bind/emacs.bind === RCS file: /cvs/lyx/lyx-devel/lib/bind/emacs.bind,v retrieving revision 1.29 diff -u -p -r1.29 emacs.bind --- bind/emacs.bind 2005/01/15 18:36:02 1.29 +++ bind/emacs.bind 2005/05/08 16:58:18 @@ -148,7 +148,7 @@ \bind "M-~S-less" "buffer-begin" \bind "M-~S-greater" "buffer-end" -\bind "C-~S-greater" "reference-goto" +\bind "C-~S-greater" "label-goto" \bind "C-~S-less" "bookmark-goto 0" \bind "C-~S-1" "bookmark-goto 1" \bind "C-~S-2" "bookmark-goto 2" Index: bind/mac.bind === RCS file: /cvs/lyx/lyx-devel/lib/bind/mac.bind,v retrieving revision 1.7 diff -u -p -r1.7 mac.bind --- bind/mac.bind 2005/01/15 18:36:02 1.7 +++ bind/mac.bind 2005/05/08 16:58:18 @@ -96,7 +96,7 @@ \bind "C-Right" "line-end" \bind "C-Left" "line-begin" -\bind "C-~S-greater" "reference-goto" +\bind "C-~S-greater" "label-goto" \bind "C-~S-less" "bookmark-goto 0" Index: bind/xemacs.bind === RCS file: /cvs/lyx/lyx-devel/lib/bind/xemacs.bind,v retrieving revision 1.31 diff -u -p -r1.31 xemacs.bind --- bind/xemacs.bind 2005/01/15 18:36:02 1.31 +++ bind/xemacs.bind 2005/05/08 16:58:19 @@ -163,7 +163,7 @@ # A bit like autoindent in c-mode \bind "Tab" "depth-increment" -\bind "C-~S-greater" "reference-goto" +\bind "C-~S-greater" "label-goto" \bind "C-~S-less" "bookmark-goto 0" \bind "C-~S-1" "bookmark-goto 1" \bind "C-~S-2" "bookmark-goto 2" Index: doc/Customization.lyx === RCS file: /cvs/lyx/lyx-devel/lib/doc/Customization.lyx,v retrieving revision 1.7 diff -u -p -r1.7 Customization.lyx --- doc/Customization.lyx 2005/02/04 12:44:22 1.7 +++ doc/Customization.lyx 2005/05/08 16:58:30 @@ -12154,7 +12154,7 @@ C-S-greater \family default \family typewriter -reference-goto +label-goto \layout List \labelwidthstring 00.00. @@ -12697,7 +12697,7 @@ C-S-greater \family default \family typewriter -reference-goto +label-goto \layout List \labelwidthstring 00.00. Index: doc/de_Customization.lyx === RCS file: /cvs/lyx/lyx-devel/lib/doc/de_Customization.lyx,v retrieving revision 1.1 diff -u -p -r1.1 de_Customization.lyx --- doc/de_Customization.lyx 2004/06/01 19:50:10 1.1 +++ doc/de_Customization.lyx 2005/05/08 16:58:45 @@ -15525,7 +15525,7 @@ C-S-greater \family default \family typewriter -reference-goto +label-goto \layout List \labelwidthstring 00.00. @@ -16068,7 +16068,7 @@ C-S-greater \family default \family typewriter -reference-goto +label-goto \layout List \labelwidthstring 00.00. Index: doc/eu_Customization.lyx === RCS file: /cvs/lyx/lyx-devel/lib/doc/eu_Customization.lyx,v retrieving revision 1.1 diff -u -p -r1.1 eu_Customization.lyx --- doc/eu_Customization.lyx 2004/06/01 19:50:11 1.1 +++ doc/eu_Customization.lyx 2005/05/08 16:58:55 @@ -10174,7 +10174,7 @@ C-S-greater \family default \family typewriter -reference-goto +label-goto \layout Labeling \labelwidthstring 00.00. @@ -10719,7 +10719,7 @@ C-S-greater \family de
Re: nested updates
On Sun, May 08, 2005 at 01:35:10PM +0200, Juergen Spitzmueller wrote: > Andre Poenitz wrote: > > frontends/qt2/QDialogView.h: qApp->processEvents(); > > frontends/qt2/QDialogView.h: qApp->processEvents(); > > frontends/qt2/QDialogView.h: qApp->processEvents(); > > frontends/qt2/QDialogView.h: qApp->processEvents(); > > No idea why/if they are needed, but I cannot reproduce the Asserts when I > comment them out. Surprise. So this is indeed the cause for the nested updates. What does the person named `cvs blame | grep processEvents | cut -c 15-23` say in his defense? Andre'
Re: [Patch] Table crash blues
On Sun, May 08, 2005 at 10:53:08AM +0200, Michael Schmitt wrote: > Andre Poenitz wrote: > > >>I see. The coord cache must be updated, but I hope the actual drawing > >>can be skipped ??? when lyx knows that it is behind. That improves > >>performance in many cases, such as an overloaded machine or X over a > >>slow network. > >> > >> > >The coord cache is filled in the drawing phase (the metrics phase fills > >the size part of the coord cache only). We _do_ have a 'null painter' > >which only fills the cache, but if we did metrics + null painter > >painting + real painting in every case, drawing might get slow. Just a > >suspicion, though. > > > > > Hmmm, I would have expected that the only expensive operations is the > real painting (provided that the null painter does not invoke any X/Qt > methods). I've never timed this, so this may be true. There is some font handling necessary and stuff like this shows up in the profile sometimes. > Is it possible to combine metrics & null painter? No. The split in metrics + drawing phase is there because the drawing phase needs position in its parent and size of all childrenb, so this is not possible in a single phase. > Without knowing the details, I think the only clean solution is to avoid > recursive/reentrant updates completely. It would certainly be a clean and robust soution. xforms seems to work without recursive updates, so I wonder why the Qt frontend needs to call processEvents() at all. > I always prefer stability over performance. I don't eeven think this is a performance problem here, just some ugly hack we put there in becuase it seemed to work Andre'
Re: unneeded update() calls
On Sun, May 08, 2005 at 10:57:04AM +0200, Georg Baum wrote: > I did also call noUpdate() instead of undispatched() in insetcommand.C. Is > that correct? 'undispatched' means: I, the doDispatch() method of InsetFoo, hereby declare that I am not able to handle that request and trust my parent will do the Rigth Thing (even if my getStatus partner said that I can do it). It is sort of a kludge that should be used only rarely... noUpdate() means: I handled that request and I can reassure you that the screen does not need to be re-drawn and all entries in the coord cache stay valid (and there are no other thing to put in the coord cache). This is a fairly rare event as well and only some optimization. Not using noUpdate() should never be wrong. Andre'
Re: [Patch] Table crash blues
On Sun, May 08, 2005 at 01:05:00PM +0200, Michael Schmitt wrote: > Asger Ottar Alstrup wrote: > > >> (There should be some assertions in the code that check that update > >>isn't executed twice simultaneously; this should be easily doable by > >>the introduction of a static variable that is incremented when > >>entering update and decrement when leaving update) > > > >There already is such an assert. It is based on a bool, not a counter. > >There was a patch to change it to a counter, but I think that is wrong. > > You are right. A boolean is enough. Are you sure that the assert is > already in the CVS repository? It isn't, at least not in BufferView_pimpl::update(). There are some weaker asserts further down the pipeline. Andre'
Re: [Patch] Table crash blues
On Sun, May 08, 2005 at 12:05:50PM +0200, Michael Schmitt wrote: > Asger Ottar Alstrup wrote: > > >A better fix is to go to the root of the cause, and in this case, as > >mentioned in another post, I believe the right fix is to either omit > >or postpone the qApps->processEvent calls during update. > > > >Of course, excessive update calls from other places, as found by > >George, should also be nailed. (Notice, that the optimisation as > >sketched out would reduce the cost of superfluous update calls to a > >metrics and null-paint job only - the outcome would be identical to > >before, and no painting on screen would be necessary). > > So the "best" solution would be: > > 1. Run metrics computation & null painter as a single This is conceptually impossible. If using the nullpainter turns out to be cheap, we could have a three-phase update (metrics + nullpainter + painter) and safely skip the third phase when needed (i.e. the next update is already pending). > non-interruptable, non-reentrant step > (There should be some assertions in the code that check that update > isn't executed twice simultaneously; this should be easily doable by the > introduction of a static variable that is incremented when entering > update and decrement when leaving update) > 2. At the end of metrics computation & null painter, check whether > there is a new event > 3a. If yes, process the event, skip real painting > 3b. If no, go on with real painting The rest is ok. In any case this will be quite some reorganzition of all current inset drawing code and I think we should try to remove the processEvent calls first. > Since real painting is suppressed if the user edits text very fast, this > solution should be more efficient than the current one. I some situations yes. In some (when the user types slowly or the machine is fast enouhg), not. Andre'
Re: nested updates
On Sun, May 08, 2005 at 03:50:56PM +0100, Angus Leeming wrote:
> sync_events appears to be called on each and every blink of the cursor:
> void LyXScreen::showCursor(BufferView & bv)
> {
> // You are not expected to understand this. This forces Qt
> // (the problem case) to deal with its event queue. This is
> // necessary when holding down a key such as 'page down' or
> // just typing: without this processing of the event queue,
> // the cursor gets ahead of itself without a selection or
> // workarea redraw having a chance to keep up. If you think
> // you can remove this, try selecting text with the mouse
> // in Qt, or holding Page Down on the User's Guide.
> lyx_gui::sync_events();
> [...]
> I'm not answering the 'why' these calls were felt to be needed, but I am
> pointing out that they appear to 'solve' some specific problem.
Now a verification of these 'problems' would be in order, wouldn't it?
Andre'
Re: [Patch] Table crash blues
On Sun, May 08, 2005 at 04:21:44PM +0200, Alfredo Braunstein wrote: > > We could save some network traffic by restricting actual drawing to some > > horizontal strip as we did in 1.3, but this would be new code, the old > > logic does not fit anymore. > > And then, we 1.3.x only saved redraws when typing at the bottom of the > screen, not at the top, right? Something like that, yes. > I don't think this is worth it, we need to > make speed uniform improvements, if any (IMHO). Indeed. And I'll only start to consider speed hacks if something turns out to be unusable slow when using the simple approach... Andre'
Re: [Patch] Table crash blues
On Sun, May 08, 2005 at 12:16:27PM +0200, Asger Ottar Alstrup wrote: > Yes, something like this should work better. This can be done in steps: > > First, just leave the excessive painting as it is, but fix the > re-entrancy problem. Then, as a second step, introduce the null-painter > as an optimisation for the cases where two sequentive updates are coming > right after each other, such as fast typing, which allows us to skip the > first paint job. > > The third step could be to build something like the update > fingerprinting data-structure, as described earlier, which will help > painting only what needs to be painted, and also help mitigate any > excessive update calls there might be. > > In parallel to this, all excessive update calls should be hunted down, > and killed without mercy. Ok, so everybody knows the plan now... Andre'
Re: nested updates
On Sun, May 08, 2005 at 07:56:47PM +0200, Andre Poenitz wrote:
> On Sun, May 08, 2005 at 03:50:56PM +0100, Angus Leeming wrote:
> > sync_events appears to be called on each and every blink of the cursor:
> > void LyXScreen::showCursor(BufferView & bv)
> > {
> > // You are not expected to understand this. This forces Qt
> > // (the problem case) to deal with its event queue. This is
> > // necessary when holding down a key such as 'page down' or
> > // just typing: without this processing of the event queue,
> > // the cursor gets ahead of itself without a selection or
> > // workarea redraw having a chance to keep up. If you think
> > // you can remove this, try selecting text with the mouse
> > // in Qt, or holding Page Down on the User's Guide.
> > lyx_gui::sync_events();
> > [...]
> > I'm not answering the 'why' these calls were felt to be needed, but I am
> > pointing out that they appear to 'solve' some specific problem.
>
> Now a verification of these 'problems' would be in order, wouldn't it?
>
> Andre'
Let us first make sure which of these processEvents() are actually
causing our 'table blues' problem(s). Surely not all of them.
- Martin
pgpmTIQ409l52.pgp
Description: PGP signature
Re: nested updates
On Sun, May 08, 2005 at 05:50:08PM +0100, John Levon wrote:
> On Sun, May 08, 2005 at 03:50:56PM +0100, Angus Leeming wrote:
>
> > sync_events appears to be called on each and every blink of the cursor:
> > void LyXScreen::showCursor(BufferView & bv)
> > {
> > // You are not expected to understand this. This forces Qt
> > // (the problem case) to deal with its event queue. This is
> > // necessary when holding down a key such as 'page down' or
> > // just typing: without this processing of the event queue,
> > // the cursor gets ahead of itself without a selection or
> > // workarea redraw having a chance to keep up. If you think
> > // you can remove this, try selecting text with the mouse
> > // in Qt, or holding Page Down on the User's Guide.
> > lyx_gui::sync_events();
>
> People have tried to take this out before, hence the comment. I can
> almost guarantee it will break if you try to remove it. Sorry, I don't
> know of a good solution either.
>
> But I'm a bit confused as to why this is an issue: when I rewrote the
> cursor handling, I'm pretty sure I made it so that showCursor is only
> /ever/ called right at the end of an update (that is, after processing
> an LFUN). So I don't see why this is nesting, unless somebody has
> changed this behaviour.
You mean the hideCursor/showCursor mechanism in redraw()?
But... isn't this cursor drawing and hiding an asynchronous process
taking place every 400 msec? What happens if a drawing activity takes
longer than 400 (800?) msec and the cursor comes back all of itself in
the middle of it?
Only Qt has a working sync_events right now...
- Martin
pgpFWn18NaV7c.pgp
Description: PGP signature
Re: nested updates
On Sun, May 08, 2005 at 10:37:33PM +0300, Martin Vermeer wrote: > But... isn't this cursor drawing and hiding an asynchronous process > taking place every 400 msec? What happens if a drawing activity takes > longer than 400 (800?) msec and the cursor comes back all of itself in > the middle of it? Right, looks like a problem. We should just set a variable and then only update the cursor at the end. john
Re: nested updates
On Sun, May 08, 2005 at 05:50:08PM +0100, John Levon wrote:
> But I'm a bit confused as to why this is an issue: when I rewrote the
> cursor handling, I'm pretty sure I made it so that showCursor is only
> /ever/ called right at the end of an update (that is, after processing
> an LFUN). So I don't see why this is nesting, unless somebody has
> changed this behaviour.
Somebody seemingly has:
void BufferView::Pimpl::workAreaKeyPress(LyXKeySymPtr key,
key_modifier::state state)
{
owner_->getLyXFunc().processKeySym(key, state);
/* This is perhaps a bit of a hack. When we move
* around, or type, it's nice to be able to see
* the cursor immediately after the keypress. So
* we reset the toggle timeout and force the visibility
* of the cursor. Note we cannot do this inside
* dispatch() itself, because that's called recursively.
*/
if (available()) {
cursor_timeout.restart();
screen().showCursor(*bv_); // !!!
}
}
Note the line marked with !!!. This workAreaKeyPress shows btw up in
the backtrace of a nested update call.
> The dialog ones are necessary because we want the dialogs to rebuild
> their sizes etc. when we can't see the individual updates. If we had a
> way to just update the dialog's events...
I don't care how the dialog manage to rebuild there sizes if the core
crashes. And there should be some solution as xforms can do it without.
What's wrong with a signal emitted at the end of update() to which all
interested dialogs can connect?
Andre'
Re: nested updates
On Sun, May 08, 2005 at 05:50:08PM +0100, John Levon wrote: > The dialog ones are necessary because we want the dialogs to rebuild > their sizes etc. when we can't see the individual updates. If we had a > way to just update the dialog's events... I do not really understand what you mean here. Maybe you could add a sentence or two? Andre' PS: I have the impression that processEvents is rarely (if ever) needed outside Qt library code. It introduces concurrency problems even in single threaded applications that are not designed for this kind of scenario.
Re: nested updates
On Sun, May 08, 2005 at 08:41:26PM +0100, John Levon wrote: > On Sun, May 08, 2005 at 10:37:33PM +0300, Martin Vermeer wrote: > > > But... isn't this cursor drawing and hiding an asynchronous process > > taking place every 400 msec? What happens if a drawing activity takes > > longer than 400 (800?) msec and the cursor comes back all of itself in > > the middle of it? > > Right, looks like a problem. We should just set a variable and then only > update the cursor at the end. What about removing cursor and cursor blinking timer at the begin of update and start a new timer with a shown cursorat the end of update? Wouldn't this also address what that BufferView::Pimpl::workAreaKeyPress 'hack' is supposed to solve? Andre'
nested updates
> Note the line marked with !!!. This workAreaKeyPress shows btw up in > the backtrace of a nested update call. Why are we calling workAreaKeyPress() during an update? > I don't care how the dialog manage to rebuild there sizes if the core > crashes. The core didn't used to crash in 1.3. You can't blame other code for changes you've decided are necessary... > And there should be some solution as xforms can do it without. xforms is completely different from Qt. > What's wrong with a signal emitted at the end of update() to which all > interested dialogs can connect? Might work OK. Or just delay the frontend dialog's update() until the core has finished updating. regards, john
Re: nested updates
On Sun, May 08, 2005 at 10:28:59PM +0200, Andre Poenitz wrote: > What about removing cursor and cursor blinking timer at the begin of > update and start a new timer with a shown cursorat the end of update? Sounds very sensible to me. regards john
Re: nested updates
On Sun, May 08, 2005 at 09:33:55PM +0100, John Levon wrote: > On Sun, May 08, 2005 at 10:28:59PM +0200, Andre Poenitz wrote: > > > What about removing cursor and cursor blinking timer at the begin of > > update and start a new timer with a shown cursorat the end of update? > > Sounds very sensible to me. Or... replace the current hideCursor/showCursor pair by a boolean LyXScreen::allow_sync_ that is set to false before, to true after redraw and tested inside showCursor making the sync_events() call conditional. - Martin pgp01pFbOcSLv.pgp Description: PGP signature
Re: nested updates
On Sun, May 08, 2005 at 08:20:55PM +0200, Andre Poenitz wrote:
> On Sun, May 08, 2005 at 05:50:08PM +0100, John Levon wrote:
...
> void BufferView::Pimpl::workAreaKeyPress(LyXKeySymPtr key,
>key_modifier::state state)
> {
> owner_->getLyXFunc().processKeySym(key, state);
>
> /* This is perhaps a bit of a hack. When we move
>* around, or type, it's nice to be able to see
>* the cursor immediately after the keypress. So
>* we reset the toggle timeout and force the visibility
>* of the cursor. Note we cannot do this inside
>* dispatch() itself, because that's called recursively.
>*/
> if (available()) {
> cursor_timeout.restart();
> screen().showCursor(*bv_); // !!!
> }
> }
>
> Note the line marked with !!!. This workAreaKeyPress shows btw up in
> the backtrace of a nested update call.
Hmmm, wouldn't it be enough to just send the cursor_timeout.timeout
signal here?
- Martin
pgpXfbKmf92bj.pgp
Description: PGP signature
Re: [PATCH 1.3] Postscript Interpreter: gswin32
Angus Leeming wrote: Can you find out from Uwe whether gswin32c exists on his box and whether it 'feels' nicer to use it. gswin32c.exe is part of all ghostscript win installations. It changes to a gs prompt in the win console. In contrast gswin32.exe opens a new window with a gs console. Both are ok, "feeling nicer" is relative so please decide to use what you prefer. regards Uwe
Re: [PATCH 1.3] Postscript Interpreter: gswin32
Angus Leeming wrote: Both. You use Windows, I try not to. I learn vicariously through the experience of others. Can you find out from Uwe whether gswin32c exists on his box and whether it 'feels' nicer to use it. Since Uwe has no objections, let's use gswin32c. Angus, Jean-Marc, please apply the new patch. Michael Index: ChangeLog === RCS file: /cvs/lyx/lyx-devel/lib/ChangeLog,v retrieving revision 1.363.2.109 diff -u -p -r1.363.2.109 ChangeLog --- ChangeLog 2005/05/08 14:11:55 1.363.2.109 +++ ChangeLog 2005/05/09 05:39:31 @@ -1,3 +1,7 @@ +2005-05-08 Michael Schmitt <[EMAIL PROTECTED]> + + * lib/configure.m4: support gswin32c as Postscript interpreter + 2005-05-08 Angus Leeming <[EMAIL PROTECTED]> * scripts/lyxpreview2ppm.py: do not quote the name of the Index: configure.m4 === RCS file: /cvs/lyx/lyx-devel/lib/configure.m4,v retrieving revision 1.60.2.18 diff -u -p -r1.60.2.18 configure.m4 --- configure.m42005/04/22 16:39:04 1.60.2.18 +++ configure.m42005/05/09 05:39:39 @@ -265,7 +265,7 @@ SEARCH_PROG([for Image converter],image_ test $image_command = "convert" && image_command="convert \$\$i \$\$o" # Search for a Postscript interpreter -LYXRC_PROG([for a Postscript interpreter], \ps_command, gs) +LYXRC_PROG([for a Postscript interpreter],\ps_command,gswin32c gs) # Search something to preview postscript SEARCH_PROG([for a Postscript previewer],GHOSTVIEW,gsview32 gv ghostview)
[PATCH] shortcut
Hello, this patch fixes the shortcuts for "Cross-reference". Please apply to LyX 1.4. Thanks, Michael Index: ChangeLog === RCS file: /cvs/lyx/lyx-devel/lib/ChangeLog,v retrieving revision 1.696 diff -u -p -r1.696 ChangeLog --- ChangeLog 2005/05/08 14:11:39 1.696 +++ ChangeLog 2005/05/09 05:49:45 @@ -1,3 +1,8 @@ +2005-05-09 Michael Schmitt <[EMAIL PROTECTED]> + + * ui/classic.ui: + * ui/stdmenus.ui: fix shortcut for Cross-reference + 2005-05-08 Angus Leeming <[EMAIL PROTECTED]> * scripts/lyxpreview2bitmap.py: Index: ui/classic.ui === RCS file: /cvs/lyx/lyx-devel/lib/ui/classic.ui,v retrieving revision 1.17 diff -u -p -r1.17 classic.ui --- ui/classic.ui 2005/05/08 10:02:35 1.17 +++ ui/classic.ui 2005/05/09 05:50:45 @@ -211,7 +211,7 @@ Menuset Separator Submenu "Special Character|S" "insert_special" Item "Citation...|C" "dialog-show-new-inset citation" - Item "Cross-reference...|R" "dialog-show-new-inset ref" + Item "Cross-reference...|r" "dialog-show-new-inset ref" Item "Label...|L" "label-insert" Item "Footnote|F" "footnote-insert" Item "Marginal Note|M" "marginalnote-insert" Index: ui/stdmenus.ui === RCS file: /cvs/lyx/lyx-devel/lib/ui/stdmenus.ui,v retrieving revision 1.46 diff -u -p -r1.46 stdmenus.ui --- ui/stdmenus.ui 2005/05/08 10:02:35 1.46 +++ ui/stdmenus.ui 2005/05/09 05:50:45 @@ -237,7 +237,7 @@ Menuset Item "Box" "box-insert Frameless" Separator Item "Citation...|C" "dialog-show-new-inset citation" - Item "Cross-reference...|R" "dialog-show-new-inset ref" + Item "Cross-reference...|r" "dialog-show-new-inset ref" Item "Label...|L" "label-insert" Item "Index Entry|d" "index-insert" # I'm going to kill this dumb dialog, but for now ...
Re: nested updates
On Sun, May 08, 2005 at 08:41:26PM +0100, John Levon wrote:
> On Sun, May 08, 2005 at 10:37:33PM +0300, Martin Vermeer wrote:
>
> > But... isn't this cursor drawing and hiding an asynchronous process
> > taking place every 400 msec? What happens if a drawing activity takes
> > longer than 400 (800?) msec and the cursor comes back all of itself in
> > the middle of it?
>
> Right, looks like a problem. We should just set a variable and then only
> update the cursor at the end.
>
> john
I tried the attached. The net effect is, that the table crashes we have
seen disappear, but instead we get a new crash. It is the assert in
startUpdating(), signaling an overrun of the screen redraw by the
keyboard input. The relevant part of the stack is (sorry for no symbols):
#4 0x080a39c2 in boost::assertion_failed (expr=0x83a7458 "!updating",
function=0x83a73a0 "void CoordCache::startUpdating()",
file=0x83a744b "coordcache.C", line=43) at boost.C:56
#5 0x080d41ee in CoordCache::startUpdating () at stl_deque.h:126
#6 0x08069f63 in BufferView::Pimpl::update () at basic_string.h:910
#7 0x080634ba in BufferView::update () at ButtonPolicies.h:70
#8 0x08116c8d in LyXFunc::dispatch () at basic_string.h:358
#9 0x0810f77d in LyXFunc::processKeySym () at basic_string.h:358
#10 0x080698d5 in BufferView::Pimpl::workAreaKeyPress () at basic_string.h:910
Apparently we ought to be able to queue incoming keystrokes / dispatches for
the duration of redraw (using, e.g., my proposed isUpdating() method in
coordcache). I remember Asger suggesting this much yesterday.
Suggestions how and where to do this? LyXFunc?
- Martin
PS NOTE the problem is _not_ the showCursor call later on in
workAreaKeyPress. It's the processKeySym call followed by the dispatch chain.
Index: screen.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/frontends/screen.C,v
retrieving revision 1.101
diff -u -p -r1.101 screen.C
--- screen.C11 Feb 2005 18:07:06 - 1.101
+++ screen.C9 May 2005 06:21:34 -
@@ -122,7 +122,7 @@ SplashScreen::SplashScreen()
LyXScreen::LyXScreen()
- : greyed_out_(true), cursor_visible_(false)
+ : greyed_out_(true), cursor_visible_(false), sync_allowed_(true)
{
// Start loading the pixmap as soon as possible
if (lyxrc.show_banner) {
@@ -155,7 +155,8 @@ void LyXScreen::showCursor(BufferView &
// workarea redraw having a chance to keep up. If you think
// you can remove this, try selecting text with the mouse
// in Qt, or holding Page Down on the User's Guide.
- lyx_gui::sync_events();
+ if (sync_allowed_)
+ lyx_gui::sync_events();
if (cursor_visible_)
return;
@@ -223,13 +224,13 @@ void LyXScreen::redraw(BufferView & bv,
{
greyed_out_ = false;
workarea().getPainter().start();
- hideCursor();
+ sync_allowed_ = false;
paintText(bv, vi);
lyxerr[Debug::DEBUG] << "Redraw screen" << endl;
expose(0, 0, workarea().workWidth(), workarea().workHeight());
workarea().getPainter().end();
theCoords.doneUpdating();
- showCursor(bv);
+ sync_allowed_ = true;
}
Index: screen.h
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/frontends/screen.h,v
retrieving revision 1.30
diff -u -p -r1.30 screen.h
--- screen.h30 Nov 2004 01:59:46 - 1.30
+++ screen.h9 May 2005 06:21:34 -
@@ -86,6 +86,9 @@ private:
/// is the cursor currently displayed
bool cursor_visible_;
+
+ ///
+ bool sync_allowed_;
};
#endif // SCREEN_H
pgpslJ6Q4voWc.pgp
Description: PGP signature
