Cursor keys not working when the worksheet contains an image cell

[wiesiwies]

Problem

When the worksheet contains an image cell, the cursor can no longer be moved with the cursor keys as expected.

Steps to reproduce

• Create a blank worksheet.
• Insert an image, for example, try wxplot2d(x, [x, 0, 1]);. This inserts an image (the plot) created by gnuplot.
• Create a new cell below the image and type something.
• Try to move the cursor left and right with the cursor keys. Nothing happens.
  • Up and down works.
  • Input cells above all image cells work; Cells between and below the images don't.
• Collapse all image cells => The cursor keys work again in the cells below the image cells.

Maybe this is some kind of focus issue, i.e., the wrong cells getting the keyboard events?

Tested versions

• Fresh build of 5604eee1f on Debian 12
• 64 bit Windows installer from SourceForge (maxima-5.47.0-win64.exe) on Windows 10. (Note: I'm not exactly sure which keys work in which circumstances, because I don't have a Windows machine accessible at the time of writing this bug report.)

[wiesiwies]

I did a git bisect and found that the bug first appears in fed284d. The problem goes away if you comment the line

m_regionToRefresh.Intersect(m_configuration->GetVisibleRegion());

However, the source of the trouble is somewhere else: Investigating the output of GetVisibleRegion(), I found that it outputs negative values for the bottom and/or top coordinates if you scroll down in the worksheet. m_regionToRefresh on the other hand never seems to contain negative vertical coordinates, causing the Intersect() call to return an empty region.

So I suspected the problem is not triggered by inserting the image cell, but simply caused by scrolling down the worksheet. And, indeed, in my test build it is sufficient to insert enough cells to make the worksheet scroll down to trigger the bug.

GetVisibleRegion() just returns m_visibleRegion which is set by SetVisibleRegion() which in turn is only called in OnPaint() and RecalculateIfNeeded() in the context of

  CalcScrolledPosition(0, 0, &upperLeftScreenCorner.x,
                       &upperLeftScreenCorner.y);
  m_configuration->SetVisibleRegion(wxRect(
                       upperLeftScreenCorner, upperLeftScreenCorner + wxPoint(width, height)));

I think the purpose of the first line is to get the position of the upper leftmost visible point in the wxScroll, but for this, CalcUnscrolledPosition must be used. CalcScrolledPosition is for the coordinate transformation in the other direction.

The following patch fixes the problem:

diff --git a/src/Worksheet.cpp b/src/Worksheet.cpp
index c361cd453..ad5e7266f 100644
--- a/src/Worksheet.cpp
+++ b/src/Worksheet.cpp
@@ -548,8 +548,9 @@ void Worksheet::OnPaint(wxPaintEvent &WXUNUSED(event)) {
    GetClientSize(&width, &height);

    wxPoint upperLeftScreenCorner;
-   CalcScrolledPosition(0, 0, &upperLeftScreenCorner.x,
+       CalcUnscrolledPosition(0, 0, &upperLeftScreenCorner.x,
                 &upperLeftScreenCorner.y);
+
    wxRect visibleRegion = wxRect(upperLeftScreenCorner,
                      upperLeftScreenCorner + wxPoint(width, height));

@@ -958,7 +959,7 @@ bool Worksheet::RecalculateIfNeeded(bool timeout) {
   GetClientSize(&width, &height);

   wxPoint upperLeftScreenCorner;
-  CalcScrolledPosition(0, 0, &upperLeftScreenCorner.x,
+  CalcUnscrolledPosition(0, 0, &upperLeftScreenCorner.x,
                        &upperLeftScreenCorner.y);
   m_configuration->SetVisibleRegion(wxRect(
                       upperLeftScreenCorner, upperLeftScreenCorner + wxPoint(width, height)));

Don't know if this causes any side effects.

@gunterkoenigsmann: Maybe you can review the patch?

[gunterkoenigsmann]

Soory for my delayed answer - and thanks for throughly analyzing the problem: I kept searching for the reason in all the wrong places so without your help I would never have found it.

Additionally to applying your patch I have moved the code out of the loop (in which it only wastes time) and removed the 2nd instance of that code that I believe to be redundant.