<html> <head> <base href="https://bugzilla.gnome.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW - wayland: Implement DND icon hotspot API" href="https://bugzilla.gnome.org/show_bug.cgi?id=759168#c8">Comment # 8</a> on <a class="bz_bug_link bz_status_NEW " title="NEW - wayland: Implement DND icon hotspot API" href="https://bugzilla.gnome.org/show_bug.cgi?id=759168">bug 759168</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 Matthias Clasen from <a href="show_bug.cgi?id=759168#c3">comment #3</a>) > Review of <a href="attachment.cgi?id=316924&action=diff" name="attach_316924" title="wayland: Implement DND icon hotspot API">attachment 316924</a> <a href="attachment.cgi?id=316924&action=edit" title="wayland: Implement DND icon hotspot API">[details]</a> <a href='review?bug=759168&attachment=316924'>[review]</a> [review]: > > ::: gdk/wayland/gdkdnd-wayland.c > @@ +51,3 @@ > gdouble y; > + gint prev_hot_x; > + gint prev_hot_y; > > I don't see the need to store these in the context. They can just be locals > in set_hot_spot True, this was part of an idea of making the offset being retrieved in another way, but now that things are done this way its not needed. Will remove. > > @@ +329,3 @@ > + .width = 1, > + .height = 1, > + }, > > This kind of on-the-fly struct looks a little out of style in the GTK+ > codebase. Do we do this elsewhere in the Wayland backend ? Hmm, no. We use it in mutter here and there, but if you prefer I can leave it out of GTK+. In fact, we might not need the invalidation call at all if we can safely assume that the hotspot is always set while setting an icon that will be drawn anyway. The reason for adding the invalidation is to ensure we get a wl_surface.attach.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are on the CC list for the bug.</li> </ul> </body> </html>