<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#c99">Comment # 99</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 Jonas Ådahl from <a href="show_bug.cgi?id=744932#c98">comment #98</a>) > (In reply to Owen Taylor from <a href="show_bug.cgi?id=744932#c95">comment #95</a>) > > Review of <a href="attachment.cgi?id=309854&action=diff" name="attach_309854" title="wayland: Introduce XWayland surface role">attachment 309854</a> <a href="attachment.cgi?id=309854&action=edit" title="wayland: Introduce XWayland surface role">[details]</a> <a href='review?bug=744932&attachment=309854'>[review]</a> [review] [review]: > > > > I attached this patch for review as part of this branch... but looking at > > it, I'd definitely like it if we can figure out a way to separate it - it > > looks entirely unrelated to the idea of the branch, and I'm not convinced by > > it as a patch yet. Is there some way we can land this branch without this> Indeed. I mentioned this in <a href="show_bug.cgi?id=744932#c86">comment 86</a> (oh, many comments on this one now). In the branch I did this, except I put the 4 patches I planned to push earlier first (<a href="https://github.com/jadahl/mutter/commits/wip/hidpi-cursor">https://github.com/jadahl/mutter/commits/wip/hidpi-cursor</a> is based on <a href="https://github.com/jadahl/mutter/commits/wip/surface-roles">https://github.com/jadahl/mutter/commits/wip/surface-roles</a> ) > > > > ::: src/wayland/meta-xwayland.c > > @@ +49,3 @@ > > + META_WAYLAND_SURFACE_ROLE_XWAYLAND, > > + surface->resource, > > + WL_DISPLAY_ERROR_INVALID_OBJECT)) > > > > Hmmm ... I'm quite surprised that meta_wayland_surface_set_role() doesn't > > follow Mutter standards for error handling, but returns 0/-1... I'd > > definitely write this as: > > > > != 0 > > > > as other callers do to make it clearer what is going on. > > > > (But please don't add more functions like this, and probably this one would > > be better changed.) > > Sure. I wanted to differentiate it from an actual server side error, but I > can make it an int(). I misread what you wrote, and misremembered what I wrote. This one is clearly just me forgetting to add the != 0 which I did in all the other set_role() places.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are on the CC list for the bug.</li> </ul> </body> </html>