Refactor Demos #212
Refactor Demos #212
Conversation
bmansurov
force-pushed
the
issue-186
branch
2 times, most recently
from
4980b41 to
064295e
Compare
|
I think the PR is ready for a review. |
dkochmanski
left a comment
dkochmanski
left a comment
There was a problem hiding this comment.
Please split this pull request into more commits, that's too big as it is for me to review, i.e
Commit:
- Make menu font larger if a larger font exists;
- Don't block the main menu rendering;
- Simplify demo registration code;
- Get rid of modelines in some files as modern editors are smart enough;
Commit:
- Allow opening multiple instances of individual demos;
Commit:
- Move demos to spearate files;
Commit:
- Disable some demos and test code until they are fixed.
- Add standalone demos to the main demo interface;
Commit:
- Fix issues where nothing shows up in some demos;
Commit:
- Fix the related issues in the manual;
I've also noticed, that clx crashes my lisp instance when I close a window, but I've verified that this issue is already present in master. Is this the same for you?
|
@dkochmanski thanks for the review. I'll make the changes.
No, I haven't seen this bug. I'm using SBCL. What Lisp are you using? What window manager? |
|
thanks for conformiation. I've actually tried to debug it and it led me to sbcl internals where it waits for fd being available (using posix subsystem). Going any deeper was unfeasible. I've formulated the following hypothesis: when we don't handle "close" event in demos, wayland (I'm using on this computer x11 compatibility layer) closes the fd, and when sbcl tries to fiddle with it, then the operating system kills sbcl for accessing unavailable resource. It does not happen on my other computer where I'm using "normal" x11. |
Create an empty demo entry point with only one menu item 'Quit'. This entry point will be used to replace the existing entry point when all demos have been ported in subsequent patches. Issue: sharplispers#186
Even before making these changes some disks had hollow middles and the patch didn't attempt to fix that. The issue will be fixed in a later patch. Issue: sharplispers#186
- Change comments to docstrings - Add delay between rectangle draws to simulate moving - Rename variables to better reflect their purpose - Remove redundant code - Reorder the order of arguments WIDTH and HEIGHT to mimick the order of xlib:draw-rectangle. Issue: sharplispers#186
The full-window-state function was using xlib:with-state to query the window's geometry. This caused the animation to start from an incorrect position (e.g., (5, 68) instead of (100, 100)). The window shows up at position (100, 100) briefly and is abruptly moved to (5, 68) before the animation is started. This discrepancy needs to be investigated and fixed in a later patch. Since we already know the window dimensions and position from the function parameters, we can use those values directly instead of querying them back from X11. This simplifies the code and fixes the positioning bug. Issue: sharplispers#186
…emos Now that the demos have been moved into separate files and are using the new demo registration code, it's safe to replace the old package xlib-demo/demos with the new one xlib-demo/demos-new. Issue: sharplispers#186
- Use `with-x11-context` - Get rid of global and other redundant variables Issue: sharplispers#186
When a clock window is destroyed, due to the asynchronous nature of calls to X11, the rendering code will keep trying to draw objects to the screen. But since the window is already destroyed, such calls result in errors like this: > DRAWABLE-ERROR in current request Code 14.0 [GetGeometry] ID #x3C00002 The patch fixes the issue by ignoring such errors. It also stops future rendering once it detects that the window has been destroyed. Issue: sharplispers#186
When a bouncing balls window is destroyed, due to the asynchronous nature of calls to X11, the rendering code will keep trying to draw objects to the screen. But since the window is already destroyed, such calls result in errors like this: > Asynchronous DRAWABLE-ERROR in request 162 (last request was 164) Code > 63.0 [CopyPlane] ID #x3C00002 The patch fixes the issue by ignoring such errors. It also stops future rendering once it detects that the window has been destroyed. The window name has also been fixed. Issue: sharplispers#186
|
@dkochmanski I've updated the pull request and addressed the issues you brought up. I tried to make each patch as minimal and self-containing as possible. I also tried to keep the original code unchanged as much as possible for easy review. I'd appreciate it if you could review it. Thank you.
I'll keep this in mind and create a bug report once I set up my computer to work with the x11 compatibility layer inside Wayland. For now, I'd like to focus on improving CLX for pure X11. |
2 participants
Here you can see how the demos look after the refactor: https://www.youtube.com/watch?v=PPpZDB61oxs
Issue: #186