01/08/2013 20:19, Hashini Senaratne:
I carefully followed your reviews and modified the code accordingly.
Your comments on them were really useful.
Hello Hashini,
I am now back from vacation for a week and took the time to review the
current code and change it according to the ideas that we have discussed
in July. It strikes me as a better way to state my ideas (and was also a
good way to understand what ideas do not work...).
With these changes, I see less problematic cases. There is still
something related to target_x that has to be sorted out.
Could you please make sure that you understand what I do there, discuss
what you do not like/understand and then commit the corresponding patch?
Then it will be time to make a list of the remaining things to do about
keyboard navigation.
Regards,
JMarc
PS: could you please commit the missing wide_image used in your
changesNeeded.lyx test files?
>From 9cb08235268e284b8c262dc556c04716f1739796 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Tue, 20 Aug 2013 15:10:28 +0200
Subject: [PATCH] Some cleanups to current row handling
This commit implements some ideas I have been discussing earlier about horizontal scrolling
* reset left_edge_ when changing cursor row (in Cursor::setCurrentRow)
* Cursor::setCurrentRow is now called in BufferView::draw
* small fix to setting of x in TextMetrics::drawParagraph(): the value used should be the one that has just been computed.
Some additional cleaups:
* the extra Cursor members are now mutable for simplicity. This can be changed later.
* There is a new previous_row_ member that is useful to redraw a row that was scrolled but which is not any more.
* new method Cursor::bottomRow(), that goes together with textRow()
* various style changes (constify variables, avoid useless declarations, move declarations where they belong...)
* Some comments updates
---
src/BufferView.cpp | 3 +++
src/Cursor.cpp | 38 +++++++++++++++++++++++++++++++-------
src/Cursor.h | 25 +++++++++++++++++--------
src/TextMetrics.cpp | 38 +++++++++++++++-----------------------
4 files changed, 66 insertions(+), 38 deletions(-)
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 8419460..a2c3d0c 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -2847,6 +2847,9 @@ void BufferView::draw(frontend::Painter & pain)
int const y = tm.first().second->position();
PainterInfo pi(this, pain);
+ // Set the row on which the cursor lives.
+ cursor().setCurrentRow(&cursor().bottomRow());
+
switch (d->update_strategy_) {
case NoScreenUpdate:
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index 95846ec..bcc84c4 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -543,33 +543,57 @@ Row const & Cursor::textRow() const
}
+Row const & Cursor::bottomRow() const
+{
+ CursorSlice const & cs = bottom();
+ ParagraphMetrics const & pm = bv().parMetrics(cs.text(), cs.pit());
+ bool const bndry = inTexted() ? boundary() : false;
+ return pm.getRow(cs.pos(), bndry);
+}
+
+
int Cursor::getLeftEdge() const
{
return left_edge_;
}
-Row const & Cursor::getCurrentRow() const
+Row const * Cursor::getCurrentRow() const
{
- return *current_row_;
+ return current_row_;
}
-void Cursor::setLeftEdge(int leftEdge)
+Row const * Cursor::getPreviousRow() const
{
- left_edge_ = leftEdge;
+ return previous_row_;
}
void Cursor::setLeftEdge(int leftEdge) const
{
- const_cast<Cursor *>(this)->setLeftEdge(leftEdge);
+ left_edge_ = leftEdge;
}
-void Cursor::setCurrentRow(Row const & wideRow) const
+void Cursor::setCurrentRow(Row const * wideRow) const
{
- *current_row_ = wideRow;
+ // nothing to do if the cursor was already on this row
+ if (current_row_ == wideRow) {
+ previous_row_ = 0;
+ return;
+ }
+
+ // if the (previous) current row was scrolled, we have to
+ // remember it in order to repaint it next time.
+ if (left_edge_ != 0)
+ previous_row_ = current_row_;
+ else
+ previous_row_ = 0;
+
+ // Since we changed row, the scroll offset is not valid anymore
+ left_edge_ = 0;
+ current_row_ = wideRow;
}
diff --git a/src/Cursor.h b/src/Cursor.h
index a48c0a6..7f47745 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -185,15 +185,22 @@ public:
void getSurroundingPos(pos_type & left_pos, pos_type & right_pos);
/// the row in the paragraph we're in
Row const & textRow() const;
+ /// the top-level row that holds the cursor
+ Row const & bottomRow() const;
+
+ ///
+ /// Methods useful for horizontal scrolling of rows
+ ///
/// returns the pixel value at the left edge of the screen
int getLeftEdge() const;
/// set the pixel value at the left edge of the screen
void setLeftEdge(int leftEdge) const;
- void setLeftEdge(int leftEdge);
- /// returns the row which slid finally
- Row const & getCurrentRow() const;
- /// set the row which slid finally
- void setCurrentRow(Row const & wideRow) const;
+ /// return the row where cursor is currently
+ Row const * getCurrentRow() const;
+ /// set the row where cursor is currently
+ void setCurrentRow(Row const * wideRow) const;
+ /// return the row where cursor was at previous draw event
+ Row const * getPreviousRow() const;
//
// common part
@@ -407,9 +414,11 @@ private:
int beforeDispatchPosX_;
int beforeDispatchPosY_;
/// the the pixel value at the left edge of the screen where the cursor is in
- int left_edge_;
- /// a pointer to the row which slid finally
- Row * current_row_;
+ mutable int left_edge_;
+ /// a pointer to the row where cursor is currently
+ mutable Row const * current_row_;
+ /// a pointer to the row where cursor was at previous draw event
+ mutable Row const * previous_row_;
///////////////////////////////////////////////////////////////////
//
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index de7f46d..830cb6a 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -2099,7 +2099,7 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co
// We store the begin and end pos of the selection relative to this par
DocIterator sel_beg_par = cur.selectionBegin();
DocIterator sel_end_par = cur.selectionEnd();
-
+
// We care only about visible selection.
if (selection) {
if (pit != sel_beg.pit()) {
@@ -2111,14 +2111,6 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co
sel_end_par.pos() = sel_end_par.lastpos();
}
}
-
- // Get the top-level row in which the cursor is, that is the
- // one that is not indide insets. This code is adapted from
- // Cursor::textRow.
- CursorSlice const & cs = cur.bottom();
- ParagraphMetrics const & bottom_pm = cur.bv().parMetrics(cs.text(), cs.pit());
- bool const bndry = (cur.depth() == 1) && cur.boundary();
- Row const & cur_bottom_row = bottom_pm.getRow(cs.pos(), bndry);
for (size_t i = 0; i != nrows; ++i) {
@@ -2131,27 +2123,24 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co
// It is not needed to draw on screen if we are not inside.
pi.pain.setDrawingEnabled(inside && original_drawing_state);
- BufferView & bv = cur.bv();
-
- // Current x position of the cursor in pixels
- int cur_x = bv.getPos(cur).x_;
+ // Check for too wide insets to handle horizontal sliding
+ if (cur.getCurrentRow() == &row) {
+ // Current x position of the cursor in pixels
+ int const cur_x = bv_->getPos(cur).x_;
- // Left edge value of the screen in pixels
- int left_edge = cur.getLeftEdge();
+ // Left edge value of the screen in pixels
+ int left_edge = cur.getLeftEdge();
- // Check for too wide insets to handle horizontal sliding
- if (&cur_bottom_row == &row)
- {
// If need to slide right
if (cur_x < left_edge + 10) {
- cur.setLeftEdge(cur_x - 10);
+ left_edge = cur_x - 10;
}
-
// If need to slide left ()
- else if (cur_x > left_edge + bv.workWidth() - 10) {
- cur.setLeftEdge(cur_x - bv.workWidth() + 10);
+ else if (cur_x > left_edge + bv_->workWidth() - 10) {
+ left_edge = cur_x - bv_->workWidth() + 10;
}
x -= left_edge;
+ cur.setLeftEdge(left_edge);
}
RowPainter rp(pi, *text_, pit, row, bidi, x, y);
@@ -2172,7 +2161,10 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co
// Row signature; has row changed since last paint?
row.setCrc(pm.computeRowSignature(row, bparams));
- bool row_has_changed = row.changed();
+ // If this row has been rememebered in cursor, it has
+ // to be redone
+ bool const row_has_changed
+ = row.changed() || &row == cur.getPreviousRow();
// Take this opportunity to spellcheck the row contents.
if (row_has_changed && lyxrc.spellcheck_continuously) {
--
1.7.0.4
