Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround for child-frame's bad behavior under macOS. #21

Closed
jiegec opened this issue Dec 27, 2017 · 18 comments
Closed

Workaround for child-frame's bad behavior under macOS. #21

jiegec opened this issue Dec 27, 2017 · 18 comments

Comments

@jiegec
Copy link
Contributor

jiegec commented Dec 27, 2017

I am using Emacs 26 pretest (i.e. 26.0.90) and lsp-ui-doc.el is really amazing.
However, similar to what I have run across when debugging pyim's child-frame support for choosing Chinese characters, the child frame might not show any more after a make-frame-invisible call. Tested make-frame-visible and set-frame-parameter with visibility, but to no avail. I think this should be a BUG somewhere in Emacs itself, but I hadn't send a bug report to bug-gnu-emacs. My temporary fix for lsp-ui:

diff --git a/lsp-ui-doc.el b/lsp-ui-doc.el
index 358a051..a8c4fa6 100644
--- a/lsp-ui-doc.el
+++ b/lsp-ui-doc.el
@@ -224,7 +224,11 @@ BUFFER is the buffer where the request has been made."
   (when (lsp-ui-doc--get-frame)
     (lsp-ui-doc--with-buffer
      (erase-buffer))
-    (make-frame-invisible (lsp-ui-doc--get-frame))))
+    (lsp-ui-doc--render-buffer "Hover on symbols to see document." "")
+    (lsp-ui-doc--move-frame (lsp-ui-doc--get-frame))
+    (set-frame-parameter (lsp-ui-doc--get-frame) 'alpha '(0 . 0))
+    ;; (make-frame-invisible (lsp-ui-doc--get-frame))
+    ))
 
 (defun lsp-ui-doc--buffer-width ()
   "Calcul the max width of the buffer."
@@ -312,6 +316,7 @@ SYMBOL STRING."
     (unless (frame-live-p (lsp-ui-doc--get-frame))
       (lsp-ui-doc--set-frame (lsp-ui-doc--make-frame)))
     (lsp-ui-doc--move-frame (lsp-ui-doc--get-frame))
+    (set-frame-parameter (lsp-ui-doc--get-frame) 'alpha '(100 . 100))
     (unless (frame-visible-p (lsp-ui-doc--get-frame))
       (make-frame-visible (lsp-ui-doc--get-frame)))))

The problem of this workaround is that, redisplay is too often called, making screen constantly flashing. To refactor this, we need to maintain a visibility state ourselves.

I am writing a PoC for this BUG and will send it to bug-gnu-emacs some time later.

Edit: might be related to calling make-frame-visible from a thread other than main thread, which is not explicitly specified in Emacs docs.

Edit: can't reproduce this in a snippet somehow.

Edit: some log shown in terminal when testing snippet but not the original code:

CoreAnimation: warning, deleted thread with uncommitted CATransaction; set CA_DEBUG_TRANSACTIONS=1 in environment to log backtraces, or set CA_ASSERT_MAIN_THREAD_TRANSACTIONS=1 to abort when an implicit transaction isn't created on a main thread.

Edit: I am mad about this now.

@sebastiencs
Copy link
Member

sebastiencs commented Dec 27, 2017

@jiegec I don't have a mac sorry.
How does your snippet look like ? Maybe I can help you to narrow it

@jiegec
Copy link
Contributor Author

jiegec commented Dec 27, 2017

I selectively C-M-x on some of the forms below, but can't reproduce the bug.

(defvar test-child-frame)

(let* ((params `((visibility . nil)
                 (minibuffer . ,(minibuffer-window))
                 (internal-border-width . 10)
                 (vertical-scroll-bars . nil)
                 (horizontal-scroll-bars . nil)
                 (left-fringe . 0)
                 (right-fringe . 0)
                 (menu-bar-lines . 0)
                 (tool-bar-lines . 0)
                 (min-width  . 0)
                 (width  . 0)
                 (min-height  . 0)
                 (height  . 0)
                 (line-spacing . 0)
                 (unsplittable . t)
                 (undecorated . t)
                 (left . -1)
                 (top . -1)
                 (no-accept-focus . t)
                 (no-special-glyphs . t)
                 (cursor-type . nil)
                 (default-minibuffer-frame . ,(selected-frame))))
       (child-frame (window-frame
                     (display-buffer-in-child-frame
                      (get-buffer-create "*test*")
                      `((child-frame-parameters . ,params))))))
  (setq test-child-frame child-frame))
(set-frame-parameter test-child-frame 'height 50)
(set-frame-parameter test-child-frame 'width 50)
(make-frame-visible test-child-frame)
(set-frame-parameter test-child-frame 'alpha '(85 . 50))
(delete-frame test-child-frame)

(make-thread (lambda ()
               (make-frame-invisible test-child-frame)))

(make-frame-visible test-child-frame)

@jiegec
Copy link
Contributor Author

jiegec commented Dec 29, 2017

What the hell:
image

@jiegec
Copy link
Contributor Author

jiegec commented Dec 29, 2017

One more snapshot:
image

@jiegec
Copy link
Contributor Author

jiegec commented Dec 29, 2017

The frame parameter after disappearing:

((tool-bar-position . top)
 (parent-id)
 (explicit-name)
 (display . "localhost")
 (visibility . t)
 (icon-name)
 (window-id . "20")
 (bottom-divider-width . 0)
 (right-divider-width . 0)
 (top . 372)
 (left + -694)
 (buried-buffer-list)
 (buffer-list #<buffer *lsp-ui-doc-13*>)
 (unsplittable . t)
 (modeline . t)
 (width . 53)
 (height . 24)
 (name . "*lsp-ui-doc-13*")
 (client . nowait)
 (cursor-color . "#839496")
 (background-mode . dark)
 (display-type . color)
 (default-minibuffer-frame . #<frame  *Minibuf-1* 0x1090b2d30>)
 (no-other-frame . t)
 (mouse-wheel-frame)
 (fullscreen)
 (alpha)
 (scroll-bar-height . 0)
 (scroll-bar-width . 0)
 (cursor-type)
 (auto-lower)
 (auto-raise)
 (icon-type)
 (title)
 (buffer-predicate)
 (tool-bar-lines . 0)
 (menu-bar-lines . 0)
 (no-accept-focus . t)
 (no-focus-on-map)
 (z-group)
 (parent-frame . #<frame  *Minibuf-1* 0x1090b2d30>)
 (ns-transparent-titlebar . unbound)
 (ns-appearance . unbound)
 (undecorated . t)
 (min-height . 0)
 (min-width . 0)
 (no-special-glyphs . t)
 (right-fringe . 0)
 (left-fringe . 0)
 (line-spacing . 0)
 (background-color . "#031A25")
 (foreground-color . "#839496")
 (horizontal-scroll-bars)
 (vertical-scroll-bars)
 (internal-border-width . 10)
 (border-width . 0)
 (font .
       "-*-Inconsolata for Powerline-normal-normal-normal-*-22-*-*-*-m-0-iso10646-1")
 (fontsize . 0)
 (font-backend mac-ct)
 (minibuffer . #<window 35 on  *Minibuf-0*>))

@jiegec
Copy link
Contributor Author

jiegec commented Dec 29, 2017

The top and left parameters somehow get corrupted, and setting them to zero has no effect.

@sebastiencs
Copy link
Member

sebastiencs commented Dec 29, 2017

A child frame can't be outside of it's parent frame, otherwise it's not a child frame but a "normal" frame.
The bug appears only after a call to make-frame-invisible ?
Can you execute this function:

(defun test-child-frame-visible ()
  (interactive)
  (let* ((window (display-buffer-in-child-frame (get-buffer-create "test")
                                                '((child-frame-parameters . ((left . -10)
                                                                             (no-accept-focus . t)
                                                                             (width  . 50)
                                                                             (height  . 10)
                                                                             (vertical-scroll-bars . nil)
                                                                             (horizontal-scroll-bars . nil)
                                                                             (left-fringe . 0)
                                                                             (right-fringe . 0)
                                                                             (menu-bar-lines . 0)
                                                                             (tool-bar-lines . 0)
                                                                             (unsplittable . t)
                                                                             (undecorated . t)
                                                                             (background-color . "red")
                                                                             (top . 10)
                                                                             (visibility . nil)
                                                                             (mouse-wheel-frame . nil)
                                                                             (no-other-frame . t))))))
         (frame (window-frame window))
         (count 0))
    (while (< count 5)
      (if (oddp count)
          (make-frame-invisible frame)
        (make-frame-visible frame))
      (sit-for 1)
      (setq count (1+ count)))
    (delete-frame frame)))

The child frame should pop on the top right of your actual frame (still inside it), be invisible/visible every second at the exact same place.

@sebastiencs
Copy link
Member

Also, does emacs on mac based on GTK or does it uses another toolkit ?

@fuxialexander
Copy link

@sebastiencs no it's not based on GTK. It's using Cocoa. I think the implementation detail is somehow different from the GTK one. See GNU manual: NS builds do not clip child frames at the parent frame’s edges, allowing them to be positioned so they do not obscure the parent frame while still being visible themselves.

@fuxialexander
Copy link

image

It seems not determined by make-frame-invisible as I do see it disappear and appear again periodically. The positioning is bad though.

sebastiencs added a commit that referenced this issue Dec 29, 2017
On OS X, child frame can be outside its parent frame.
Coordinates are relative to the whole screen and not the emacs frame.
By using positive values, we ensure to place the child frame at the
right position.
@sebastiencs
Copy link
Member

sebastiencs commented Dec 29, 2017

@fuxialexander Thank for you feedback
IIRC there are 2 bugs ? The wrong position and the child frame staying invisible ?
Can you test if the last commit d050b4f fix the position ?

@jiegec
Copy link
Contributor Author

jiegec commented Dec 30, 2017

Still doesn't work here. The small snippet you proposed works well.

@jiegec
Copy link
Contributor Author

jiegec commented Dec 31, 2017

OMG I know the problem: if set the position of a invisible frame, the frame gets corrupted.
Should be fixed in pr #30.

Thanks for https://emacs-china.org/t/topic/4662/18

MaskRay pushed a commit that referenced this issue Jan 1, 2018
sebastiencs added a commit that referenced this issue Jan 2, 2018
If we move/resize the frame after making it visible, it makes a
flickering effect.
So we do it only on systems where the bug is present (os x)
@sebastiencs
Copy link
Member

sebastiencs commented Jan 2, 2018

If we move/resize the frame after making it visible, it makes a
flickering effect.
So we do it only on systems where the bug is present (os x)

Can you report the bug to bug-gnu-emacs ?

(let* ((window (display-buffer-in-child-frame
                (get-buffer-create "test")
                '((child-frame-parameters . ((left . 10)
                                             (no-accept-focus . t)
                                             (width  . 50)
                                             (height  . 10)
                                             (vertical-scroll-bars . nil)
                                             (horizontal-scroll-bars . nil)
                                             (left-fringe . 0)
                                             (right-fringe . 0)
                                             (menu-bar-lines . 0)
                                             (tool-bar-lines . 0)
                                             (unsplittable . t)
                                             (undecorated . t)
                                             (background-color . "red")
                                             (top . 10)
                                             (visibility . nil)
                                             (mouse-wheel-frame . nil)
                                             (no-other-frame . t))))))
       (frame (window-frame window))
       (count 0))
  (while (< count 5)
    (if (oddp count)
        (make-frame-invisible frame)
      (set-frame-parameter frame 'left (+ 10 (* count 100)))
      (set-frame-parameter frame 'top (+ 10 (* count 100)))
      (make-frame-visible frame))
    (sit-for 1)
    (setq count (1+ count)))
  (delete-frame frame))

@jiegec
Copy link
Contributor Author

jiegec commented Jan 2, 2018

Thanks, with this snippet, I can always reproduce the bug.

Just reported it to bug-gnu-emacs as BUG #29953. Link: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29953

@jiegec
Copy link
Contributor Author

jiegec commented Jan 4, 2018

With Alan Third's patch, I can confirm that his patch fixes it for me.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29953#8

@jiegec jiegec closed this as completed Jan 4, 2018
@sebastiencs
Copy link
Member

@jiegec So can I revert 651f935 and b6523db ?

@jiegec
Copy link
Contributor Author

jiegec commented Jan 10, 2018

Yes, those are no longer needed with a upstream Emacs. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants