15/09/2013 19:32, Hashini Senaratne:
I do not know how to get a CursorSlice that points the start character of a
row. So I tried with the following, within the Cursor.cpp.
Have a look at the updated patch below that implements the new way of
identifying rows. It seems to work in my limited testing.
In general, I think that approaches that hardcode tests for whether
cursor is in mathed are bound to fail. You are testing one particular
use case, but there are many others. Therefore we need more than
workarounds.
I checked the current implementation with too wide tables by commenting the
content in void InsetTabular::resetPos(Cursor & cur) const. It seems to work
with tables, but the proper behaviour seems to appearing with fullpaint updates.
What do you mean about fullpain updates?
Encountered a new bug,
When deleting the content of a too wide row using backspace, the row does
not slide. Results is:
https://dl.dropboxusercontent.com/u/105510128/Bug_4.webm
Hmm, is this really a bug? You can try to check whether row.width() -
leftEdge is less that workwidth and set left_edge more precisely, if you
prefer.
JMarc
>From ba4bfa3749b98a4de94255433941c627651c2aec Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Mon, 16 Sep 2013 11:19:06 +0200
Subject: [PATCH] Fix problem with disappearing Row objects
It appears that row objects can disappear when doing a decoration update for example. This means that comparing row pointers is not a good idea, be cause they can change without notice.
In this patch, a row is identified by a CursorSlice that points to the beginning of the row. The Cursor members are modified correspondingly.
Then, using this new framework, a few fixes are made:
- remove early return in checkLeftEdge
- force update also if the previous row needs to be repainted
- remove places in Cursor code where left_edge was reset to 0 explicitly
---
src/BufferView.cpp | 21 ++++++++-------------
src/Cursor.cpp | 30 +++++++++++++-----------------
src/Cursor.h | 22 ++++++++++++----------
src/CursorSlice.h | 2 ++
src/TextMetrics.cpp | 10 +++++++---
5 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 4222315..4b2628f 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -2846,21 +2846,12 @@ void checkCursorLeftEdge(PainterInfo & pi, Cursor const & cur,
{
Bidi bidi;
Row const & row = cur.bottomRow();
+ CursorSlice rowSlice = cur.bottom();
+ rowSlice.pos() = row.pos();
BufferView const & bv = cur.bv();
- bool row_moved = false;
-
- // Left edge value of the screen in pixels
- int left_edge = cur.getLeftEdge();
// Set the row on which the cursor lives.
- cur.setCurrentRow(&row);
-
- // If the row has changed, return for the first time
- // This is because the row chage within math inset for mouse clicks
- if (cur.getLeftEdge() == 0
- && left_edge != 0) {
- return;
- }
+ cur.setCurrentRowSlice(rowSlice);
// Force the recomputation of inset positions
bool const drawing = pi.pain.isDrawingEnabled();
@@ -2873,6 +2864,10 @@ void checkCursorLeftEdge(PainterInfo & pi, Cursor const & cur,
// 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();
+ bool row_moved = false;
+
// If need to slide right
if (cur_x < left_edge + 10) {
left_edge = cur_x - 10;
@@ -2885,7 +2880,7 @@ void checkCursorLeftEdge(PainterInfo & pi, Cursor const & cur,
row_moved = true;
}
- if (strategy == NoScreenUpdate && row_moved) {
+ if (strategy == NoScreenUpdate && (row_moved || !cur.getPreviousRowSlice().empty())) {
// FIXME: if one uses SingleParUpdate, then home/end
// will not work on long rows. Why?
strategy = FullScreenUpdate;
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index 2c537d3..92348c2 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -556,15 +556,15 @@ int Cursor::getLeftEdge() const
}
-Row const * Cursor::getCurrentRow() const
+CursorSlice const & Cursor::getCurrentRowSlice() const
{
- return current_row_;
+ return current_row_slice_;
}
-Row const * Cursor::getPreviousRow() const
+CursorSlice const & Cursor::getPreviousRowSlice() const
{
- return previous_row_;
+ return previous_row_slice_;
}
@@ -574,24 +574,24 @@ void Cursor::setLeftEdge(int leftEdge) const
}
-void Cursor::setCurrentRow(Row const * wideRow) const
+void Cursor::setCurrentRowSlice(CursorSlice const & rowSlice) const
{
// nothing to do if the cursor was already on this row
- if (current_row_ == wideRow) {
- previous_row_ = 0;
+ if (current_row_slice_ == rowSlice) {
+ previous_row_slice_ = CursorSlice();
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_;
+ previous_row_slice_ = current_row_slice_;
else
- previous_row_ = 0;
+ previous_row_slice_ = CursorSlice();
// Since we changed row, the scroll offset is not valid anymore
left_edge_ = 0;
- current_row_ = wideRow;
+ current_row_slice_ = rowSlice;
}
@@ -1792,7 +1792,6 @@ int Cursor::targetX() const
int x = 0;
int y = 0;
getPos(x, y);
- const_cast<Cursor *>(this)->setLeftEdge(0);
return x;
}
@@ -1896,10 +1895,9 @@ bool Cursor::upDownInMath(bool up)
// check if we had something else in mind, if not, this is the future
// target
- if (x_target_ == -1){
+ if (x_target_ == -1)
setTargetX(xo);
- left_edge_=0;
- }
+
else if (inset().asInsetText() && xo - textTargetOffset() != x_target()) {
// In text mode inside the line (not left or right) possibly set a new target_x,
// but only if we are somewhere else than the previous target-offset.
@@ -2037,10 +2035,8 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
// update the targetX - this is here before the "return false"
// to set a new target which can be used by InsetTexts above
// if we cannot move up/down inside this inset anymore
- if (x_target_ == -1){
+ if (x_target_ == -1)
setTargetX(xo);
- left_edge_=0;
- }
else if (xo - textTargetOffset() != x_target() &&
depth() == beforeDispatchCursor_.depth()) {
// In text mode inside the line (not left or right)
diff --git a/src/Cursor.h b/src/Cursor.h
index 7f47745..3a98d7a 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -195,12 +195,14 @@ public:
int getLeftEdge() const;
/// set the pixel value at the left edge of the screen
void setLeftEdge(int leftEdge) 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;
+ /// return a slice describing the row where cursor is currently
+ CursorSlice const & getCurrentRowSlice() const;
+ /// set the slice describing the row where cursor is currently
+ void setCurrentRowSlice(CursorSlice const & rowSlice) const;
+ /// return the row slice where cursor was at previous draw
+ /// event. Slice is empty if there is no need to redraw this
+ /// row.
+ CursorSlice const & getPreviousRowSlice() const;
//
// common part
@@ -415,10 +417,10 @@ private:
int beforeDispatchPosY_;
/// the the pixel value at the left edge of the screen where the cursor is in
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_;
+ /// a slice pointing to the start of the row where cursor is currently
+ mutable CursorSlice current_row_slice_;
+ /// a slice pointing to the start of the row where cursor was at previous draw event
+ mutable CursorSlice previous_row_slice_;
///////////////////////////////////////////////////////////////////
//
diff --git a/src/CursorSlice.h b/src/CursorSlice.h
index 01634bd..4da530d 100644
--- a/src/CursorSlice.h
+++ b/src/CursorSlice.h
@@ -65,6 +65,8 @@ public:
friend bool operator<=(CursorSlice const &, CursorSlice const &);
//@}
+ /// return true if the slice has not been initialized
+ bool empty() const { return !inset_; }
/// the current inset
Inset & inset() const { return *inset_; }
/// return the cell this cursor is in
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index f53c46f..0311427 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -2116,6 +2116,10 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co
for (size_t i = 0; i != nrows; ++i) {
Row const & row = pm.rows()[i];
+ CursorSlice rowSlice(const_cast<InsetText &>(text_->inset()));
+ rowSlice.pit() = pit;
+ rowSlice.pos() = row.pos();
+
if (i)
y += row.ascent();
@@ -2123,7 +2127,7 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co
&& y - row.ascent() < ww);
// Adapt to cursor row left edge if applicable.
- if (cur.getCurrentRow() == &row)
+ if (cur.getCurrentRowSlice() == rowSlice)
x -= cur.getLeftEdge();
// It is not needed to draw on screen if we are not inside.
@@ -2147,10 +2151,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));
- // If this row has been rememebered in cursor, it has
+ // If this row has been remembered in cursor, it has
// to be redone
bool const row_has_changed
- = row.changed() || &row == cur.getPreviousRow();
+ = row.changed() || rowSlice == cur.getPreviousRowSlice();
// Take this opportunity to spellcheck the row contents.
if (row_has_changed && lyxrc.spellcheck_continuously) {
--
1.7.0.4
