After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759222 - MetaWaylandDataDevice: Don't set surface offset as anchor offset
MetaWaylandDataDevice: Don't set surface offset as anchor offset
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2015-12-09 02:57 UTC by Jonas Ådahl
Modified: 2016-01-19 13:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
MetaWaylandDataDevice: Don't set surface offset as anchor offset (2.06 KB, patch)
2015-12-09 02:57 UTC, Jonas Ådahl
none Details | Review
MetaWaylandDataDevice: Don't set surface offset as anchor offset (3.86 KB, patch)
2015-12-23 07:51 UTC, Jonas Ådahl
committed Details | Review

Description Jonas Ådahl 2015-12-09 02:57:14 UTC
This fixes the mutter side of bug 759168.
Comment 1 Jonas Ådahl 2015-12-09 02:57:35 UTC
Created attachment 316995 [details] [review]
MetaWaylandDataDevice: Don't set surface offset as anchor offset

Since we are using the surface actor to draw the DND icon, the offset
is already accounted for by MetaSurfaceActorWayland, and passing the
surface position offset would effectively double the actual offset,
causing the icon to be misplaced.

This patch always sets the anchor offset to (0, 0) when the icon is a
Wayland surface, and lets the surface actor deal with the offsetting.
Comment 2 Carlos Garnacho 2015-12-10 11:18:06 UTC
Comment on attachment 316995 [details] [review]
MetaWaylandDataDevice: Don't set surface offset as anchor offset

Looks mostly good, I definitely didn't notice the offset was already set in the surface. It seems like all of meta_wayland_data_device_update_dnd_surface() is superfluous then, though.
Comment 3 Jonas Ådahl 2015-12-23 07:39:39 UTC
(In reply to Carlos Garnacho from comment #2)
> Comment on attachment 316995 [details] [review] [review]
> MetaWaylandDataDevice: Don't set surface offset as anchor offset
> 
> Looks mostly good, I definitely didn't notice the offset was already set in
> the surface. It seems like all of
> meta_wayland_data_device_update_dnd_surface() is superfluous then, though.

Is there more stuff related to this to remove? Like the "anchor-x/y" in MetaFeedbackActor? Is the feedback used only for Wayland client surfaces, or are there non-wl_surface based icons that needs the offset?
Comment 4 Jonas Ådahl 2015-12-23 07:51:10 UTC
Created attachment 317809 [details] [review]
MetaWaylandDataDevice: Don't set surface offset as anchor offset

Since we are using the surface actor to draw the DND icon, the offset
is already accounted for by MetaSurfaceActorWayland, and passing the
surface position offset would effectively double the actual offset,
causing the icon to be misplaced.

This patch always sets the anchor offset to (0, 0) when the icon is a
Wayland surface, and lets the surface actor deal with the offsetting.
Comment 5 Carlos Garnacho 2015-12-23 12:17:45 UTC
(In reply to Jonas Ådahl from comment #3)
> (In reply to Carlos Garnacho from comment #2)
> > Comment on attachment 316995 [details] [review] [review] [review]
> > MetaWaylandDataDevice: Don't set surface offset as anchor offset
> > 
> > Looks mostly good, I definitely didn't notice the offset was already set in
> > the surface. It seems like all of
> > meta_wayland_data_device_update_dnd_surface() is superfluous then, though.
> 
> Is there more stuff related to this to remove? Like the "anchor-x/y" in
> MetaFeedbackActor? Is the feedback used only for Wayland client surfaces, or
> are there non-wl_surface based icons that needs the offset?

anchor-x/y was also added with the patches in bug #732367 in mind, where the feedback actors were actually a subclass of MetaFeedbackActor. I need to go back to reworking that, but if I make the actors StWidget as was last suggested, then anchor-x/y is debatable too.

It could perhaps be done in a separate commit, which can be reverted if turns out necessary.
Comment 6 Carlos Garnacho 2015-12-23 12:19:11 UTC
Comment on attachment 317809 [details] [review]
MetaWaylandDataDevice: Don't set surface offset as anchor offset

Looks good!
Comment 7 Carlos Garnacho 2016-01-19 13:34:34 UTC
Ended up pushing this one.

Attachment 317809 [details] pushed as bcdda50 - MetaWaylandDataDevice: Don't set surface offset as anchor offset