[Wayland-bugs] [Bug 744932] Wrong (ultra tiny/small) cursor size on HiDPI screen

[mutter (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=744932

Owen Taylor <otaylor at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #310995|none                        |reviewed
             status|                            |

--- Comment #136 from Owen Taylor <otaylor at redhat.com> ---
Review of attachment 310995:

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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-bugs/attachments/20150909/953a3341/attachment.html>