<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 #310995 status</td>
<td>none
</td>
<td>reviewed
</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#c136">Comment # 136</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=310995&action=diff" name="attach_310995" title="Support scaling of cursor sprites given what output they are on">attachment 310995</a> <a href="attachment.cgi?id=310995&action=edit" title="Support scaling of cursor sprites given what output they are on">[details]</a></span> <a href='review?bug=744932&attachment=310995'>[review]</a>:
It looks like in this version, you are no longer reusing buffers, but always
allocating new buffers. I wanted to note (perhaps for a future improvement)
that if buffers are not being reused, then this can be simplified considerably,
and the handling of avoiding messing with buffers that are being scanned out
could be handled in done in the *renderer* rather than in the sprite, with a
single BO attached to the sprite. update_hw_cursor then does:
if (cursor_private->bo != renderer->current_bo)
{
bo *last_bo = renderer->last_bo;
renderer->last_bo = renderer->current_bo;
renderer->current_bo = cursor_private->bo;
drmModeSetCursor2(renderer->current_bo);
if (last_bo)
unref_bo(last_bo);
}
The "bo" here is a trival structure that wraps struct gbm_bo with a reference
count.
This approach would also fix the problem you have here (though it's not an
important one) that if a cursor surface is *destroyed* by a client, it's
buffers can no longer be kept referenced to prevent glitches.
Anyways, generally this looks like it should work, and I'm OK landing this as
is, with a few minor fixes noted below.
::: src/backends/meta-cursor-renderer.h
@@ +59,3 @@
MetaCursorRenderer * meta_cursor_renderer_new (void);
+void meta_cursor_renderer_pre_paint (MetaCursorRenderer *renderer);
Left-over
::: src/backends/native/meta-cursor-renderer-native.c
@@ +508,3 @@
+ * should unset, or succeed, which will set new buffer.
+ */
+ destroy_pending_cursor_sprite_gbm_bo (cursor_sprite);
Putting the private into an "invalid state", then having a later call to fix it
up seems fiddly and fragile - why isn't the destroy part of the
set_pending_cursor_sprite_gbm_bo[_failed]()?
You could possibly just call set_pending_cursor_sprite_gbm_bo_failed() up front
rather than in each failure path - similar to how you set the state to FAILED
at initialization.
::: src/backends/x11/meta-cursor-renderer-x11.c
@@ +30,3 @@
+#include <meta/util.h>
+
Don't think this addition was needed
::: src/compositor/compositor.c
@@ +77,3 @@
#include "meta-sync-ring.h"
+#include "backends/meta-stage.h"
extraneous?
@@ +1098,3 @@
compositor->have_x11_sync_object = meta_sync_ring_insert_wait ();
else
+ XSync (display->xdisplay, False);
all changes in this file seem to be leftoverse</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>