<html> <head> <base href="https://bugzilla.gnome.org/" /> </head> <body><a href="page.cgi?id=describeuser.html&login=otaylor%40redhat.com" title="Owen Taylor <otaylor@redhat.com>"> Owen Taylor</a> 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> <table border="1" cellspacing="0" cellpadding="8"> <tr> <th>What</th> <th>Removed</th> <th>Added</th> </tr> <tr> <td style="text-align:right;">CC</td> <td> </td> <td>otaylor@redhat.com </td> </tr></table> <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#c24">Comment # 24</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=otaylor%40redhat.com" title="Owen Taylor <otaylor@redhat.com>"> Owen Taylor</a> <pre>(In reply to Jonas Ådahl from <a href="show_bug.cgi?id=744932#c23">comment #23</a>) > (In reply to Owen Taylor from <a href="show_bug.cgi?id=744932#c21">comment #21</a>) > > Review of <a href="attachment.cgi?id=299278&action=diff" name="attach_299278" title="Make MetaCursorSprite a GObject">attachment 299278</a> <a href="attachment.cgi?id=299278&action=edit" title="Make MetaCursorSprite a GObject">[details]</a> <a href='review?bug=744932&attachment=299278'>[review]</a> [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.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are on the CC list for the bug.</li> </ul> </body> </html>