<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
</span>[...]<br>
<span class="">> + else if (reply->major_version < major || reply->minor_version < minor)<br>
> + {<br>
> + xf86DrvMsg(scrnIndex,<br>
> + X_ERROR,<br>
> + "Host X server doesn't support RandR %d.%d, needed for Option \"Output\" usage.\n",<br>
> + major, minor);<br>
<br>
</span>Why is 4.0 not better than 1.1? Shouldn't this check be replaced with the following?<br>
<br>
if (reply->major_version < major ||<br>
(reply->major_version == major && reply->minor_version < minor))<br></blockquote><div>You're absolutely right! Fixed here. Thank you!<br> <span class=""></span><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="">> +static void<br>
> +_NestedClientSetWMClass(NestedClientPrivatePtr pPriv,<br>
> + const char* wm_class)<br>
> +{<br>
> + size_t class_len = strlen(wm_class) + 1;<br>
> + char *class_hint = malloc(class_len);<br>
> +<br>
> + if (class_hint)<br>
> + {<br>
> + strcpy(class_hint, wm_class);<br>
> + xcb_change_property(pPriv->conn,<br>
> + XCB_PROP_MODE_REPLACE,<br>
> + pPriv->window,<br>
> + XCB_ATOM_WM_CLASS,<br>
> + XCB_ATOM_STRING,<br>
> + 8,<br>
> + class_len,<br>
> + class_hint);<br>
> + free(class_hint);<br>
> + }<br>
<br>
</span>Why is this strcpy needed?<br></blockquote><div>I've copied over this piece of code from Xephyr. In that context, class_hint stores a concatenation bewteen two strings, so that strcpy is needed, but here the WM_CLASS string is much more simple, so that strcpy is not needed. I'll remove it. Thanks!<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +}<br>
> +<br>
[...]<br>
<span class="">> +static void<br>
> +_NestedClientCreateWindow(NestedClientPrivatePtr pPriv)<br>
> +{<br>
> + xcb_size_hints_t sizeHints;<br>
> +<br>
> + sizeHints.flags = XCB_ICCCM_SIZE_HINT_P_POSITION<br>
> + | XCB_ICCCM_SIZE_HINT_P_SIZE<br>
> + | XCB_ICCCM_SIZE_HINT_P_MIN_SIZE<br>
> + | XCB_ICCCM_SIZE_HINT_P_MAX_SIZE;<br>
> + sizeHints.min_width = pPriv->width;<br>
> + sizeHints.max_width = pPriv->width;<br>
> + sizeHints.min_height = pPriv->height;<br>
> + sizeHints.max_height = pPriv->height;<br>
> +<br>
> + pPriv->window = xcb_generate_id(pPriv->conn);<br>
> + pPriv->img = NULL;<br>
> +<br>
> + xcb_create_window(pPriv->conn,<br>
> + XCB_COPY_FROM_PARENT,<br>
> + pPriv->window,<br>
> + pPriv->rootWindow,<br>
> + 0, 0, 100, 100, /* will resize */<br>
<br>
</span>Why not creating it at the correct size and position and skip at least the first<br>
of the following two xcb_configure_window()?</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> (Although I don't understand the<br>
second one either)<br></blockquote><div>TBH, for the window size case, I don't know either. It's just the way it's done in Xephyr, whose code I've copied here. I'll drop the first xcb_configure_window() and wait for any complaints. Than you!<br><br></div><div>For the position case, however, there's a reason: setting it after xcb_map_window() call is the only way I've found to prevent modern WMs from "ignoring" my explicit positioning and putting nested Xorg window where it wants. I'll comment that code for clarity.<br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="">> +void<br>
> +NestedClientUpdateScreen(NestedClientPrivatePtr pPriv,<br>
> + int16_t x1, int16_t y1,<br>
> + int16_t x2, int16_t y2)<br>
> +{<br>
> + if (pPriv->usingShm)<br>
> + xcb_image_shm_put(pPriv->conn, pPriv->window,<br>
> + pPriv->gc, pPriv->img,<br>
> + pPriv->shminfo,<br>
> + x1, y1, x1, y1, x2 - x1, y2 - y1, FALSE);<br>
> + else<br>
> + xcb_image_put(pPriv->conn, pPriv->window, pPriv->gc,<br>
> + pPriv->img, x1, y1, 0);<br>
> +<br>
> + xcb_aux_sync(pPriv->conn);<br>
> +}<br>
<br>
</span>Does this really need a sync? Wouldn't a flush be enough? Or is the sync only<br>
needed in the usingShm case so that the data isn't modified too early?<br></blockquote><div>I'll replace it with xcb_flush() and see if anyone complains. I've also put a xcb_flush() right after window creation, just in case. Thank you!<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5"><br>
> +static inline void<br>
> +_NestedClientProcessExpose(NestedClientPrivatePtr pPriv,<br>
> + xcb_generic_event_t *ev)<br>
> +{<br>
> + xcb_expose_event_t *xev = (xcb_expose_event_t *)ev;<br>
> + NestedClientUpdateScreen(pPriv,<br>
> + xev->x,<br>
> + xev->y,<br>
> + xev->x + xev->width,<br>
> + xev->y + xev->height);<br>
> +}<br>
> +<br>
> +static inline void<br>
> +_NestedClientProcessClientMessage(NestedClientPrivatePtr pPriv,<br>
> + xcb_generic_event_t *ev)<br>
> +{<br>
> + xcb_client_message_event_t *cmev = (xcb_client_message_event_t *)ev;<br>
> +<br>
> + if (cmev->data.data32[0] == atom_WM_DELETE_WINDOW)<br>
> + {<br>
> + /* XXX: Is there a better way to do this? */<br>
> + xf86DrvMsg(pPriv->scrnIndex,<br>
> + X_INFO,<br>
> + "Nested client window closed.\n");<br>
> + free(ev);<br>
> + exit(0);<br>
<br>
</div></div>You can as well just let the WM kill you if that's all that you do. :-)<br>
(No, I'm not totally serious)<br></blockquote><div>I want to terminate nested Xorg properly when I click on its close window button. Without this exit(0) call, the window will be destroyed, but X server will keep running. My complain about this exit(0) is that such an "abrupt" termination may lead to memory leaks due to some allocated pointers not being properly free'd. Does it proceed?<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + }<br>
> +}<br>
> +<br>
[...]<br>
<span class="">> +void<br>
> +NestedClientCheckEvents(NestedClientPrivatePtr pPriv)<br>
> +{<br>
> + xcb_generic_event_t *ev;<br>
> +<br>
> + while (TRUE)<br>
> + {<br>
> + ev = xcb_poll_for_event(pPriv->conn);<br>
<br>
</span>In the name of micro-optimization I will mention xcb_poll_for_queued_event()<br>
once and then shut up again (the idea being to poll only in the first iteration<br>
and afterwards using poll_for_queued_event, but I don't think that this really<br>
makes much of a difference.<br></blockquote><div>OK, it's quite easy to implement, so I'm changing the code here. Thank you for the suggestion! <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5"><br>
> + if (!ev)<br>
> + {<br>
> + if (_NestedClientConnectionHasError(pPriv->scrnIndex,<br>
> + pPriv->conn))<br>
> + {<br>
> + /* XXX: Is there a better way to do this? */<br>
> + xf86DrvMsg(pPriv->scrnIndex,<br>
> + X_ERROR,<br>
> + "Connection with host X server lost.\n");<br>
> + free(ev);<br>
> + NestedClientCloseScreen(pPriv);<br>
> + exit(1);<br>
> + }<br>
> +<br>
> + break;<br>
> + }<br>
> +<br>
> + switch (ev->response_type & ~0x80)<br>
> + {<br>
> + case XCB_EXPOSE:<br>
> + _NestedClientProcessExpose(pPriv, ev);<br>
> + break;<br>
> + case XCB_CLIENT_MESSAGE:<br>
> + _NestedClientProcessClientMessage(pPriv, ev);<br>
> + break;<br>
> + case XCB_MOTION_NOTIFY:<br>
> + _NestedClientProcessMotionNotify(pPriv, ev);<br>
> + break;<br>
> + case XCB_KEY_PRESS:<br>
> + _NestedClientProcessKeyPress(pPriv, ev);<br>
> + break;<br>
> + case XCB_KEY_RELEASE:<br>
> + _NestedClientProcessKeyRelease(pPriv, ev);<br>
> + break;<br>
> + case XCB_BUTTON_PRESS:<br>
> + _NestedClientProcessButtonPress(pPriv, ev);<br>
> + break;<br>
> + case XCB_BUTTON_RELEASE:<br>
> + _NestedClientProcessButtonRelease(pPriv, ev);<br>
> + break;<br>
> + }<br>
> +<br>
> + free(ev);<br>
> + xcb_flush(pPriv->conn);<br>
<br>
</div></div>Why is this flushing inside of the event loop? Wouldn't a flush after the loop<br>
be enough?<br></blockquote><div>Well... Comparing it with other XCB snippets I've found in the web, it seems this xcb_flush() call is not needed at all. Calling it right atfer window creation or redrawing (XCB_EXPOSE event only) should be enough, right?<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + }<br>
> +}<br>
[...]<br>
<br>
Besides the above (minor) points, this look good to me, but I don't really have<br>
much of a clue.<br></blockquote><div>Once again, thank you very much for the appointments! <br></div></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><b>Laércio de Sousa</b><br></div><div style="text-align:center"><div style="text-align:left"><i>Orientador de Informática</i></div><div style="text-align:left"><u>Escola Municipal "Professor Eulálio Gruppi"</u></div><div style="text-align:left"><i>Rua Ismael da Silva Mello, 559, Mogi Moderno</i></div><i><div style="text-align:left"><i>Mogi das Cruzes - SP</i></div><div style="text-align:left"><i>CEP 08717-390</i></div></i></div><div><span style="font-family:arial;font-size:small">Telefone: (11) 4726-8313</span></div></div></div>
</div></div>