Skip to content

Commit

Permalink
Merge #1256
Browse files Browse the repository at this point in the history 
1256: Move debugging methods from wm to connection class r=AlanGriffiths a=wmww

Makes it more convenient to surface useful debugging messages from the surface class

Co-authored-by: William Wold <[email protected]>
  • Loading branch information
@bors @wmww @AlanGriffiths
2 people authored and AlanGriffiths committed Feb 17, 2020
1 parent ae76246 commit 7c50dee
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 190 deletions.
148 changes: 145 additions & 3 deletions src/server/frontend_xwayland/xcb_connection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,32 @@
#include "mir/log.h"

#include "boost/throw_exception.hpp"
#include <sstream>

namespace mf = mir::frontend;
namespace geom = mir::geometry;

namespace
{
template<typename T, typename U = T>
auto data_buffer_to_debug_string(
T* data,
size_t elements,
std::function<U(T element)> converter = [](T e) -> T { return e; }) -> std::string
{
std::stringstream ss;
ss << "[";
for (T* i = data; i != data + elements; i++)
{
if (i != data)
ss << ", ";
ss << converter(*i);
}
ss << "]";
return ss.str();
}
}

mf::XCBConnection::Atom::Atom(std::string const& name, XCBConnection* connection)
: connection{connection},
name_{name},
Expand Down Expand Up @@ -52,6 +74,7 @@ auto mf::XCBConnection::Atom::name() const -> std::string

mf::XCBConnection::XCBConnection(int fd)
: xcb_connection{xcb_connect_to_fd(fd, nullptr)},
xcb_screen{xcb_setup_roots_iterator(xcb_get_setup(xcb_connection)).data},
wm_protocols{"WM_PROTOCOLS", this},
wm_take_focus{"WM_TAKE_FOCUS", this},
wm_delete_window{"WM_DELETE_WINDOW", this},
Expand Down Expand Up @@ -106,7 +129,17 @@ mf::XCBConnection::operator xcb_connection_t*() const
return xcb_connection;
}

auto mf::XCBConnection::query_name(xcb_atom_t atom) -> std::string
auto mf::XCBConnection::screen() const -> xcb_screen_t*
{
return xcb_screen;
}

auto mf::XCBConnection::root_window() const -> xcb_window_t
{
return xcb_screen->root;
}

auto mf::XCBConnection::query_name(xcb_atom_t atom) const -> std::string
{
// TODO: cache, for cheaper lookup

Expand All @@ -132,12 +165,12 @@ auto mf::XCBConnection::query_name(xcb_atom_t atom) -> std::string
return name;
}

auto mf::XCBConnection::reply_contains_string_data(xcb_get_property_reply_t const* reply) -> bool
auto mf::XCBConnection::reply_contains_string_data(xcb_get_property_reply_t const* reply) const -> bool
{
return reply->type == XCB_ATOM_STRING || reply->type == utf8_string;
}

auto mf::XCBConnection::string_from(xcb_get_property_reply_t const* reply) -> std::string
auto mf::XCBConnection::string_from(xcb_get_property_reply_t const* reply) const -> std::string
{
if (!reply_contains_string_data(reply))
{
Expand All @@ -150,6 +183,12 @@ auto mf::XCBConnection::string_from(xcb_get_property_reply_t const* reply) -> st
static_cast<unsigned long>(xcb_get_property_value_length(reply))};
}

bool mf::XCBConnection::is_ours(uint32_t id) const
{
auto setup = xcb_get_setup(xcb_connection);
return (id & ~setup->resource_id_mask) == setup->resource_id_base;
}

auto mf::XCBConnection::read_property(
xcb_window_t window,
xcb_atom_t prop,
Expand Down Expand Up @@ -315,6 +354,109 @@ void mf::XCBConnection::configure_window(
}
}

auto mf::XCBConnection::reply_debug_string(xcb_get_property_reply_t* reply) const -> std::string
{
if (reply == nullptr)
{
return "(null reply)";
}
else if (reply_contains_string_data(reply))
{
auto const text = string_from(reply);
return "\"" + text + "\"";
}
else
{
// The length returned by xcb_get_property_value_length() is in bytes for some reason
size_t const len = xcb_get_property_value_length(reply) / (reply->format / 8);
std::stringstream ss;
ss << std::to_string(reply->format) << "bit " << query_name(reply->type) << "[" << len << "]";
if (len < 32 && (
reply->type == XCB_ATOM_CARDINAL ||
reply->type == XCB_ATOM_INTEGER ||
reply->type == XCB_ATOM_ATOM ||
reply->type == XCB_ATOM_WINDOW))
{
ss << ": ";
void* const ptr = xcb_get_property_value(reply);
switch (reply->type)
{
case XCB_ATOM_CARDINAL: // unsigned number
switch (reply->format)
{
case 8: ss << data_buffer_to_debug_string(static_cast<uint8_t*>(ptr), len); break;
case 16: ss << data_buffer_to_debug_string(static_cast<uint16_t*>(ptr), len); break;
case 32: ss << data_buffer_to_debug_string(static_cast<uint32_t*>(ptr), len); break;
}
break;

case XCB_ATOM_INTEGER: // signed number
switch (reply->format)
{
case 8: ss << data_buffer_to_debug_string(static_cast<int8_t*>(ptr), len); break;
case 16: ss << data_buffer_to_debug_string(static_cast<int16_t*>(ptr), len); break;
case 32: ss << data_buffer_to_debug_string(static_cast<int32_t*>(ptr), len); break;
}
break;

case XCB_ATOM_ATOM:
if (reply->format != sizeof(xcb_atom_t) * 8)
{
ss << "Atom property has format " << std::to_string(reply->format);
break;
}
ss << data_buffer_to_debug_string<xcb_atom_t, std::string>(
static_cast<xcb_atom_t*>(ptr),
len,
[this](xcb_atom_t atom) -> std::string
{
return query_name(atom);
});
break;

case XCB_ATOM_WINDOW:
if (reply->format != sizeof(xcb_window_t) * 8)
{
ss << "Window property has format " << std::to_string(reply->format);
break;
}
ss << data_buffer_to_debug_string<xcb_window_t, std::string>(
static_cast<xcb_window_t*>(ptr),
len,
[this](xcb_window_t window) -> std::string
{
return window_debug_string(window);
});
break;
}
}
return ss.str();
}
}

auto mf::XCBConnection::client_message_debug_string(xcb_client_message_event_t* event) const -> std::string
{
switch (event->format)
{
case 8: return data_buffer_to_debug_string(event->data.data8, 20);
case 16: return data_buffer_to_debug_string(event->data.data16, 10);
case 32: return data_buffer_to_debug_string(event->data.data32, 5);
}
return "unknown format " + std::to_string(event->format);
}

auto mf::XCBConnection::window_debug_string(xcb_window_t window) const -> std::string
{
if (!window)
return "null window";
else if (window == xcb_screen->root)
return "root window";
else if (is_ours(window))
return "our window " + std::to_string(window);
else
return "window " + std::to_string(window);
}

auto mf::XCBConnection::xcb_type_atom(XCBType type) const -> xcb_atom_t
{
switch (type)
Expand Down
20 changes: 17 additions & 3 deletions src/server/frontend_xwayland/xcb_connection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class XCBConnection
{
private:
xcb_connection_t* const xcb_connection;
xcb_screen_t* const xcb_screen;

public:
class Atom
Expand All @@ -80,11 +81,17 @@ class XCBConnection
~XCBConnection();

operator xcb_connection_t*() const;
auto screen() const -> xcb_screen_t*;
auto root_window() const -> xcb_window_t;


/// Does a round-trip to the X server and does not cache. Should be improved if needed on normal code paths
auto query_name(xcb_atom_t atom) -> std::string;
auto reply_contains_string_data(xcb_get_property_reply_t const* reply) -> bool;
auto string_from(xcb_get_property_reply_t const* reply) -> std::string;
auto query_name(xcb_atom_t atom) const -> std::string;
auto reply_contains_string_data(xcb_get_property_reply_t const* reply) const -> bool;
auto string_from(xcb_get_property_reply_t const* reply) const -> std::string;

/// If the window was created by us
auto is_ours(xcb_window_t window) const -> bool;

/// Read a single property of various types from the window
/// Returns a function that will wait on the reply before calling action()
Expand Down Expand Up @@ -162,6 +169,13 @@ class XCBConnection
xcb_flush(xcb_connection);
}

/// Create strings for debug messages
/// @{
auto reply_debug_string(xcb_get_property_reply_t* reply) const -> std::string;
auto client_message_debug_string(xcb_client_message_event_t* event) const -> std::string;
auto window_debug_string(xcb_window_t window) const -> std::string;
/// @}

Atom const wm_protocols;
Atom const wm_take_focus;
Atom const wm_delete_window;
Expand Down
12 changes: 11 additions & 1 deletion src/server/frontend_xwayland/xwayland_surface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,10 @@ void mf::XWaylandSurface::run_on_wayland_thread(std::function<void()>&& work)

void mf::XWaylandSurface::wl_surface_destroyed()
{
if (verbose_xwayland_logging_enabled())
{
log_debug("%s's wl_surface destoyed", connection->window_debug_string(window).c_str());
}
close();
}

Expand Down Expand Up @@ -678,6 +682,11 @@ void mf::XWaylandSurface::create_scene_surface_if_needed()
return;
}

if (verbose_xwayland_logging_enabled())
{
log_debug("creating scene surface for %s", connection->window_debug_string(window).c_str());
}

state = window_state;
state.withdrawn = false;

Expand Down Expand Up @@ -744,7 +753,8 @@ void mf::XWaylandSurface::inform_client_of_window_state(WindowState const& new_w
{
if (verbose_xwayland_logging_enabled())
{
log_debug("inform_client_of_window_state(withdrawn: %s", new_window_state.withdrawn ? "yes" : "no");
log_debug("inform_client_of_window_state(window: %s", connection->window_debug_string(window).c_str());
log_debug(" withdrawn: %s", new_window_state.withdrawn ? "yes" : "no");
log_debug(" minimized: %s", new_window_state.minimized ? "yes" : "no");
log_debug(" maximized: %s", new_window_state.maximized ? "yes" : "no");
log_debug(" fullscreen: %s)", new_window_state.fullscreen ? "yes" : "no");
Expand Down
Loading

0 comments on commit 7c50dee

Please sign in to comment.