-
Notifications
You must be signed in to change notification settings - Fork 11
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
Use webgl for cursor #44
Conversation
See last batch of comments on #24, also uses code from https://gist.github.com/romgrk/56169975ccd8fa2ffafd134afcdfeedc
breaks things, still very much a WIP
Usually it's easier to review when the PR is split in smaller self-consistent parts. It also makes discussion on it more focused.
I'd also suggest to put in place tests that can be run easily and regularly to check if anything introduces a performance regression. |
Note: this doesn't apply anymore because you've removed the timeout stuff but I'll post anyway: I've tested a bit more, one thing I notice is that you've added 2 timeouts. This is counter-productive. What is happening is the events trigger a first timer, and when this timer is called it renders some stuff plus triggers a second timer, which then finally renders more stuff. You're adding latency here, not removing it. You need to
I've also noticed that everytime, the text is redrawn before the cursor is moved. I'm not extacly sure which event you're using to move the cursor, is it the uivonim-position one or the ui-event grid_cursor_goto? In any case, the cursor update should happen at the same time as the text update. You can see this for yourself in the profiles, see example below. |
Yeah . . . I got a bit carried away with this PR haha, was originally gonna focus on doing the layout thing, but then started doing cursor stuff . . . my bad.
TBH I don't even remember adding that second webgl timeout, or why I did it. Just removing that extra timeout should be enough to make things work though, right? Or will it require more work than that?
Yeah, I've noticed the cursor lagging behind, just have yet to look into exactly why it's happening. The
Good idea, but first I need to figure out what those should be, what exactly they'll measure, how they'll work, etc., which I've yet to do. |
This isn't happening anymore it seems; I think it was caused by the double timeout, since the second timeout was delaying the webgl redraw, and thus delaying the cursor redraw. |
Do you really need to draw them with webgl? Does drawing with the |
I suppose using webgl is not strictly necessary, but I don’t see a real reason not to use it. There’s already code in the codebase that makes using it fairly straightforward, and if I can simply tweak that to also draw the cursor then I see no real reason not to, unless of course the tweaks slow down rendering, but I haven’t noticed any slowdowns so far. I’m not sure how much cost (if any) there is with using the 2D context of the canvas API vs. WebGL, but doing that would probably add more complexity and require more code. Regardless, I’ve figured out the line drawing for the most part; for some reason didn’t think to just make the rectangle that’s already being drawn smaller 😅 Still a bit of work to do, for example the other half of the cursor cell’s background is the wrong color atm, but overall it shouldn’t be as hard as I thought. |
also add some code back that I removed, use `moveCursor` instead of `windows.webgl.updateCursorPosition` directly in `grid_cursor_goto`, and maybe some other things
Interestingly, it appears the cursor lagging behind is inconsistent across setups. On Sway, for example, the lag is fairly obvious when holding |
Should fix #42, also removes code from
src/core/input.ts
for remapping input keys.