GNOME Bugzilla – Bug 698282
set location bubble pixel-aligned translation on notify::width
Last modified: 2013-04-26 01:18:43 UTC
Created attachment 241812 [details] [review] UserLocation: set pixel-aligned translation on notify::width Setting translation on first allocation is not enough as the size may change: Use notify::width as that's what the translation depends on. Also, make sure we only translate full pixels to avoid text smearing.
Review of attachment 241812 [details] [review]: Also I'd divide this in two patches: 1. Keep adjusting translation according to actor size changes 2. Pixel-align the x tanslation ::: src/userLocation.js @@ +41,3 @@ this._locationMarker = new Champlain.CustomMarker(); this._locationMarker.set_location(this.latitude, this.longitude); + this._locationMarker.connect('notify::width', Lang.bind(this, why only width? Do we know for sure that height is already allocated when width is? @@ +44,2 @@ function() { + this._locationMarker.set_translation(-(Math.floor(this._locationMarker.get_width() / 2)), I'd use a variable here: let translate_x = -(Math.floor(this._locationMarker.get_width() / 2);
(In reply to comment #1) > why only width? Do we know for sure that height is already allocated when width You have a point, I wasn't thinking it through either... Luckily Actor has a "size" property as well. I'll fix that and other issues you mentioned. Thanks.
Created attachment 241998 [details] [review] UserLocation: translate pin and bubble on notify::size Setting translation on first allocation is not enough as the size may change later: Use notify::size instead.
Created attachment 241999 [details] [review] UserLocation: pixel-align pin and bubble translation Avoid non-pixel-aligned actors as that leads to blurry text rendering.
Review of attachment 241998 [details] [review]: ACK
Review of attachment 241999 [details] [review]: ACK with this change. ::: src/userLocation.js @@ +43,3 @@ this._locationMarker.connect('notify::size', Lang.bind(this, function() { + let translate_x= -Math.floor(this._locationMarker.get_width() / 2); space before '='
Created attachment 242010 [details] [review] UserLocation: pixel-align pin and bubble translation Avoid non-pixel-aligned actors as that leads to blurry text rendering.
Review of attachment 242010 [details] [review]: ACK. For future ref, if an ACK is given, you can simply directly push with the minor changes requested.
Attachment 241998 [details] pushed as 5758d74 - UserLocation: translate pin and bubble on notify::size Attachment 242010 [details] pushed as 8bdfced - UserLocation: pixel-align pin and bubble translation
You fixed the translation of pin but not the accuracy circle.
Created attachment 242303 [details] Screenshot of issue Screenshot of accuracy circle not being positioned correctly on zoom level changes.
(In reply to comment #10) > You fixed the translation of pin but not the accuracy circle. NM, it turned out to be a Champlain issue. See bug#698910.