selectionEdgeDestroy: misc cleanups

[N-R-K]

• 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. (this may help with: https://github.com/resurrecting-open-source-projects/scrot/issues/284)
• compare window ID against None rather than 0. no functional difference, but using None is more idiomatic for X11 things.

[N-R-K]

(this may help with: https://github.com/resurrecting-open-source-projects/scrot/issues/284)

Just noticed that that bug happens on classic selection mode, not edge. I'll remove that part of the commit message before merging.

[daltomi]

This PR breaks what you want to avoid. That the selection line does not appear in the capture.

scrot --select=hide --line mode=edge,width=4,color=red

[N-R-K]

This PR breaks what you want to avoid. That the selection line does not appear in the capture.

Weird, it's working fine on my end. The selection line doesn't appear on the shot:

[N-R-K]

@daltomi A couple questions

1. Are you running a compositor?
2. Does the follow patch make any difference for you?

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);
     }

[daltomi]

Yes with composer, but it also fails without composer: if you select on a window that plays video, the drag is shown:

The master branch works fine. (I originally implemented the delay to avoid these issues, answering your second question)

[N-R-K]

I originally implemented the delay to avoid these issues, answering your second question

I see, I think I understand why this is happening a bit better now. Although we received a DestroyNotify event, the frame still might not have been updated. Or a compositor might be buffering frames.

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).

[N-R-K]

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.