<html> <head> <base href="https://bugzilla.gnome.org/" /> </head> <body> <div> <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#c138">Comment # 138</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 <a href="page.cgi?id=describeuser.html&login=jadahl%40gmail.com" title="Jonas Ådahl <jadahl@gmail.com>"> Jonas Ådahl</a> <pre>(In reply to Owen Taylor from <a href="show_bug.cgi?id=744932#c132">comment #132</a>) > Review of <a href="attachment.cgi?id=310996&action=diff" name="attach_310996" title="wayland: Support sending wl_surface.enter/leave to cursor surfaces">attachment 310996</a> <a href="attachment.cgi?id=310996&action=edit" title="wayland: Support sending wl_surface.enter/leave to cursor surfaces">[details]</a> <a href='review?bug=744932&attachment=310996'>[review]</a> [review]: > > @@ +993,3 @@ > + MetaWaylandSurfaceRoleCursor *cursor_role = > + META_WAYLAND_SURFACE_ROLE_CURSOR (surface->role); > + MetaCursorSprite *displayed_cursor_sprite; > > No change needed, but I wanted to mention that there is no line length limit > in Mutter source code, and the goal of introducing line breaks is to > maximize readability. FWIW I more or less always use vertical split editor buffers, so long lines are usually unreadable in my setup. In reply to Owen Taylor from <a href="show_bug.cgi?id=744932#c136">comment #136</a>) > Review of <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> <a href='review?bug=744932&attachment=310995'>[review]</a> [review]: > > 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. Yea, I dropped that part, but didn't do any more major changes because its not the right time to redesign things just right now. I agree with you here anyhow. > ::: 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. Thought about doing it up front, but "FAILED" didn't make sense as a intermediate state. INVALIDATED would maybe make more sense. I'll go with that.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are on the CC list for the bug.</li> </ul> </body> </html>