<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 #299282 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#c30">Comment # 30</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=299282&action=diff" name="attach_299282" title="MetaCursorSprite: Move Wayland part to own its frontend implementation">attachment 299282</a> <a href="attachment.cgi?id=299282&action=edit" title="MetaCursorSprite: Move Wayland part to own its frontend implementation">[details]</a></span> <a href='review?bug=744932&attachment=299282'>[review]</a>:

If the point of the refactoring is to get to a clean design, there are elements
of this patch that feel off to me

::: src/frontends/meta-cursor.c
@@ +156,2 @@
 MetaCursorSprite *
+meta_cursor_sprite_new (void)

meta_foo_new() should create a MetaFoo, not a MetaFoo or a subclass. This is
something like meta_create_cursor_sprite().

@@ +160,3 @@
+    return g_object_new (META_TYPE_CURSOR_SPRITE_WAYLAND, NULL);
+  else
+    return g_object_new (META_TYPE_CURSOR_SPRITE, NULL);

Hmm, I think this reflects confusion whether MetaCursorSprite is a frontend or
a backend object. If it's genuinely a "frontend" object, then when creating it
we should always know what protocol we are creating it for. And
meta_cursor_sprite_from_theme() would be a third subclass independent of the
backend.

Obviously every transformation of frontend to backend isn't going to be
possible - e.g., creating an X cursor for a Wayland surface - but I don't think
that's a big problem - you just need to have the code somewhere that knows what
can be transformed to what.

::: src/frontends/wayland/meta-cursor-wayland.c
@@ +66,3 @@
+  MetaCursorSpriteWayland *self;
+
+  self = META_CURSOR_SPRITE_WAYLAND (meta_cursor_sprite_new ());

This doesn't make sense to me - this *has* to be a MetaCursorSpriteWayland, so
just create one.</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>