<html>
<head>
<base href="https://bugzilla.gnome.org/" />
</head>
<body><span class="vcard"><a href="page.cgi?id=describeuser.html&login=otaylor%40redhat.com" title="Owen Taylor <otaylor@redhat.com>"> <span class="fn">Owen Taylor</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - Wrong (ultra tiny/small) cursor size on HiDPI screen"
href="https://bugzilla.gnome.org/show_bug.cgi?id=744932">bug 744932</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #309854 status</td>
<td>none
</td>
<td>needs-work
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Wrong (ultra tiny/small) cursor size on HiDPI screen"
href="https://bugzilla.gnome.org/show_bug.cgi?id=744932#c95">Comment # 95</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Wrong (ultra tiny/small) cursor size on HiDPI screen"
href="https://bugzilla.gnome.org/show_bug.cgi?id=744932">bug 744932</a>
from <span class="vcard"><a href="page.cgi?id=describeuser.html&login=otaylor%40redhat.com" title="Owen Taylor <otaylor@redhat.com>"> <span class="fn">Owen Taylor</span></a>
</span></b>
<pre>Review of <span class=""><a href="attachment.cgi?id=309854&action=diff" name="attach_309854" title="wayland: Introduce XWayland surface role">attachment 309854</a> <a href="attachment.cgi?id=309854&action=edit" title="wayland: Introduce XWayland surface role">[details]</a></span> <a href='review?bug=744932&attachment=309854'>[review]</a>:
I attached this patch for review as part of this branch... but looking at it,
I'd definitely like it if we can figure out a way to separate it - it looks
entirely unrelated to the idea of the branch, and I'm not convinced by it as a
patch yet. Is there some way we can land this branch without this>
* It's not clear to me why Xwayland windows are a separate role... they seem
to behave a lot like shell surfaces, and in a brief look it appears Xwayland
calls wl_shell_get_shell_surface() on them - which by the protocol makes them
have the shell surface role. I don't particularly like introducing the idea
that we have internal pseudo-roles that are different from the roles of the
Wayland protocol. If we want to special-case Xwayland windows, shouldn't we do
it by some other method?
* The commit message here doesn't reflect the idea of the patch at all,
because looking at the huge comment in meta-wayland-surface.c, it looks like
the *idea* of the patch is that we want to somehow treat frame callbacks
differently on Xwayland windows to try and fix visual artifacts during map?
::: src/wayland/meta-xwayland.c
@@ +49,3 @@
+ META_WAYLAND_SURFACE_ROLE_XWAYLAND,
+ surface->resource,
+ WL_DISPLAY_ERROR_INVALID_OBJECT))
Hmmm ... I'm quite surprised that meta_wayland_surface_set_role() doesn't
follow Mutter standards for error handling, but returns 0/-1... I'd definitely
write this as:
!= 0
as other callers do to make it clearer what is going on.
(But please don't add more functions like this, and probably this one would be
better changed.)</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>