Abdelrazak Younes wrote:
rgheck wrote:
Thoroughly? Hard to know. Perhaps we should wait a bit and see if we
get any problems in trunk.
It should be fine in trunk but I am not sure it is 100% safe in branch.
OK. Well, we know the reporter is capable of patching it himself, so
he's welcome to do so.
I'll tell you, Abdel, after actually working with this code a bit, I can
see for myself the benefit of all the work you've done. It is MUCH
cleaner. Nice job.
There are a number of calls to start/stopBlinkingCursor(), some of
those are important because we don't want to do something else while
processing a mouse or keyboard event for example. I reckon those
things are cleaner in trunk so there should be no problem but, as I
said I am not sure about branch. The safest solution would be the stop
the general timer you introduced when entering stopBlinkingCursor()
and restarting it when leaving startBlinkingCursor(). Maybe we should
do that in trunk too but I'll let you decide.
Well, you know this code a lot better than I do, so let me say how
things seem to me, and you tell me what you think.
We could stop and restart the timer in start- and stopBlinkingCursor().
I worry, though, that restarting the timer too often might cause
problems. What if it got restarted every 0.25 seconds? Then it would
never fire. Qt 4.3.x has a QTimer::isActive() function that can be used
to check for this, viz:
if (!general_timer_.isActive())
general_timer_.start();
but this does not exist in 4.2.x, so it doesn't help us. That said,
maybe we shouldn't rely upon these st(op|art)BlinkingCursor() methods
here, but should instead start and stop the timer when we need to do so.
Some of the time, it doesn't seem we need to reset that timer. But in
other cases, we do. So I grepped for BlinkingCursor:
[EMAIL PROTECTED] qt4]$ srcgrep BlinkingCursor
GuiView.cpp:623:
d.current_work_area_->stopBlinkingCursor();
GuiView.cpp:626:
d.current_work_area_->startBlinkingCursor();
GuiView.cpp:1557: d.current_work_area_->startBlinkingCursor();
GuiWorkArea.cpp:279:void GuiWorkArea::stopBlinkingCursor()
GuiWorkArea.cpp:286:void GuiWorkArea::startBlinkingCursor()
GuiWorkArea.cpp:337: stopBlinkingCursor();
GuiWorkArea.cpp:371: stopBlinkingCursor();
GuiWorkArea.cpp:390: startBlinkingCursor();
GuiWorkArea.cpp:490: stopBlinkingCursor();
GuiWorkArea.cpp:498: startBlinkingCursor();
GuiWorkArea.cpp:571: startBlinkingCursor();
GuiWorkArea.cpp:577: stopBlinkingCursor();
GuiWorkArea.cpp:840: startBlinkingCursor();
GuiWorkArea.cpp:842: stopBlinkingCursor();
GuiWorkArea.h:121: void stopBlinkingCursor();
GuiWorkArea.h:123: void startBlinkingCursor();
and fixed the ones that looked potentially dangerous. The patch is attached.
Here are the locations of the start/stop calls that didn't seem to me to
need the timer bit: In GuiWorkArea: scrollTo; focusInEvent;
focusOutEvent; inputMethodEvent; in GuiView: restartCursor(). If that
doesn't seem right, let me know.
Richard
Index: src/frontends/qt4/GuiView.cpp
===================================================================
--- src/frontends/qt4/GuiView.cpp (revision 22756)
+++ src/frontends/qt4/GuiView.cpp (working copy)
@@ -619,10 +619,13 @@
{
if (d.current_work_area_) {
d.current_work_area_->setUpdatesEnabled(busy);
- if (busy)
+ if (busy) {
d.current_work_area_->stopBlinkingCursor();
- else
+ d.current_work_area_->stopGeneralTimer();
+ } else {
d.current_work_area_->startBlinkingCursor();
+ d.current_work_area_->startGeneralTimer();
+ }
}
if (busy)
Index: src/frontends/qt4/GuiWorkArea.cpp
===================================================================
--- src/frontends/qt4/GuiWorkArea.cpp (revision 22755)
+++ src/frontends/qt4/GuiWorkArea.cpp (working copy)
@@ -211,7 +211,7 @@
general_timer_.setInterval(500);
connect(&general_timer_, SIGNAL(timeout()),
this, SLOT(handleRegularEvents()));
- general_timer_.start();
+ startGeneralTimer();
screen_ = QPixmap(viewport()->width(), viewport()->height());
cursor_ = new frontend::CursorWidget();
@@ -333,11 +333,16 @@
void GuiWorkArea::processKeySym(KeySymbol const & key, KeyModifier mod)
{
// In order to avoid bad surprise in the middle of an operation,
- // we better stop the blinking cursor.
+ // we better stop the blinking cursor...
stopBlinkingCursor();
+ // ...and the general timer.
+ stopGeneralTimer();
theLyXFunc().setLyXView(lyx_view_);
theLyXFunc().processKeySym(key, mod);
+
+ // the cursor gets restarted in GuiView::restartCursor()
+ startGeneralTimer();
}
@@ -364,11 +369,15 @@
else
cmd = cmd0;
+ bool const notJustMovingTheMouse =
+ cmd.action != LFUN_MOUSE_MOTION || cmd.button() != mouse_button::none;
+
// In order to avoid bad surprise in the middle of an operation, we better stop
- // the blinking cursor.
- if (!(cmd.action == LFUN_MOUSE_MOTION
- && cmd.button() == mouse_button::none))
+ // the blinking cursor and the general timer
+ if (notJustMovingTheMouse) {
stopBlinkingCursor();
+ stopGeneralTimer();
+ }
buffer_view_->mouseEventDispatch(cmd);
@@ -379,15 +388,15 @@
}
// GUI tweaks except with mouse motion with no button pressed.
- if (!(cmd.action == LFUN_MOUSE_MOTION
- && cmd.button() == mouse_button::none)) {
+ if (notJustMovingTheMouse) {
// Slight hack: this is only called currently when we
// clicked somewhere, so we force through the display
// of the new status here.
lyx_view_->clearMessage();
- // Show the cursor immediately after any operation.
+ // Show the cursor immediately after any operation
startBlinkingCursor();
+ startGeneralTimer();
}
}
Index: src/frontends/qt4/GuiWorkArea.h
===================================================================
--- src/frontends/qt4/GuiWorkArea.h (revision 22749)
+++ src/frontends/qt4/GuiWorkArea.h (working copy)
@@ -121,6 +121,10 @@
void stopBlinkingCursor();
///
void startBlinkingCursor();
+ ///
+ void startGeneralTimer() { general_timer_.start(); }
+ ///
+ void stopGeneralTimer() { general_timer_.stop(); }
/// Process Key pressed event.
/// This needs to be public because it is accessed externally by GuiView.
void processKeySym(KeySymbol const & key, KeyModifier mod);
