On Thu, May 19, 2005 at 09:46:27AM +0200, Lars Gullik Bjønnes wrote: > Andre Poenitz <[EMAIL PROTECTED]> writes: > > | On Thu, May 12, 2005 at 06:47:50PM +0200, Lars Gullik Bjønnes wrote: > >> this is kindo a proof-of-concept. > > > | Is there a specific reason this queue is in the frontend and not in the > | core? > > > | If it were in the core we could have an 'update' lfuns, and a 'dont need > | up-to-date cache' lfun flags and play games... > > > | Suppose we have a queue like a, b, <Down>, c, i.e. > > > | <Key-a>[implicitupdate] <Key-b>[implicitupdate] > | <Down>[implicitupdate] <Key-c>[implicitpdate] ... > > > | we'd flag <Key-*> as 'dont need cache' and <Down> as 'need cache' and > | transform the queue to > > > | <Key-a> <Key-b> <Update> <Down> <Key-c> ... > > For 1.5. I have plans to move some into the core. (the possibility to > handle more than one char-insert f.x. with update only on last one) > but the handling of the queue and the event pruning (which is all that > is happening now) should stay in the frontend. > > -- > Lgb
Actually I have now had time to think about this for two weeks, and my
considered opinion is a bit different...
I believe the core problem we have been fighting with is abusing
processEvents for something it never was intended to do. It is designed
to be used, during, e.g., a long computational run, to allow windows to
repaint themselves after exposure or such. Properly done, this requires
"slicing" the computation into suitable mouthfuls. But it is asynchronous
in nature, and especially calling it from a cursor blink routine (what is
effectively happening here) is asking for trouble.
In the attached patch I have reversed this situation (partly a previous
patch of mine) and implemented a different approach.
The problem for which the sync_events/processEvents solution was
introduced in the first place, were the rendering artefacts that
occurred, e.g., when making the Page Down key repeat in the User
Guide. But in my opinion this is not a fix, but a workaround or
cover-up. A proper fix means understanding the cause, which IMHO
is the call to showCursor found in workAreaKeypress.
For whatever reason, when you keep a key down, the cursor rendering is
called and executed straightaway for every key event handed to LyX, even
if the whole-screen update is not. These whole-screen updates are
coalesced by X over a number of keystrokes. The result: several cursor
images superimposed on an un-updated LyX screen.
My fix for this looks as follows:
@@ -521,8 +521,10 @@ void BufferView::Pimpl::workAreaKeyPress
* dispatch() itself, because that's called recursively.
*/
if (available()) {
+ screen().prepareCursor();
+ cursor_timeout.setTimeout(100);
cursor_timeout.restart();
- screen().showCursor(*bv_);
+ cursor_timeout.setTimeout(400);
}
}
As you see, showCursor is not called directly anymore. Instead, the
cursor timer is temporarily shortened to 0.1 s and re-started. The
original intent of this code is to make the cursor appear immediately
after a keystroke, and humanly speaking 0.1 s is immediate. The
difference is however, that if key events are coming in at key repeat
rate (some 20/s), _this timer will never expire_.
This is precisely what we see. I am not sure this is the whole story,
but I don't manage to see these artefacts anymore at key repeat, or at
any rate of typing. And we never lose a single keystroke.
What is also beautiful now, is that, without any code of our own,
drawing events are naturally coalesced, so that the screen is only
redrawn after every so many key events. When holding "Page Down" down
this means that the scrollbar shows where we are going (at a fixed
rate!), but only a few intermediate screenfuls are shown during the
process. I find it very natural and easy to get used to. Try it!
This patch includes the single-paragraph-only drawing speed-up patch
corrected as per Lars's remarks.
Please review.
- Martin
PS there is still a call to processEvents in two places in the Qt
front-end. I don't think these are a problem, but probably the use
of processEvents here is avoidable in some way.
Index: ChangeLog
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/ChangeLog,v
retrieving revision 1.2188
diff -u -p -r1.2188 ChangeLog
--- ChangeLog 24 May 2005 10:23:29 -0000 1.2188
+++ ChangeLog 27 May 2005 18:29:21 -0000
@@ -1,3 +1,17 @@
+2005-05-27 Martin Vermeer <[EMAIL PROTECTED]>
+
+ * BufferView.[Ch] (update):
+ * BufferView_pimpl.[Ch] (update, metrics):
+ * dimension.h (operator==):
+ * lyxfunc.C (dispatch):
+ * metricsinfo.h (ViewMetricsInfo):
+ * rowpainter.C (paintText):
+ * lyxtext.h:
+ * text.C (redoParagraph):
+ * text3.C (dispatch): Make LyX only repaint current paragraph in
+ case of character insert --> speedup. Also fix cursor draw
+ artifacts
+
2005-05-23 Jean-Marc Lasgouttes <[EMAIL PROTECTED]>
* vspace.C (asGUIName): new method. A version of the space
Index: BufferView.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView.C,v
retrieving revision 1.261
diff -u -p -r1.261 BufferView.C
--- BufferView.C 19 May 2005 18:53:05 -0000 1.261
+++ BufferView.C 27 May 2005 18:29:21 -0000
@@ -142,9 +142,9 @@ bool BufferView::fitCursor()
}
-void BufferView::update(bool fitcursor, bool forceupdate)
+void BufferView::update(Update::flags flags)
{
- pimpl_->update(fitcursor, forceupdate);
+ pimpl_->update(flags);
}
Index: BufferView.h
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView.h,v
retrieving revision 1.186
diff -u -p -r1.186 BufferView.h
--- BufferView.h 26 Apr 2005 11:12:09 -0000 1.186
+++ BufferView.h 27 May 2005 18:29:22 -0000
@@ -35,6 +35,24 @@ class LyXView;
class Painter;
class ParIterator;
+
+namespace Update {
+ enum flags {
+ FitCursor = 1,
+ Force = 2,
+ SinglePar = 4
+ };
+
+inline flags operator|(flags const f, flags const g)
+{
+ int const intf(static_cast<int>(f));
+ int const intg(static_cast<int>(g));
+ return static_cast<flags>(intf | intg);
+}
+
+} // namespace
+
+
/**
* A buffer view encapsulates a view onto a particular
* buffer, and allows access to operate upon it. A view
@@ -81,7 +99,8 @@ public:
* position changes. \c forceupdate means to force an update
* in any case.
*/
- void update(bool fitcursor = true, bool forceupdate = true);
+
+ void update(Update::flags flags = Update::FitCursor | Update::Force);
/// move the screen to fit the cursor. Only to be called with
/// good y coordinates (after a bv::metrics)
bool fitCursor();
Index: BufferView_pimpl.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView_pimpl.C,v
retrieving revision 1.583
diff -u -p -r1.583 BufferView_pimpl.C
--- BufferView_pimpl.C 11 May 2005 07:44:18 -0000 1.583
+++ BufferView_pimpl.C 27 May 2005 18:29:22 -0000
@@ -521,8 +521,10 @@ void BufferView::Pimpl::workAreaKeyPress
* dispatch() itself, because that's called recursively.
*/
if (available()) {
+ screen().prepareCursor();
+ cursor_timeout.setTimeout(100);
cursor_timeout.restart();
- screen().showCursor(*bv_);
+ cursor_timeout.setTimeout(400);
}
}
@@ -606,11 +608,12 @@ bool BufferView::Pimpl::fitCursor()
}
-void BufferView::Pimpl::update(bool fitcursor, bool forceupdate)
+void BufferView::Pimpl::update(Update::flags flags)
{
lyxerr << BOOST_CURRENT_FUNCTION
- << "[fitcursor = " << fitcursor << ','
- << " forceupdate = " << forceupdate
+ << "[fitcursor = " << (flags & Update::FitCursor)
+ << ", forceupdate = " << (flags & Update::Force)
+ << ", singlepar = " << (flags & Update::SinglePar)
<< "] buffer: " << buffer_ << endl;
// Check needed to survive LyX startup
@@ -621,10 +624,6 @@ void BufferView::Pimpl::update(bool fitc
CoordCache backup;
std::swap(theCoords, backup);
- // This call disallows cursor blink to call
- // processEvents. It is necessary to prevent screen
- // redraw being called recursively.
- screen().unAllowSync();
// This, together with doneUpdating(), verifies (using
// asserts) that screen redraw is not called from
// within itself.
@@ -632,10 +631,11 @@ void BufferView::Pimpl::update(bool fitc
// First drawing step
ViewMetricsInfo vi = metrics();
+ bool forceupdate(flags & Update::Force);
- if (fitcursor && fitCursor()) {
+ if ((flags & Update::FitCursor) && fitCursor()) {
forceupdate = true;
- vi = metrics();
+ vi = metrics(flags & Update::SinglePar);
}
if (forceupdate) {
// Second drawing step
@@ -934,7 +934,10 @@ bool BufferView::Pimpl::workAreaDispatch
if (cur.result().dispatched()) {
// Redraw if requested or necessary.
- update(cur.result().update(), cur.result().update());
+ if (cur.result().update())
+ update(Update::FitCursor | Update::Force);
+ else
+ update();
}
// See workAreaKeyPress
@@ -1246,7 +1249,7 @@ bool BufferView::Pimpl::dispatch(FuncReq
}
-ViewMetricsInfo BufferView::Pimpl::metrics()
+ViewMetricsInfo BufferView::Pimpl::metrics(bool singlepar)
{
// Remove old position cache
theCoords.clear();
@@ -1274,7 +1277,7 @@ ViewMetricsInfo BufferView::Pimpl::metri
// Redo paragraphs above cursor if necessary
int y1 = y0;
- while (y1 > 0 && pit1 > 0) {
+ while (!singlepar && y1 > 0 && pit1 > 0) {
y1 -= text->getPar(pit1).ascent();
--pit1;
text->redoParagraph(pit1);
@@ -1298,7 +1301,7 @@ ViewMetricsInfo BufferView::Pimpl::metri
// Redo paragraphs below cursor if necessary
int y2 = y0;
- while (y2 < bv.workHeight() && pit2 < int(npit) - 1) {
+ while (!singlepar && y2 < bv.workHeight() && pit2 < int(npit) - 1) {
y2 += text->getPar(pit2).descent();
++pit2;
text->redoParagraph(pit2);
@@ -1321,5 +1324,5 @@ ViewMetricsInfo BufferView::Pimpl::metri
<< " y2: " << y2
<< endl;
- return ViewMetricsInfo(pit1, pit2, y1, y2);
+ return ViewMetricsInfo(pit1, pit2, y1, y2, singlepar);
}
Index: BufferView_pimpl.h
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView_pimpl.h,v
retrieving revision 1.127
diff -u -p -r1.127 BufferView_pimpl.h
--- BufferView_pimpl.h 19 Jan 2005 15:03:26 -0000 1.127
+++ BufferView_pimpl.h 27 May 2005 18:29:22 -0000
@@ -60,7 +60,7 @@ public:
//
bool fitCursor();
///
- void update(bool fitcursor = false, bool forceupdate = true);
+ void update(Update::flags flags = Update::Force);
///
void newFile(std::string const &, std::string const &, bool);
///
@@ -186,7 +186,7 @@ private:
///
int offset_ref_;
///
- ViewMetricsInfo metrics();
+ ViewMetricsInfo metrics(bool singlepar = false);
};
Index: dimension.h
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/dimension.h,v
retrieving revision 1.6
diff -u -p -r1.6 dimension.h
--- dimension.h 18 Jan 2005 14:15:55 -0000 1.6
+++ dimension.h 27 May 2005 18:29:22 -0000
@@ -61,4 +61,10 @@ public:
int des;
};
+inline
+bool operator==(Dimension const & a, Dimension const & b)
+{
+ return a.wid == b.wid && a.asc == b.asc && a.des ==b.des ;
+}
+
#endif
Index: lyxfunc.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyxfunc.C,v
retrieving revision 1.657
diff -u -p -r1.657 lyxfunc.C
--- lyxfunc.C 6 May 2005 20:00:30 -0000 1.657
+++ lyxfunc.C 27 May 2005 18:29:22 -0000
@@ -1522,7 +1522,10 @@ void LyXFunc::dispatch(FuncRequest const
// Redraw screen unless explicitly told otherwise.
// This also initializes the position cache for all
insets
// in (at least partially) visible top-level paragraphs.
- view()->update(true, update);
+ if (update)
+ view()->update(Update::FitCursor |
Update::Force);
+ else
+ view()->update(Update::FitCursor);
// if we executed a mutating lfun, mark the buffer as
dirty
// FIXME: Why not use flag.enabled() but call getStatus
again?
Index: lyxtext.h
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyxtext.h,v
retrieving revision 1.321
diff -u -p -r1.321 lyxtext.h
--- lyxtext.h 12 Apr 2005 18:42:24 -0000 1.321
+++ lyxtext.h 27 May 2005 18:29:22 -0000
@@ -95,7 +95,7 @@ public:
void setFont(LCursor & cur, LyXFont const &, bool toggleall = false);
/// rebreaks the given par
- void redoParagraph(pit_type pit);
+ bool redoParagraph(pit_type pit);
/// returns pos in given par at given x coord
pos_type x2pos(pit_type pit, int row, int x) const;
Index: metricsinfo.h
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/metricsinfo.h,v
retrieving revision 1.15
diff -u -p -r1.15 metricsinfo.h
--- metricsinfo.h 19 Jan 2005 15:03:29 -0000 1.15
+++ metricsinfo.h 27 May 2005 18:29:22 -0000
@@ -97,12 +97,14 @@ class TextMetricsInfo {};
class ViewMetricsInfo
{
public:
- ViewMetricsInfo(lyx::pit_type p1, lyx::pit_type p2,
- int y1, int y2) : p1(p1), p2(p2), y1(y1), y2(y2) {}
+ ViewMetricsInfo(lyx::pit_type p1, lyx::pit_type p2, int y1, int y2,
+ bool singlepar) : p1(p1), p2(p2), y1(y1), y2(y2),
+ singlepar(singlepar) {}
lyx::pit_type p1;
lyx::pit_type p2;
int y1;
int y2;
+ bool singlepar;
};
Index: rowpainter.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
retrieving revision 1.149
diff -u -p -r1.149 rowpainter.C
--- rowpainter.C 5 May 2005 13:11:05 -0000 1.149
+++ rowpainter.C 27 May 2005 18:29:22 -0000
@@ -785,18 +785,19 @@ void paintText(BufferView const & bv, Vi
yy += text->getPar(pit).descent();
}
+ if (!vi.singlepar) {
+ // paint one paragraph above and one below
+ if (vi.p1 > 0) {
+ text->redoParagraph(vi.p1 - 1);
+ paintPar(pi, *bv.text(), vi.p1 - 1, 0,
+ vi.y1 - text->getPar(vi.p1 - 1).descent());
+ }
- // paint one paragraph above and one below
- if (vi.p1 > 0) {
- text->redoParagraph(vi.p1 - 1);
- paintPar(pi, *bv.text(), vi.p1 - 1, 0,
- vi.y1 - text->getPar(vi.p1 - 1).descent());
- }
-
- if (vi.p2 < lyx::pit_type(text->paragraphs().size()) - 1) {
- text->redoParagraph(vi.p2 + 1);
- paintPar(pi, *bv.text(), vi.p2 + 1, 0,
- vi.y2 + text->getPar(vi.p2 + 1).ascent());
+ if (vi.p2 < lyx::pit_type(text->paragraphs().size()) - 1) {
+ text->redoParagraph(vi.p2 + 1);
+ paintPar(pi, *bv.text(), vi.p2 + 1, 0,
+ vi.y2 + text->getPar(vi.p2 + 1).ascent());
+ }
}
// and grey out above (should not happen later)
Index: text3.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/text3.C,v
retrieving revision 1.293
diff -u -p -r1.293 text3.C
--- text3.C 5 May 2005 13:13:56 -0000 1.293
+++ text3.C 27 May 2005 18:29:22 -0000
@@ -262,7 +262,7 @@ void update(LCursor & cur)
{
//we don't call update(true, false) directly to save a metrics call
if (cur.bv().fitCursor())
- cur.bv().update(false, true);
+ cur.bv().update(Update::Force);
}
@@ -1129,9 +1129,13 @@ void LyXText::dispatch(LCursor & cur, Fu
cur.resetAnchor();
moveCursor(cur, false);
+ needsUpdate = redoParagraph(cur.pit());
+ if (!needsUpdate)
+ // update only this paragraph
+ cur.bv().update(Update::SinglePar | Update::Force);
+
// real_current_font.number can change so we need to
// update the minibuffer
- if (old_font != real_current_font)
bv->updateScrollbar();
break;
}
Index: text.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/text.C,v
retrieving revision 1.601
diff -u -p -r1.601 text.C
--- text.C 5 May 2005 18:33:47 -0000 1.601
+++ text.C 27 May 2005 18:29:22 -0000
@@ -1649,7 +1649,7 @@ Row const & LyXText::firstRow() const
}
-void LyXText::redoParagraph(pit_type const pit)
+bool LyXText::redoParagraph(pit_type const pit)
{
// remove rows of paragraph, keep track of height changes
Paragraph & par = pars_[pit];
@@ -1700,8 +1700,13 @@ void LyXText::redoParagraph(pit_type con
dim.asc += par.rows()[0].ascent();
dim.des -= par.rows()[0].ascent();
+
+ bool const same = dim == par.dim();
+
par.dim() = dim;
//lyxerr << "redoParagraph: " << par.rows().size() << " rows\n";
+
+ return !same;
}
Index: frontends/ChangeLog
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/frontends/ChangeLog,v
retrieving revision 1.281
diff -u -p -r1.281 ChangeLog
--- frontends/ChangeLog 20 May 2005 06:36:27 -0000 1.281
+++ frontends/ChangeLog 27 May 2005 18:29:23 -0000
@@ -1,3 +1,8 @@
+2005-05-27 Martin Vermeer <[EMAIL PROTECTED]>
+
+ * screen.[hC]: better fix, processEvents -related screen update
+ bug
+
2005-05-20 Lars Gullik Bjonnes <[EMAIL PROTECTED]>
* Move the gnome subdir to the Attic
Index: frontends/screen.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/frontends/screen.C,v
retrieving revision 1.102
diff -u -p -r1.102 screen.C
--- frontends/screen.C 11 May 2005 07:44:19 -0000 1.102
+++ frontends/screen.C 27 May 2005 18:29:23 -0000
@@ -122,7 +122,7 @@ SplashScreen::SplashScreen()
LyXScreen::LyXScreen()
- : greyed_out_(true), cursor_visible_(false), sync_allowed_(true)
+ : greyed_out_(true), cursor_visible_(false)
{
// Start loading the pixmap as soon as possible
if (lyxrc.show_banner) {
@@ -147,20 +147,6 @@ void LyXScreen::checkAndGreyOut()
void LyXScreen::showCursor(BufferView & bv)
{
- // This code is currently meaningful only for the Qt frontend.
- // This is the place (like below in hideCursor) where
- // processEvents is being called, and things like keystrokes and
- // mouse clicks are being handed to the LyX core, once every
- // cursor blink.
- // THERE IS NOT SUPPOSED TO BE ANY OTHER CALL TO processEvents
- // ANYWHERE ELSE.
- // in BufferView::Pimpl::update() and here, the sync_allowed_
- // guard is set/cleared which is used here to prevent recursive
- // calls to screen update. startUpdating() and doneUpdating() in
- // coordcache again contain asserts to detect such recursion.
- if (sync_allowed_)
- lyx_gui::sync_events();
-
if (cursor_visible_)
return;
@@ -206,9 +192,6 @@ void LyXScreen::showCursor(BufferView &
void LyXScreen::hideCursor()
{
- if (sync_allowed_)
- lyx_gui::sync_events();
-
if (!cursor_visible_)
return;
@@ -226,6 +209,12 @@ void LyXScreen::toggleCursor(BufferView
}
+void LyXScreen::prepareCursor()
+{
+ cursor_visible_ = false;
+}
+
+
void LyXScreen::redraw(BufferView & bv, ViewMetricsInfo const & vi)
{
greyed_out_ = false;
@@ -235,7 +224,6 @@ void LyXScreen::redraw(BufferView & bv,
expose(0, 0, workarea().workWidth(), workarea().workHeight());
workarea().getPainter().end();
theCoords.doneUpdating();
- sync_allowed_ = true;
}
Index: frontends/screen.h
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/frontends/screen.h,v
retrieving revision 1.31
diff -u -p -r1.31 screen.h
--- frontends/screen.h 11 May 2005 07:44:19 -0000 1.31
+++ frontends/screen.h 27 May 2005 18:29:23 -0000
@@ -54,8 +54,9 @@ public:
/// toggle the cursor's visibility
void toggleCursor(BufferView & bv);
- ///
- void unAllowSync() { sync_allowed_ = false; };
+ /// set cursor_visible_ to false in prep for re-display
+ void prepareCursor();
+
protected:
/// cause the display of the given area of the work area
@@ -90,8 +91,6 @@ private:
/// is the cursor currently displayed
bool cursor_visible_;
- ///
- bool sync_allowed_;
};
#endif // SCREEN_H
pgpaDZz9ywAPT.pgp
Description: PGP signature
