-
Notifications
You must be signed in to change notification settings - Fork 53
selectionEdgeDestroy: misc cleanups #304
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
selectionEdgeDestroy: misc cleanups #304
Conversation
Just noticed that that bug happens on classic selection mode, not edge. I'll remove that part of the commit message before merging. |
@daltomi A couple questions
diff --git a/src/selection_edge.c b/src/selection_edge.c
index f0cc3c7..94ac4d1 100644
--- a/src/selection_edge.c
+++ b/src/selection_edge.c
@@ -135,6 +135,7 @@ void selectionEdgeDestroy(void)
if (ev.type == DestroyNotify && ev.xdestroywindow.window == pe->wndDraw)
break;
}
+ nanosleep(&(struct timespec){ .tv_nsec = 120 * 1000L * 1000L }, NULL);
XFree(pe->classHint);
} |
* xeventUnmap() did not check the union type before reading from it. * unmap event should always come, no need to sleep/retry 30 times. * remove xeventUnmap() and waitUnmapWindowNotify() entirely. the main logic is short enough to inline directly at the call site. * rather than unmapping the window, and then destroying it, destroy it directly and wait for the DestroyNotify event. * compare window ID against `None` rather than `0`. no functional difference, but using `None` is more idiomatic for X11 things. this commit also fixes a regression introduced in b2d4358. because we pull the event out now, the DestroyNotify event won't stick around in the queue anymore. Fixes: resurrecting-open-source-projects#319
I see, I think I understand why this is happening a bit better now. Although we received a I pushed a new commit adding the delay back. I think it should fix the issue. (I've also added a comment, because it's not obvious why there's a sleep there). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it works well.
Thanks for confirming. Looks like my understanding was correct. I've merged the sleep for now, but I think we'd want to see if there's a more reliable way to detect repaint. I'll open a separate issue for it soon. |
scrot --select
borders caught up in the screenshot #284)None
rather than0
. no functional difference, but usingNone
is more idiomatic for X11 things.