bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#17684: #22873 (multiple fake cursors); and, #17684 (crosshairs / fil


From: Keith David Bershatsky
Subject: bug#17684: #22873 (multiple fake cursors); and, #17684 (crosshairs / fill-column).
Date: Sat, 02 May 2020 13:50:18 -0700

VERSION:  022.008 [05/02/2020]


CHANGELOG:

- Calculate and set it->lnum when both NILP (Vdisplay_line_numbers) and features
  22873 / 17684 are active.  Line numbers are now recorded in the struct of each
  glyph:  glyph->lnum = it->lnum.  The cache of multiple fake cursors has been
  revised to also record the lnum of each fake cursor.  The recorded lnum is 
used
  to test for a `same_p` fake cursor situation; i.e., whether fake cursors 
should
  be erased and/or redrawn.

. The lnum is needed for an apparently uncommon, but nevertheless reproducible
  situation wherein a screen line is added/deleted and there are two or more
  subsequent consecutive lines having the same content; and, the character(s)
  added/deleted is/are the same size/shape of the subsequent consecutive screen
  lines.  `mc_update_window_erase' will erase a fake cursor on one of those
  subsequent consecutive lines depending upon whether the a screen line above is
  added/deleted.  With the unique exception of the recorded lnum, all other data
  in the cache of fake cursors will be identical as to one of those subsequent
  consecutive screen lines.

. EXAMPLE (word-wrap is non-nil) -- modification of the first paragraph by 
adding/
  deleting the word "boy" causes the screen line containing only the word group
  "fudge." to be added/deleted.  The fake cursor for the first _or_ second line
  containing "~//" will be erased, but the fake cursor for said line will not be
  redrawn by `mc_helper' unless the lnum data was previously used by
  `mc_update_window_erase' to make a the `same_p` determination.  In other 
words,
  the fake cursor at isue would be `same_p` _without_ using the lnum data, but
  is `!same_p` when we use the lnum data.

  Every good boy | deserves fudge.  Every good boy deserves fudge.  Every good ↩
  boy deserves fudge.| Every good boy deserves fudge.  Every good boy deserves ↩
  fudge.         |
                 |
  ~//            |   <=== This fake cursor IF adding the whole word "boy" with 
redo;
  ~//            |   <=== or, this fake cursor IF deleting the whole word "boy".

. The performance hit is zero when built-in line numbers are active, but perhaps
  mildly detectable when built-in line numbers are turned off.  Although line
  numbers are not being generated on the glass in the latter situation, Emacs 
will
  nevertheless be calling `display_count_lines_visually/logically' at the same
  locations as if the built-in line numbers were turned on.

. When `mc_update_text_area' processes the line containing the fake cursor that
  needs to be redrawn, the line is deemed to be UNCHANGED.  `clear_end_of_line'
  is not triggered and no floating fake cursors are redrawn based thereon.  This
  analysis was helpful in narrowing down the issue to the need for the lnum data
  as described hereinabove.


VIDEOS:

w32:  https://youtu.be/r3BdJVlsAnQ

ns:  https://youtu.be/bc1h8jtbXmw

x11:  https://youtu.be/aCIFhD2Xz5s


SCREENSHOTS:

https://www.lawlist.com/images/22873_17684_light_dark_backgrounds.png


SETUP:

Step 1:  git clone -b master git://git.sv.gnu.org/emacs.git

Step 2:  Due to lack of free time available to dedicate to the development of
         features 22873 / 17684, applicability of the patch to the master branch
         is frozen at 07/14/2019:  ac57c5093829ee09084c562bbbc1c412179be13d

         In the new emacs repository folder, execute a hard reset to 07/14/2019:

         git reset --hard ac57c5093829ee09084c562bbbc1c412179be13d

Step 3:  From within the new emacs folder created in Step 1, apply the patch:

         git apply /path/to/the/patch.diff

Step 4:  ./autogen.sh

Step 5:  ./configure ... [your custom options]

Step 6:  make

Step 7:  make install


USAGE:

- For a minimal working example of built-in fake cursors, type:  M-x mc-test

  ;;; TURN ON FAKE CURSORS (buffer position, cursor-type, cursor color):

  (setq mc-conf '((1 "hbar" "magenta")
                  (2 "bar" "purple")
                  (3 "box" "#00FF00")
                  (4 "hollow" "#0000FF")
                  (5 ("hbar" 3) [1.0 0.0 1.0])
                  (6 ("bar" 3) [0.0 1.0 1.0])
                  (7 "framed" "OrangeRed")))

  ;;; TURN OFF FAKE CURSORS:

  (setq mc-conf nil)

- To try out both the crosshairs feature and the visible fill column indicator
  feature, type:  M-x +-mode

- To try out just the visible fill column indicator feature, type:  M-x fc-mode

- To try out built-in fake cursors with Magnar Sveen's multiple-cursors package,
  that package must be installed.  If the multiple-cursors package is already
  installed, then just (require '+-mode) and `+-mode.el` will redefine a
  few of the multiple-cursors functions and set up a few keyboard shortcuts.
  If the multiple-cursors package by Magnar Sveen is not already installed, then
  here are two easy ways to install that package:

  Type:  M-x mc-install

  OR, evaluate the following snippet:

  (progn
    (require 'package)
    (add-to-list 'package-archives '("melpa" . 
"http://melpa.milkbox.net/packages/";) t)
    (package-initialize)
    (package-refresh-contents)
    (package-install 'multiple-cursors)
    (mc/built-in-cursors))


NOTES:

- Our journey begins at the outset of `update_window' when
  `mc_update_window_dryrun' performs a `!draw_p` simulation to create a new
  cache of fake cursors that are stored in the `w->mc_matrix`.  The cache of
  fake cursors from the previous redisplay is copied to a temporary `mc_matrix`
  under the name of `old_matrix`.  `mc_update_text_area' (used for `!draw_p` /
  `draw_p` situations) calls `mc_draw_glyphs' (writes glyphs to the glass)
  followed by `mc_draw_row' (writes fake cursors to the glass immediately
  thereafter).  At the tail end of `mc_update_window_dryrun',
  `mc_update_window_erase' compares the new `w->mc_matrix` with the `old_matrix`
  to determine which fake cursors are the same -- setting the `same_p` boolean
  struct member accordingly for each applicable fake cursor within the
  `w->mc_matrix`.  All fake cursors in the `old_matrix` that are `!same_p` get
  erased at this juncture.  [Fn 1.]  After the dryrun is complete,
  `update_window' does the real thing -- `draw_p`.  As to the `from_where`
  situations of SKIPPED, POST_CHANGED, UNCHANGED, and SET_CURSOR_TWO,
  `mc_helper' compares the incoming tentative fake cursor with the
  `w->mc_matrix` to see if it has previously been marked as `same_p` -- if it is
  `!same_p`, then the fake cursor is drawn and we `return`.  Fake cursors that
  have a `cursor_type` of MC_LEFT_FRINGE_BITMAP or MC_RIGHT_FRINGE_BITMAP are
  always reset even if they are `same_p`.  As to the `from_where` situations of
  SCRIBE_ONE, SCRIBE_TWO, SCRIBE_THREE, and NOWHERE, `mc_helper' always causes
  fake cursors to be drawn (because new glyphs were written to the glass, or the
  area to the right of the display line was cleared) and we `return`.

. Fn. 1:  Even though writing new glyphs to the glass (SCRIBE_ONE, SCRIBE_TWO,
  and SCRIBE_THREE) would erase fake cursors within those boundaries, it is
  still necessary to erase `!same_p` fake cursors prior thereto.  This is
  because `scrolling_window' occurs before new glyphs are written and
  `rif->scroll_run_hook' may copy one or more rows to other areas of the glass.
  It is not worth the effort to programmatically track both the _copied_ fake
  cursors and the _original_ fake cursors from the previous redisplay cycle,
  whose glyphs may remain where they are in the current redisplay cycle because
  they satisfy the `GLYPH_EQUAL_P' test.

- The rest of our journey takes place wherever `draw_glyphs' would ordinarily be
  called, excluding `update_text_area'.  If features 17684/22873 are active,
  `mc_redraw_row' calls `mc_draw_glyphs' (writes glyphs to the glass) and fake
  cursors are written to the glass immediately thereafter (if the coordinates
  coincide with prerecorded data in the `w->mc_matrix`).  The functions
  containing the aforementioned calls are:
  . `gui_insert_glyphs'
  . `gui_fix_overlapping_area'
  . `draw_row_with_mouse_face'
  . `expose_area'
  . `expose_line'
  . `redraw_overlapped_rows' [which has been incorporated into `update_window']

- As to `mc_scrolling_window', it would appear that there is no tangible
  benefit to rotating the current/prospective cache of fake cursors to compare
  the data before removing the fake cursors.  When scrolling the display, only
  _some_ lines are copied to new locations.  Areas that are not overwritten may
  have fake cursors and those may not necessarily be removed if desired/current
  matrix glyphs are equal.  The test for `GLYPH_EQUAL_P' does not take into
  consideration the existence of a fake cursor, and the glyph (with a fake
  cursor) may not be updated as a result thereof.  As to lines that are not
  copied, portions may be updated and fake cursors would be removed thereby.
  `mc_rotate_matrix' and `mc_reverse_vpos' were removed with patch v. 022.002.

- NS:  As of 09/28/2018 (7946445962372c4255180af45cb7c857f1b0b5fa), the NS port
  no longer does anything useful during `update_window' except mark dirty
  rectangles.  All drawing is now done when the MacOS calls `drawRect', which in
  turn calls `expose_frame'.  This was done to add support for MacOS Mojave ....


TODO:

- When an idle-timer fires and point is at the end of a horizontally scrolled
  line in a narrow window, the temporary horizontal scroll is canceled.  Create
  a minimal working example and file a bug report.

  (progn
    (defun test ()
    (interactive)
      (let ((ov (make-overlay (point) (1+ (point)) nil t t)))
        (overlay-put ov 'face '(:foreground "red"))))
    (global-set-key [f5] 'test)
    (split-window-horizontally)
    (switch-to-buffer (get-buffer-create "foo"))
    (setq bidi-display-reordering nil)
    (setq-local auto-hscroll-mode 'current-line)
    (dotimes (i 80)
      (insert (char-to-string (+ 65 i)))))

- `ns_draw_window_cursor' calls `ns_clip_to_rect', which does _not_ take
  `row->clip` into consideration when drawing cursors with `NSRectFill'.  When
  it comes time to draw glyphs on top of the box/hollow family of cursors,
  `row->clip` is taken into consideration by `get_glyph_string_clip'.  Fake
  cursors can be drawn even though the glyphs cannot, resulting in hollow/box
  family of cursors without text.  The issue can be reproduced with M-x mc-test.
  [A temporary workaround is to disable `row->clip` while drawing the glyphs.]
  https://lists.gnu.org/archive/html/emacs-devel/2019-04/msg00009.html

- Deal with left/right overwritten glyphs in the w32 and X ports of Emacs.

- The current test for `auto_hscroll_mode_p' only looks for `current_line` and
  all five related tests are based upon that assumption, which may not be true.

- Multiple Cursors:  If point is in the middle of a composite character, then
  select a fully composed character so that the fake cursor is visible.

- Implement functionality similar to the Lisp multiple-cursors by Magnar Sveen.

- Follow up with the Emacs team re bug#32177; i.e., (Current line number shifts
  one column to the left.)

- Follow up with the Emacs team re bug#32060; i.e., Horizontal Scrolling
  (Current Line):  Wrong line gets h-scrolled.

- Determine if bug #28936 needs to be fixed and help the Emacs team re same.

- Is there any additional meaningful optimization that can be added to the
  three calls of `mc_pre_scroll_clean'?

- There is a bug affecting an older version of Emacs for the NS port that causes
  partial line flickering when the same characters are grouped together (;;;;;;)
  and MC_GLYPHLESS cursors are above or below -- having the same background 
color
  as the frame; e.g., black on black (used to erase a glyphless cursor).  The
  partial flickering is only noticeable with rapid fire; e.g., holding down the
  right/left arrow key.  When changing the color of the glyphless cursor, the
  issue is not present.  [@lawlist has verified that the X and HPOS coordinates
  are accurate.]

Attachment: 2020_05_02__13_04_51_850.diff
Description: application/diff


reply via email to

[Prev in Thread] Current Thread [Next in Thread]