Jason Ekstrand <ja...@jlekstrand.net> writes: > As with patch 1, I've gone through and made a pile of style changes to > bring things back under 80 characters. You can find it on this > branch:
I've adopted that style across the whole series. >> struct wsi_display { >> @@ -1414,5 +1421,468 @@ wsi_release_display(VkPhysicalDevice >> physical_device, >> close(wsi->fd); >> wsi->fd = -1; >> } >> +#ifdef VK_USE_PLATFORM_XLIB_XRANDR_EXT >> + wsi_display_connector_from_handle(display)->output = None; >> > > Does this function ever return NULL? That's a standard handle to pointer macro -- if display is valid, then the macro will return a valid pointer. >> +#ifdef VK_USE_PLATFORM_XLIB_XRANDR_EXT >> + >> +static struct wsi_display_connector * >> +wsi_display_find_output(struct wsi_device *wsi_device, >> + RROutput output) >> > > You're using RROutput here but xcb_rander_output_t elsewhere. Why not just > pick one? The API is specified using Xlib types, but the implementation uses XCB. They are the "same" underlying type of course; I have switched to use the xcb types uniformly in the implementation except in the public API which then casts from RROutput to xcb_randr_output_t. > Things are going way over the usual 80 character limit. I'm fixing up > whitespace as I read and will put a fixup in my branch. All fixed. >> + xcb_randr_query_version_reply_t *qv_r = >> xcb_randr_query_version_reply(connection, qv_c, NULL); >> + free(qv_r); >> > > What's up with the version query? We're throwing it away without ever > using it. Does this provide us with some error checkint of some sort? You're required to call the query_version request before doing any other RandR requests, and this function may well be making the very first calls to RandR. In this case, we're about to call a RandR function supported since time immemorial, so we don't actually care what version the server reports. You'll note that we *do* check the version before we start trying to perform any leasing operations. >> + if (!connector) { >> > > This is always true. ... > And this can be merged in. Yeah, I noticed that this got refactored several times and the conditions became mixed up (not "wrong", but illogical). I've rewritten it so that it makes sense again. >> + for (int o = 0; o < num_outputs; o++) >> + if (outputs[o] == output && num_outputs == 1) { >> > > checking for num_outputs == 1 inside the loop is a bit odd. Thanks. Again, refactoring this code left it looking strange. I've changed this to: if (num_outputs == 1 && outputs[0] == output) active_crtc = rc[c]; -- -keith
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev