search

lem-project / lem

Common Lisp editor/IDE with high expansibility
http://lem-project.github.io/
MIT License
2.4k stars 178 forks source link

SDL2: display-width and display-height calls behave differently depending on renderer-target #911

Closed cxxxr closed 11 months ago

resttime commented 1 year ago

Been investigating and able to reproduce, it's not affecting just find-file. All floating windows with a height of 1 will also break such as C-x b for select-buffer. C-l is mapped onto the command recenter but handling is incorrect causing stuff like the floating window's height to be an invalid number.

resttime commented 1 year ago

https://github.com/lem-project/lem/blob/c70e0266196912944725cf120604a34263fb5b96/src/ext/prompt-window.lisp#L173

During execution of recenter the function (display-height) evaluates to 1. This causes the form (alexandria:clamp height +min-height+ (- (display-height) 2)) to evaluate to (alexandria:clamp height +min-height+ -1) clamping the value to -1 instead of +min-height+ as intended.

resttime commented 1 year ago

I believe I found the overall issue in redraw-display called in src/window.lisp

Turns out in the implementation of the SDL2 backend the render target changes between operations. The evaluation of recenter on C-l in particular clears a list of windows.

https://github.com/lem-project/lem/blob/c70e0266196912944725cf120604a34263fb5b96/frontends/sdl2/main.lisp#L600-L603

This leaves the render target set to the last window cleared. This will be queried in (display-height) which returns a value of 1. In the subsequent calculation of the window rectangle size, this causes the height to be clamped down to -1 in an arithmetic operation.

I believe there's an expectation the render target set by *display* when calling (display-height), not the last window that the render target of set to. To verify in redraw-display, place the call to update-floating-prompt-window on L1384 right after (lem-if:will-update-display (implementation)) on L1372 and the issue is gone since lem-if:will-update-display sets the render target to *display*.

https://github.com/lem-project/lem/blob/c70e0266196912944725cf120604a34263fb5b96/src/window.lisp#L1371-L1388

Another way to resolve the issue is redefining render-clear in frontends/sdl2/main.lisp to preserve the render target after the operation. Maybe a WITH-RENDER-TARGET macro would fit here. However this implies changes to the overall design to preserve the render target in all operations and I don't know if that's intended. 

(defmethod render-clear ((view view))
  (let ((current-render-target (sdl2:get-render-target (current-renderer))))
    (sdl2:set-render-target (current-renderer) (view-texture view))
    (set-render-color *display* (display-background-color *display*))
    (sdl2:render-clear (current-renderer))
    (sdl2:set-render-target (current-renderer) current-render-target)))
cxxxr commented 12 months ago

@resttime Sorry for the late reply. Your detailed explanation was very helpful. Thank you very much.

Sasanidas commented 11 months ago

Seems like from this comment https://github.com/lem-project/lem/issues/1163#issuecomment-1830477116, this issue should also be solved. I'll close it now, feel free to re-open it if that's not the case :+1: