devdraw: rewrite the cocoa screen #194
Conversation
|
This should fix #185. I think the code is much more readable in this way, but this is a subjective matter. I also reworked the cursor change code, which I borrowed some from 9front's drawterm. The big arrow finally works.
Since I dropped most of the code, please let me know if there is any feature that is missing in this code.
|
|
Neat!. Dumping some observations:
I'll test some more to see what else I find. |
These should be fixed. |
|
FWIW, I can't compile this on macOS 10.13: Not that it matters because this explicitly targets 10.14 and up. |
|
Now it should be usable. The new revision uses CALayer of the view, and no longer uses drawRect. sample(1) shows that three parts dominate, read time (maybe from pipe), CGContext draw time (the offscreen part within the _flushmemscreen), and the CALayer draw time (push to screen). I guess the last two should be improvable. |
|
I have been benchmarking what happens when you run pump a lot of text into an Acme window. The use case is accidentally opening / catting a large file. In this benchmark, the latest version here is slower than #191. This is my silly benchmark: https://gist.github.com/jacobvosmaer/2b70d04833219b9bb3c9ae11608ae7e6 I wonder if CALayer is working against us because it always does a full screen redraw, while devdraw clients may often just invalidate small portions of the screen. @jxy how are you benchmarking performance? |
|
I'm using sample(1) while doing the cat thing. When I get around, I'm going to change this to drawInContext later, and eliminate the additional offscreen drawing. |
|
Cool. I started with I have no complaints about the current performance but I thought it was worth mentioning the difference. Considering that macOS 10.14 is still broken on plan9port master, you could consider leaving performance for a later PR. I think it's super awesome that you did a clean rewrite. I think it's a better idea than what I did in #191; this lets us sunset the old Cocoa. |
|
I found another bug. When I start Acme on a low-DPI external monitor, and then disconnect the monitor, the text rendering does not update to hi-DPI (Retina). I end up with tiny letters until I restart Acme. |
|
Yes. There is one more event to process. At the mean time, you can try Cmd-r. |
|
I found something else. When I do a three finger tap, I used to get a middle click. E.g. by three-finger-tapping 'Put' in a dirty Acme window I would save the window. In this branch that is not currently working. Nothing happens when I do the three finger tap. |
|
Another bug: entering Latin non-ASCII characters with Alt-key combinations no longer works. In my local environment I can type |
|
Thanks @jacobvosmaer. These should be easy to fix. I'm a little confused about the thread safety now. Apparently On the other hand, it seems that the Tresize event is never received. Can somebody shed some light about the resize event in devdraw? |
|
I've updated the commit using MetalKit. @jacobvosmaer I'll look into the missing features later. |
jxy
force-pushed
the
cocoa-mojave
branch
2 times, most recently
from
b3a00d5
to
7c9e176
Compare
|
I added most of the existing features, except for the drag and drop. Does anybody actually use the drag and drop? Please test the external low DPI monitor support. It now allows touch events to simulate mouse clicks: two finger tap for the right mouse button; three finger tap for the middle mouse button; four finger tap for the 2-1 chord. It also includes the control click #119 simulating left button (mainly for 2-1 chord). It also supports macOS input sources including the basic dead keys and the advanced CJK input methods (I only tested the Chinese and Japanese ones). There is a different implementation in #121. This uses artificial keystrokes to show IME's input buffer in any application. You can test and see the behavior by pressing |
|
Changed to CAMetalLayer. This fixes the graphical glitches appearing in the MTKView based renderer. Things are much smoother and responsive now. |
|
Oddly, I'm having trouble compiling at dcfb631. |
|
@jacobvosmaer you updated to 10.14.1? You need Xcode, and set Apple seems to botched their system header files. |
|
Yes, 10.14.1. That CC9 override works, thanks. Will try your latest version now! |
|
When I do a two-finger tap (i.e. button 3) to search for a word in an Acme window, I get two searches in a row sometimes. It seems to register two events instead of one. |
When you move the window, the cursor is supposed to end up in the "button". When you open a new window by right click a file/dir or via plumber, the cursor is supposed to end up at the beginning of the window, or position
OK. I added it back, see fcc76fb.
This is a serious problem. Unfortunately I cannot reproduce it. I only have my 2017 13-inch MacBook pro and I have not seen such an issue. Do you always get black screen? Can you change the line
I got used to two finger pinch to toggle full screen. Cmd-F is back for people who like it. |
Thank you. When I'm at my desk I use an external keyboard and don't have any trackpad to pinch. |
You are correct - I was wrong about the button vs the #0 position. What I'm seeing then is that the cursor is way too far down vertically but probably aligned correctly horizontally.
|
|
For the black full-screen, I see it on both my main laptop screen and my external monitor. Here's a log. Various things happening before I typed Cmd-F: I waited until 00:35:00 and typed Cmd-F: The black screen appeared. I clicked, knowing the cursor was in the white part (not in an acme window): Nothing happened. Then I moved the cursor into a real acme window and clicked again: At this point the screen was no longer black. |
|
This fixes the cursor problem, which only occurs on an external monitor: I just put the line from cocoa-screen.m back in. :-) |
|
At this point I think the black screen on full-screen mode is the only problem. It happens on the main laptop as well as the external monitor. I have: I will try updating to 10.14.1. |
From the documentation:
Naively, I would have thought that the “screen with the active window” sounds like what we would want, but maybe we’re not working in its coordinate system. |
|
10.14.1 did not fix the black screen on full-size. In addition to the black screen, can we go back to the system arrow cursor instead of the big Plan 9 arrow? The Plan 9 arrow does not look right on retina screens (the white outline has weird glitches). |
|
I tried to debug the low-DPI monitor issue, where the scaling factor is wrong after disconnecting my monitor. This time I got a crash... |
|
I can't reproduce the black screen problem on my machines sadly. Unrelated: I still need to override |
|
@jacobvosmaer I installed Xcode from the App Store on a fresh Mojave system last night and I've been running ./INSTALL and mk install with no CC9 override. I'm not sure what's wrong on your system but maybe updating Xcode would fix it? |
|
I will take another look at all this tonight and merge this PR. It's working well enough and any bugs are limited to Mojave (and the alternative is nothing working at all). I'd still like to go back to the system cursor instead of the big cursor. If someone can figure out why the white lines in the cursor are not white (the screen pixels below them display inverted instead!) I'd be happy to try the big cursor again at that point. |
|
|
Thanks for the fixes. I'll wait for your next patch set to do the full push.
|
|
FWIW, I agree that the locking is suspect here. I applied this patch locally, to make programs build with ThreadSanitizer: After rebuilding all the libraries and devdraw, I got consistent reports about races between the graphics thread and the serving thread. I applied this diff and they went away: Please apply those changes to your copy. Thanks. |
|
Add a new macOS cocoa screen, cocoa-screen-metal.m. Rewrite the macOS cocoa drawing code to use the builtin runloop, and use Metal to push pixels with CAMetalLayer. Remove all of the deprecated code, and simplify some of the logic. Modify mkwsysrules.sh such that the new code is used only when the system version is equal or higher than 10.14. Allow touch events to simulate mouse clicks: three finger tap for the middle mouse button; four finger tap for the 2-1 chord. Support Tresize. Scale 16x16 Cursor up to 32x32 with an EPX algorithm. Support macOS input sources including the basic dead keys and the advanced CJK input methods. Increase the communication buffers in cocoa-srv.c to allow more input, especially for long sentences prepared by the macOS input souces.
|
Please check the new patch 7bded71 Completed the For the fullscreen black issue, @rsc, can you try changing |
|
Regarding the default clang, I have the same versions and xcode-select prints the correct path. The only thing I can see that's different when invoking clang directly or xcrun with sdk parameter is that the framework directory is different. I wonder if |
|
I wonder if the CC9 issue is somehow the same root cause as golang/go#26073. It's like recent Xcodes have a bug where they are just out of sync with themselves. |
|
I merged the latest commit. Thanks for all the work on this! Re fullscreen black, changing NSViewLayerContentsRedrawOnSetNeedsDisplay did not help. The others all drew garbage during the resize and then went to black when it was done. I left it alone. For now it's OK to just click for redraw after the full-screen switch. The pixel-art scaling algorithm was interesting, but it drove me nuts how the point of the arrow got rounded. I prefer the sharpness of plain upscaling-by-doubling to the mushiness of the pixel-art output. I changed the upscaling to use doubling instead. But I also added support to libdraw to allow applications to supply their own 32x32 cursors, and I added a hand-edited 32x32 bigarrow that looks nice. I also changed INSTALL to write a CC9 setting to $PLAN9/config saying xcrun --sdk macosx clang. |
|
Good. For what it's worth, I reinstalled the system from option-command-r and directly calling clang works now. I guess Apple probably served a buggy Xcode command-line tools at the beginning of the 10.14.1 release. |
Add a new macOS cocoa screen, cocoa-screen-metal.m.
Rewrite the macOS cocoa drawing code to use the builtin runloop,
and use Metal to push pixels with CAMetalLayer.
Remove all of the deprecated code, and simplify some of the logic.
Modify mkwsysrules.sh such that the new code is used only when
the system version is equal or higher than 10.14.
Allow touch events to simulate mouse clicks:
three finger tap for the middle mouse button;
four finger tap for the 2-1 chord.
Support macOS input sources including the basic dead keys and the
advanced CJK input methods.
Increase the communication buffers in cocoa-srv.c to allow more
input, especially for long sentences prepared by the macOS input
souces.