[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
----------------------------------------------------------------------------
                 CC|                            |otaylor at redhat.com

--- Comment #24 from Owen Taylor <otaylor at redhat.com> ---
(In reply to Jonas Ådahl from comment #23)
> (In reply to Owen Taylor from comment #21)
> > Review of attachment 299278 [details] [review] [review]:
> > 
> > ::: src/backends/meta-cursor-tracker.c
> > @@ +90,3 @@
> > +    {
> > +      g_object_ref (displayed_cursor);
> > +      tracker->displayed_cursor = displayed_cursor;
> > 
> > g_object_returns the ref'ed object as well - not sure if this change is
> > because you aren't aware of that or object to that stylistically.
> 
> Is it a stylistic guideline to do o = g_object_ref (o)? I was aware of it
> returning the object, but assumed relying on the side effect was enough. If
> the guideline is to do o = g_object_ref (o) I can change it.

You'd find that the vast majority of uses of g_object_ref() in mutter do use
the return value - it's very common throughout GNOME and certainly how I'd
write this type of code. I probably wouldn't have commented it if you just did
it in two lines in new code - the comment was inspired by the fact that you
changed code using a return value (for a custom ref function) to not using the
return value and that made me do a double take.... it seemed like a bigger
delta than necessary.

In addition to just saving a line of code, the reason I like using the return
value is that it makes it clear that assigning a newly acquired reference is a
single conceptual action - the assignment can't become accidentally detached
from the reference.

> > ::: src/backends/meta-cursor.c
> > @@ +54,3 @@
> > +} MetaCursorImage;
> > +
> > +struct _MetaCursorSpritePrivate
> > 
> > This is OK, but to express my opinion, I don't really see any reason to use
> > a private structure - private structures are basically useful for publicly
> > derivable objects where you need to have the object class in a public
> > header. MetaCursorSprite isn't derivable and it isn't even public.
> 
> The point of making it private is to not tempt anyone to use
> cursor_sprite->internal_data and instead use a provided API. I think this
> makes sense even if the object is not exposed publicly.

The same could be done by moving the MetaCursor structure meta-cursor.c. I
don't have a real objection to using a private data structure - there is
certainly no significant performance penalty. I think a lot of people like to
just use them everywhere to avoid having different patterns of writing objects
depending on whether they are publicly derivable or not.

-- 
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/20150409/f02eb1c9/attachment.html>