Hello Jean-Marc,
> 1/ In checkCursorLeftEdge, you introduce a new bool row_need_slide that
> in my mind is equivalent to the old test "left_edge !=
> cur.getLeftEdge()", since the goal is to know whether the row needs to
> be repainted. Does the new form make a difference?
>
> I am OK with the form with a boolean, actually. I would just suggest to
> rename it as row_needs_repaint. In general, I think it would be better
> to avoid the verb slide (that I proposed, I think :) and use "scroll"
> everywhere instead.
Yes, it is to know whether the row needs to be repainted, as you mentioned.
But, at that point I did not used left_edge != > cur.getLeftEdge() because
as I mentioned in my previous message, left_edge_ attribute in Cursor is set
to 0 by the method setCurrentRow().
I used this boolean to decide whether to change the strategy of painting in
the following code segment. That code segment is needed when we are moving
from a normal row to a too wide row, in such a way that we are accessing a
position that is not appeared to the current screen. eg: use left arrow key
to move to the right most position of the too wide row that is just above.
Unless we use this boolean/ the condition you have reminded (if it is
working properly all the time) painting strategy will get changed when there
is a no need to scroll, when moving among different rows.
if (cur.getCurrentRow() != cur.getPreviousRow()
&& strategy == NoScreenUpdate
&& row_need_slide) {
ScreenUpdateStrategy const oldstrat = strategy;
strategy = FullScreenUpdate;
}
But after the last commit I think now I can use, left_edge != >
cur.getLeftEdge() too. Although, to me this boolean is more clear. Yes, I
will follow the term you suggest. Hope that is more appropriate.
> 2/ In this same function, you add a test
> cur.getCurrentRow() != cur.getPreviousRow()
>
> Can you mention cases when this test is not true? In my mind,
> previousRow is NULL when the previous row does not need a repaint, and
> is non-null only in case where we left a row that was scrolled and need
> to repaint it. I agree that the naming is not very good, but still I
> wonder about the test you added.
Yes, I used that property along with the other conditions to call a full
update where only needed.
When the left_edge_ != 0, previous_row_ = current_row_. Otherwise
previous_row_ = 0 where cur.getCurrentRow() != cur.getPreviousRow().
> 3/ If I understand correctly, the fix that makes stuff work is that you
> force a full redraw when a row has been moved. Right?
Yes, I have forced full redraw where only needed. If you try debugging, you
will see that this only get called (will go in to this if condition) when
the below situation happens. So I do not think that is not a good thing to do.
*when we are moving from the leftmost position of a normal row to the
rightmost position of a too wide row, that is just above.
(Other way this is not get called)
*When using Home/ End in a too wide row (when the other end is not visible)
> 4/ Two questions about the following code
>
> + // Reset left edge if the row has changed and return
> + if (cur.getLeftEdge() == 0
> + && left_edge != 0) {
> + cur.setLeftEdge(0);
> + return;
> + }
>
> First, it seems to me that the "cur.setLeftEdge(0);" line does nothing.
> Am I right?
Yes, Thank you for showing that to me. I have forgotten to delete that line
after testing.
> Second, could you explain why returning early is a good idea in this case?
cur.getLeftEdge() == 0 happens normally when moving into a new row. And
also, it happens when I select a cursor position within a math inset, using
the mouse pointer. That is because we call setCurrentRow() before this.
left_edge == 0 if it is a new row (when moving into a new row). So with the
above condition true and with left_edge != 0, it indicates that we are on an
already drawn row (where the cursor lies on just before). And as the
selected cursor position is visible on screen, no need of scrolling.
This is actually to avoid unwanted scrolling happens when we are selecting
different cursor positions on a math inset, due to the changing value of '&row'.
> 5/ Finally, the part that is the most intriguing to me is the following:
>
> - left_edge_ = 0;
> + if (!selectionEnd().inMathed())
> + left_edge_ = 0;
>
> I guess it is related to the discussion below.
Yes when I am selecting different cursor positions using the mouse pointer,
&row gives varying values.
Thanks
Hashini
