nsterm.m: How to prevent _inactive_ window update from overreaching.

nsterm.m: How to prevent _inactive_ window update from overreaching.

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

In working on feature requests #17684 (crosshairs / visible fill column) and #22873 (multiple fake cursors), I have come across a situation where nsterm.m updates a portion of the _active_ window when updating the _inactive_ window.  Although this is the case as of 11/16/2018, it was not the case back on 07/07/2018.  The change occurred sometime between 07/07/2018 and 11/16/2018.

In the following examples, the _active_ window on the right gets updated first in time.  Using a version of Emacs from 07/07/2018, updating the _inactive_ window does not cause the active window to be modified.  Using a version of Emacs from 11/16/2018, updating the _inactive_ window updates a portion of the _active_ window.  The update happens from the top of the window to the X axis of the cursor in the _inactive_ window.

In the 07/07/2018 screenshot, the crosshairs and visible fill column in the _active_ window are not touched by updates to the _inactive_ window.

In the 11/16/2018 screenshot, the crosshairs and visible fill column in the _active_ window are partially erased by updates to the _inactive_ window.

https://www.lawlist.com/images/nsterm_07_07_2018_a.png

https://www.lawlist.com/images/nsterm_11_16_2018_a.png

The 07/07/2018 example was made with OSX (10.6.8) using Emacs master branch bearing commit 98463bed07ecc9057094928ffdc2321eaaf3ab52 and applying the attached patch (98463bed07ecc9057094928ffdc2321eaaf3ab52__2018_07_07.diff).

The 11/16/2018 example was made with OSX (10.6.8) using Emacs master branch bearing commit 644a308b4e1513e04be9360e1586e14b32ec0159 and applying the attached patch (644a308b4e1513e04be9360e1586e14b32ec0159__2018_11_16.diff).

QUESTION:  When updating the _inactive_ window, how can I prevent the current version of nsterm.m from updating a portion of the _active_ window (from the top of the window to the X axis of the cursor in the _inactive_ window)?

STEP 1:  Apply the relevant patch to the applicable Emacs versions from either 07/07/2018 or 11/16/2018, and build Emacs --with-ns.

STEP 2:  From the *GNU Emacs* screen, type:

M-F10 (toggle-frame-maximized)

M-x split-window-horizontally

C-x o (other-window)

M-x +-mode

Move the cursor in the _active_ window (on the right side of the frame) to the letter "i" in "Darwin", which is the third line from the bottom.  The results should be the same as in the screenshots above.



[-- Attachment #2: 644a308b4e1513e04be9360e1586e14b32ec0159__2018_11_16.diff --]
[-- Type: application/diff, Size: 375541 bytes --]

[-- Attachment #3: 98463bed07ecc9057094928ffdc2321eaaf3ab52__2018_07_07.diff --]
[-- Type: application/diff, Size: 374957 bytes --]

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Keith David Bershatsky]

Correction to previous post ... instead of the "X axis", I meant to say the "Y axis".

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> * * *
> 
> In the following examples, the _active_ window on the right gets updated first in time.  Using a version of Emacs from 07/07/2018, updating the _inactive_ window does not cause the active window to be modified.  Using a version of Emacs from 11/16/2018, updating the _inactive_ window updates a portion of the _active_ window.  The update happens from the top of the window to the X axis of the cursor in the _inactive_ window.
> 
> * * *
> 
> QUESTION:  When updating the _inactive_ window, how can I prevent the current version of nsterm.m from updating a portion of the _active_ window (from the top of the window to the X axis of the cursor in the _inactive_ window)?
> 
> * * *

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Alan Third]

Hi Keith,

On Thu, Nov 22, 2018 at 11:07:26AM -0800, Keith David Bershatsky wrote:
> In working on feature requests #17684 (crosshairs / visible fill
> column) and #22873 (multiple fake cursors), I have come across a
> situation where nsterm.m updates a portion of the _active_ window
> when updating the _inactive_ window. Although this is the case as of
> 11/16/2018, it was not the case back on 07/07/2018. The change
> occurred sometime between 07/07/2018 and 11/16/2018.
> 
> In the following examples, the _active_ window on the right gets
> updated first in time. Using a version of Emacs from 07/07/2018,
> updating the _inactive_ window does not cause the active window to
> be modified. Using a version of Emacs from 11/16/2018, updating the
> _inactive_ window updates a portion of the _active_ window. The
> update happens from the top of the window to the X axis of the
> cursor in the _inactive_ window.
> 
> In the 07/07/2018 screenshot, the crosshairs and visible fill column
> in the _active_ window are not touched by updates to the _inactive_
> window.
> 
> In the 11/16/2018 screenshot, the crosshairs and visible fill column
> in the _active_ window are partially erased by updates to the
> _inactive_ window.

There are two (potential) issues here:

 1. Something is marking a large portion of the frame as dirty, and
 it’s overflowing from the left window to the right, probably all the
 way to the right edge of the frame. That’s almost certainly a call to
 ns_clip_to_rect.

 2. When the right window is marked as dirty it’s failing to redraw
 completely. I’d assume what is happening is that the expose_frame
 functions (and its descendants) don’t notice that the crosshair needs
 to be redrawn. I’m unsure how that side of it works, something must
 have to compare the rectangle to be exposed with the contents of the
 frame and decide what needs redrawn.

IMHO both should be fixed. 1 because it’s redrawing too much, and 2
because there are other circumstances where part of your crosshair
could be marked as dirty and therefore need redrawn.

-- 
Alan Third

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Keith David Bershatsky]

Thank you, Alan, for pointing me in the right direction.

I will continue working on this project in my spare time.

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [11-22-2018 14:57:20] <22 Nov 2018 22:57:20 +0000>
> From: Alan Third <alan@idiocy.org>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Subject: Re: nsterm.m: How to prevent _inactive_ window update from overreaching.
> 
> * * *
> 
> There are two (potential) issues here:
> 
>  1. Something is marking a large portion of the frame as dirty, and
> it's overflowing from the left window to the right, probably all the
> way to the right edge of the frame. That's almost certainly a call to
> ns_clip_to_rect.
> 
>  2. When the right window is marked as dirty it's failing to redraw
> completely. I'd assume what is happening is that the expose_frame
> functions (and its descendants) don't notice that the crosshair needs
> to be redrawn. I'm unsure how that side of it works, something must
> have to compare the rectangle to be exposed with the contents of the
> frame and decide what needs redrawn.
> 
> IMHO both should be fixed. 1 because it's redrawing too much, and 2
> because there are other circumstances where part of your crosshair
> could be marked as dirty and therefore need redrawn.
> 
> --
> Alan Third

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 3876 bytes --]

I isolated the issue to the following changes within nsterm.m that were made between the dates of 07/07/2018 and 11/16/2018.  The additions (+) permit crosshairs to work properly; whereas, the subtractions (-) cause the issue outlined at the beginning of this thread.

The crosshairs code (in the attached patch from 11/16/2018) work correctly on all three GUI platforms:  NS, NT, X11.

Is it possible that something in the changes below is responsible for the issue, rather than the crosshairs code itself?

Keith

diff --git a/src/nsterm.m b/src/nsterm.m
index bcc23ff..eb7b72b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -278,6 +278,9 @@ - (NSColor *)colorUsingDefaultColorSpace
 long context_menu_value = 0;
 
 /* display update */
+static struct frame *ns_updating_frame;
+static NSView *focus_view = NULL;
+static BOOL gsaved = NO;
 static int ns_window_num = 0;
 static BOOL ns_fake_keydown = NO;
 #ifdef NS_IMPL_COCOA
@@ -1103,6 +1116,9 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
       [toolbar setVisible: tbar_visible];
   }
 
+  ns_updating_frame = f;
+  [view lockFocus];
+
   /* drawRect may have been called for say the minibuffer, and then clip path
      is for the minibuffer.  But the display engine may draw more because
      we have set the frame as garbaged.  So reset clip path to the whole
@@ -1206,12 +1222,21 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
    external (RIF) call; for whole frame, called after update_window_end
    -------------------------------------------------------------------------- */
 {
+  EmacsView *view = FRAME_NS_VIEW (f);
+
   NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_end");
 
 /*   if (f == MOUSE_HL_INFO (f)->mouse_face_mouse_frame) */
   MOUSE_HL_INFO (f)->mouse_face_defer = 0;
-}
 
+  block_input ();
+
+  [view unlockFocus];
+  [[view window] flushWindow];
+
+  unblock_input ();
+  ns_updating_frame = NULL;
+}
 
 static BOOL
 ns_clip_to_rect (struct frame *f, NSRect *r, int n)
@@ -1226,23 +1261,31 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
     {
       NSTRACE_RECT ("r", *r);
 
-      if ([NSView focusView] == FRAME_NS_VIEW (f))
-        {
-          [[NSGraphicsContext currentContext] saveGraphicsState];
-          if (n == 2)
-            NSRectClipList (r, 2);
-          else
-            NSRectClip (*r);
-
-          return YES;
-        }
-      else
+      if (f != ns_updating_frame)
         {
           NSView *view = FRAME_NS_VIEW (f);
-          int i;
-          for (i = 0 ; i < n ; i++)
-            [view setNeedsDisplayInRect:r[i]];
+          if (view != focus_view)
+            {
+              if (focus_view != NULL)
+                {
+                  [focus_view unlockFocus];
+                  [[focus_view window] flushWindow];
+              /* debug_lock--; */
+              return YES;
+                }
+              if (view)
+                [view lockFocus];
+              focus_view = view;
+              /* if (view) debug_lock++; */
+            }
         }
+      [[NSGraphicsContext currentContext] saveGraphicsState];
+      if (n == 2)
+        NSRectClipList (r, 2);
+      else
+        NSRectClip (*r);
+      gsaved = YES;
+      return YES;
     }
 
   return NO;
@@ -1256,7 +1299,22 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 {
   NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "ns_reset_clipping");
 
-  [[NSGraphicsContext currentContext] restoreGraphicsState];
+  if (gsaved)
+    {
+      [[NSGraphicsContext currentContext] restoreGraphicsState];
+      gsaved = NO;
+    }
+
+  if (f != ns_updating_frame)
+    {
+      if (focus_view != NULL)
+        {
+          [focus_view unlockFocus];
+          [[focus_view window] flushWindow];
+          focus_view = NULL;
+/* debug_lock--; */
+        }
+    }
 }
 
 


[-- Attachment #2: 644a308b4e1513e04be9360e1586e14b32ec0159__2018_11_16.diff --]
[-- Type: application/diff, Size: 377892 bytes --]

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Alan Third]

On Sun, Nov 25, 2018 at 06:14:19PM -0800, Keith David Bershatsky wrote:
> I isolated the issue to the following changes within nsterm.m that
> were made between the dates of 07/07/2018 and 11/16/2018. The
> additions (+) permit crosshairs to work properly; whereas, the
> subtractions (-) cause the issue outlined at the beginning of this
> thread.
> 
> The crosshairs code (in the attached patch from 11/16/2018) work
> correctly on all three GUI platforms: NS, NT, X11.
> 
> Is it possible that something in the changes below is responsible
> for the issue, rather than the crosshairs code itself?

Well, yes, in that your patch simply undoes the key changes required
for macOS Mojave compatibility.

However the crosshair code needs to behave itself WRT the expose
functions. I strongly suspect it isn’t.

I’m not sure how either really works, so I can’t point out what needs
done without some investigation. It’s possible we just need to call
the crosshair code from somewhere similar to the various places we
call the cursor code in nsterm.m.

Because we can’t draw directly to the screen any more, when redisplay
is running all we can do is mark the parts of the frame that need
updated. Later we call expose_frame on those parts and it does the
actual drawing.

That process must be working to some extent. I don’t know your code
very well but I suspect the crosshair code is called when the cursor
is redrawn. If part of the frame is marked as dirty, but that doesn’t
include the cursor, then the cursor isn’t redrawn and therefore your
crosshair code will not be called. Does that make sense?

If that is the case we need to work out how to ensure the crosshair
code is called even if the cursor is not redrawn.

I’ve had a quick look at the patches and, if I’m honest, it would take
me some time to work out what’s going on. How is the crosshair drawn?
-- 
Alan Third

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Keith David Bershatsky]

Feature requests #17684 (crosshairs / visible fill column) and #22873 (multiple fake cursors) have not been approved by anyone on the Emacs development team.  There has been some discussion regarding whether multiple fake cursors should become a part of Emacs someday, but nothing was ever decided.  I continue to develop these features because I use them in my every day workflow and am hoping to convince the Emacs development team to someday include similar features.  I figure that if I do a good enough job, eventually the Emacs development team will say:  "sure, why not ..."

-  Fake cursors are removed in two (2) locations within redisplay_internal (if the cursor cache exists).  If the frame is not being updated, then call update_begin before removing and call update_end when removing is complete.

-  Fake cursors are removed in two (2) locations within redisplay_window (if the cursor cache exists).  If the frame is not being updated, then call update_begin before removing and call update_end when removing is complete.

-  Whenever display_and_set_cursor is called and not immediately returned, fake cursors are calculated and drawn; or if the cursor cache already exist, then erase and redraw.  If the frame is not being updated, then call update_begin before drawing/removing and call update_end when drawing/removing is complete.

-  The calculations for drawing fake cursors are based upon modified versions of the functions dump_glyph_matrix and dump_glyph_row, which generate the correct coordinates for placing the fake cursors.

-  Fake cursors come in two types:  (1) cursor with a glyph on top; (2) cursor with nothing on top [e.g., floating anywhere on the visible window].

-  There are two basic settings:  (1) draw or erase/redraw forthwith (whenever the real cursor would ordinarily be drawn); or, (2) draw on an idle-timer and, if the cursor cache exists, then erase/redraw in certain circumstances when the real cursor would ordinarily be redrawn.  New calculations and cursor cache update occur on the initial call of either of the two scenarios.

The present design calculates and draws/erases fake cursors based upon particular visible windows that get updated during redisplay, rather than rectangles within a particular window or set of windows.  If I am understanding your comments correctly, it sounds like a new rectangle system of fake cursor calculation and drawing/erasing of fake cursors would need to be designed (solely for the benefit of MacOS Mojave users).  Or, at the very least, calculate fake cursors once for each visible window (where fake cursors are active) and then loop through cached coordinates when a rectangle is drawn to see if any fake cursors are contained within -- if so, then draw/erase fake cursors within that rectangle.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [11-26-2018 12:09:02] <26 Nov 2018 20:09:02 +0000>
> From: Alan Third <alan@idiocy.org>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: emacs-devel@gnu.org
> Subject: Re: nsterm.m: How to prevent _inactive_ window update from overreaching.
> 
> * * *
> 
> Because we can't draw directly to the screen any more, when redisplay
> is running all we can do is mark the parts of the frame that need
> updated. Later we call expose_frame on those parts and it does the
> actual drawing.
> 
> That process must be working to some extent. I don't know your code
> very well but I suspect the crosshair code is called when the cursor
> is redrawn. If part of the frame is marked as dirty, but that doesn't
> include the cursor, then the cursor isn't redrawn and therefore your
> crosshair code will not be called. Does that make sense?
> 
> If that is the case we need to work out how to ensure the crosshair
> code is called even if the cursor is not redrawn.
> 
> I've had a quick look at the patches and, if I'm honest, it would take
> me some time to work out what's going on. How is the crosshair drawn?
> --
> Alan Third

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Alan Third]

On Mon, Nov 26, 2018 at 08:58:59PM -0800, Keith David Bershatsky wrote:
> 
> The present design calculates and draws/erases fake cursors based
> upon particular visible windows that get updated during redisplay,
> rather than rectangles within a particular window or set of windows.
> If I am understanding your comments correctly, it sounds like a new
> rectangle system of fake cursor calculation and drawing/erasing of
> fake cursors would need to be designed (solely for the benefit of
> MacOS Mojave users).

It’s not new and it’s not solely for Mojave. It’s the ‘expose’
functionality which comes into use when, say, one window is partially
covering another, but is then removed. The GUI toolkit fires an
‘expose event’ for the area that has just been uncovered (a
rectangle), and the application redraws that area.

You can find the code in xdisp.c beginning with the function
expose_frame.

What makes this particularly noticeable with the NS port now is that
we’re using it as the primary drawing mechanism as Mojave has removed
the previously used method.

> Or, at the very least, calculate fake cursors
> once for each visible window (where fake cursors are active) and
> then loop through cached coordinates when a rectangle is drawn to
> see if any fake cursors are contained within -- if so, then
> draw/erase fake cursors within that rectangle

‘expose_window’ appears to redraw the cursor in exposed windows, so
that may be a good place to look.

FWIW I asked about how the crosshair is drawn (since that was what you
were discussing originally) but you only talked about multiple
cursors. Am I to assume then that the crosshair drawing is tightly
integrated with the multiple cursors?
-- 
Alan Third

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Keith David Bershatsky]

Thank you, Alan, for the additional pointers regarding how this issue may ultimately be resolved.  I will continue to think about and work on this in the coming days.

Yes, all three features (multiple fake cursors, crosshairs, and visible fill column) operate on the same principal.  There are similar functions for each feature, with the only main difference being that multiple fake cursors look for a particular buffer position when dumping the glyph matrix and looping through each row, whereas crosshairs and visible fill column look for a particular window relative X coordinate.

At this time, there is nothing fancy about multiple fake cursors -- i.e., Emacs only looks for a buffer position and creates a fake cursor with the desired cursor type and cursor color.  Magnar's Lisp library is still the only mechanism I am aware of that does the magic.  Instead of placing a visible overlay where fake cursors would ordinarily be created using Magnar's library, there are a few modifications to his Lisp functions that are contained within crosshairs.el.  From the list of fake cursor overlays created by Magnar's library, Emacs gathers the list of buffer positions and then draws built-in fake cursors that are within the confines of the visible windows.

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [11-28-2018 13:31:09] <28 Nov 2018 21:31:09 +0000>
> From: Alan Third <alan@idiocy.org>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: emacs-devel@gnu.org
> Subject: Re: nsterm.m: How to prevent _inactive_ window update from overreaching.
> 
> * * *
> 
> FWIW I asked about how the crosshair is drawn (since that was what you
> were discussing originally) but you only talked about multiple
> cursors. Am I to assume then that the crosshair drawing is tightly
> integrated with the multiple cursors?
> --
> Alan Third

Re: nsterm.m: How to prevent _inactive_ window update from overreaching.

[Keith David Bershatsky]

Alan:

I came up with two different approaches to resolve the issue described at the outset of this thread:

  A.  Add the fake cursors to the list of dirty rectangles, and then ensure that those areas get drawn during the expose frame/window calls from drawRect.

OR

  B.  Subsequent to the call of the dirty rectangle loop within drawRect, add all of the fake cursors wherever needed (at the tail end of drawRect).

I opted for the second approach and it appears to be working correctly.  I anticipate having a next draft of feature requests 17684 and 22873 within the coming days.

Thank you for the prior detailed explanations and helpful guidance -- greatly appreciated!

Keith