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 720951 - focus-tracking sets wrong coordinates in magnifier
focus-tracking sets wrong coordinates in magnifier
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: magnifier
3.10.x
Other Linux
: Normal major
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 720952 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-12-22 22:24 UTC by Magdalen Berns (irc magpie)
Modified: 2014-02-06 00:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Take x,y from center of focused widget (962 bytes, patch)
2013-12-28 01:37 UTC, Magdalen Berns (irc magpie)
reviewed Details | Review
Take x,y from center of focused widget (962 bytes, patch)
2014-02-05 18:26 UTC, Magdalen Berns (irc magpie)
committed Details | Review

Description Magdalen Berns (irc magpie) 2013-12-22 22:24:51 UTC
To recreate you turn the magnifier on and try navigating with they keys through some applications which have larger widgets to focus on and you are soon likely to note that view of the focused widget can often be too far away to be in visible ranges of the magnifier zoom regions. 

Sometimes the widget remains completely out of view. Yet it is clear the tracking is active and that the view moved because a widget somewhere gained focus to prompt a visual change. 

I suspect this is happening because the focused widget needs responds to the query made for its extents with x, and y values which need to have correction applied to them before these x,y coordinates get interpreted since their interpretation is wrong otherwise. 

The API documentation does not say which part of a component it takes to be x nor y, I am assuming convention to be top left at 0, 0 at the moment which would make sense given the problem it seems to have in finding larger widgets to magnifiy 

I did a badly drawn ascii diagram to express what I think got overlooked but whatever, way round the principle of correcting the systematic error which results would be the same in any case and I think it should be straight forward to correct and get the focus tracking working properly, at last as long as my assumptions are warm. 


 Sketch: THE Desktop

(0,0)     x  (x,0)
  --------------> 
  |  *--------*
y |  |    | o || the 'o' and | | reoresebts the widget and windows respectively
  |  *--------*
  \/
  (0,y)


I reckon an evaluation of the advantages of the default focus-tracking settings needs a second look too. As the issue does seems to be worse under 'push' consider how quickly focus can be gained and how slowly it reacts on top of the fact it seems to be contributing to the negative impact skewed data being read by the magnifier seem to make on the experience of trying to key navigate with it activated.

Thanks.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-12-22 23:06:23 UTC
*** Bug 720952 has been marked as a duplicate of this bug. ***
Comment 2 Magdalen Berns (irc magpie) 2013-12-28 01:37:09 UTC
Created attachment 264950 [details] [review]
Take x,y from center of focused widget

Having checked I think it is safe to say the top left corner of the component is the x, y reference point for focused events and obviously that is something which needs correction or it will always be wrong.

I have made a simple change which adds half the length of the focused component width to its x coordinate and half the length of the component's height to its y coordinate so that we take use center of the component when we zoom the view to a focused component. Note, it may not be completely accurate because the dimensions are not rounded when they get divided by 2 but if I need to change that I would like to do it in every situation where this happens in the magnifier rather than just here so that is why I refrained from using rounding.

The patch seems to work well with 'proportional' but the 'push' setting continues to have some toggling/flipping problems. This erratic behaviour has been a gripe expressed by a couple of users so it reasonable to assume we do not want it to happen but, given that the proportional setting does what it should and does it well with this patch I think the logical thing to do is to change the default to 'proportional' in gsettings and deal with 'push' separately. I guess it can be a point of debate for others to decide since it is not a priority to have focus-tracking 'push' tweaked when I am not at all certain the setting has any added value to offer.
Comment 3 Florian Müllner 2014-02-02 15:14:39 UTC
Review of attachment 264950 [details] [review]:

OK.
Comment 4 Florian Müllner 2014-02-04 21:52:33 UTC
Review of attachment 264950 [details] [review]:

Actually, you should remove the trailing whitespace before pushing ;-)
Comment 5 Jasper St. Pierre (not reading bugmail) 2014-02-04 21:58:24 UTC
Review of attachment 264950 [details] [review]:

(And also add the missing semicolon)
Comment 6 Magdalen Berns (irc magpie) 2014-02-05 18:26:32 UTC
Created attachment 268196 [details] [review]
Take x,y from center of focused widget 

How about this?
Comment 7 Magdalen Berns (irc magpie) 2014-02-05 19:44:57 UTC
Review of attachment 268196 [details] [review]:

Sorry, I seem to have accidentally committed this when pushing https://bugzilla.gnome.org/show_bug.cgi?id=712649 as I am not in the habit of writing the specific patch out for a push so I did not think when I pushed 712649 that it would take this one along with it! Will be more careful next time, apologies. Please let me know if you see any problems with the commit.
Comment 8 Florian Müllner 2014-02-05 22:13:13 UTC
(In reply to comment #7)
> Please let me know if you see any problems with the commit.

They are style issue, so not that much of a deal. If you care deeply, you can do a follow-up commit ...
Comment 9 Magdalen Berns (irc magpie) 2014-02-05 23:25:52 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > Please let me know if you see any problems with the commit.
> 
> They are style issue, so not that much of a deal. If you care deeply, you can
> do a follow-up commit ...

Thanks! Actually those two recommendations from you and Jasper I had dealt with before pushing 712649 so they should not be present in the committed patch at least. However, if you do see anything else there that I missed I would be happy to sort it out since I had intended to wait and give you a chance to check the changes before pushing them.
Comment 10 Florian Müllner 2014-02-06 00:03:13 UTC
(In reply to comment #9)
> Thanks! Actually those two recommendations from you and Jasper I had dealt with
> before pushing 712649 so they should not be present in the committed patch at
> least.

Ha, even better! That was what the two comments were actually asking for :-)