GNOME Bugzilla – Bug 680077
Malformed boxpointer arrows
Last modified: 2012-09-06 19:01:20 UTC
Created attachment 218999 [details] ibus popup screenshot This happens when an object is close to the screen edge and the boxpointer cannot be placed close enough to it. The point of the arrow is draw close to the object and the pointer itself becomes stretched and distorted. You can see this in the power off menu in the login screen and in the new iBus suggestion popups (see screenshot). A couple of recommendations here: * Reduce the horizontal padding between between the boxpointer and the screen edge. * Always ensure that the pointer is draw the same - it should have a fixed form that doesn't ever get distorted. If the pointer is a little way away from its parent then so be it.
Created attachment 223165 [details] [review] boxpointer: Ensure that the arrows are never obtuse angled
Created attachment 223166 [details] Screenshot with first compensation
Created attachment 223167 [details] [review] boxpointer: Try to improve the worst-case shape of the arrows
Created attachment 223168 [details] Screenshot with second compensation
In case it is not obvious, the second compensation/patch is meant to work with the first one. It is tries to improve over the first one.
That's an improvement, but it's still a bit odd looking. Why not give the pointer a fixed form?
Created attachment 223197 [details] [review] boxpointer: Always aim for an isosceles-angled arrow If the arrow was going to be acute angled, but non-isosceles, we try to adjust its direction and reduce the padding between the edge of the monitor by half, effectively settling for a slightly inaccurate arrow. If the arrow was going to be obtuse angled, we reduce the padding and adjust its direction as above, and also skip the rounded corner to achieve an isosceles-angled arrow. Currently this is only implemented for top and bottom arrows because I could not find a scenario where this would be needed for left and right arrows, which means I would not be able to test it. Such a need may arise in the future, though. For this to work, -arrow-border-radius should not be greater than half the -arrow-base. Existing style classes satisfy this requirement.
Created attachment 223208 [details] [review] boxpointer: Account for hanging actors Reducing the padding from the edge of the monitor to preserve the shape of the arrow can cause actors that are hanging out from the box (eg., close buttons) to be clipped. We should have a way to accomodate them. Introducing -boxpointer-extra to specify the amount of extra space that is needed. This is along the X axis when the arrow is at the top or bottom, and is on the Y axis when the arrow is on the left or right.
Created attachment 223291 [details] Striving for isosceles arrows, sometimes at the cost of the rounded corners
Jimmac wants the arrows to be right-angled when the rounded corners are skipped: https://raw.github.com/gnome-design-team/gnome-mockups/master/shell/bubble-arrow.png
Created attachment 223650 [details] [review] boxpointer: Avoid malformed boxpointer arrow
Comment on attachment 223208 [details] [review] boxpointer: Account for hanging actors Withdrawing it for the time being because we do not adjust the padding any more.
Review of attachment 223650 [details] [review]: Overall looks good. 2 comments: - even if it's not used right now I think it doesn't hurt to implement this for arrows on the left and right side; - we can at least fix one of the inaccuracies introduced here. This case: x |\____ we can just shift the whole box and keep the arrow form such that it actually points at x. ::: js/ui/boxpointer.js @@ +457,3 @@ + + let halfBase = Math.floor(arrowBase/2); + let halfBorder = borderWidth / 2; I think this would be a bit easier to read if you moved this hunk... @@ +460,3 @@ let halfMargin = margin / 2; let themeNode = this.actor.get_theme_node(); This was already here, but please remove this duplicated line. Either on this or other patch would be fine. @@ +465,3 @@ + let [x1, y1] = [halfBorder, halfBorder]; + let [x2, y2] = [natWidth - halfBorder, natHeight - halfBorder]; ... and this one... @@ +501,3 @@ + // always isosceles. + + let arrowOrigin; ... over here.
(In reply to comment #13) > Review of attachment 223650 [details] [review]: > > Overall looks good. 2 comments: > > - even if it's not used right now I think it doesn't hurt to implement this > for arrows on the left and right side; Done. > - we can at least fix one of the inaccuracies introduced here. This case: > > x > |\____ Done. > ::: js/ui/boxpointer.js > @@ +457,3 @@ > + > + let halfBase = Math.floor(arrowBase/2); > + let halfBorder = borderWidth / 2; > > I think this would be a bit easier to read if you moved this hunk... Done. > @@ +460,3 @@ > let halfMargin = margin / 2; > > let themeNode = this.actor.get_theme_node(); > > This was already here, but please remove this duplicated line. Either on this > or other patch would be fine. Done, in the same patch since we are already moving around things. > @@ +465,3 @@ > > + let [x1, y1] = [halfBorder, halfBorder]; > + let [x2, y2] = [natWidth - halfBorder, natHeight - halfBorder]; > > ... and this one... Done.
Created attachment 223670 [details] [review] boxpointer: Avoid malformed boxpointer arrow
Review of attachment 223670 [details] [review]: ACN with the comments below fixed. ::: js/ui/boxpointer.js @@ +313,3 @@ if (this._arrowSide == St.Side.TOP) { + if (skipTopLeft) { + cr.moveTo(x1, y1 + borderRadius); Y here has to be (y2 - borderRadius) because in the end the bottom left arc ends there and we would end up without the vertical left line. @@ +330,3 @@ + if (!skipTopRight) { + // top-right corner These comments are kind of needless now that we have these variables here. I think we can drop them. @@ +458,1 @@ + let halfMargin = margin / 2; This can also move below.
(In reply to comment #16) > Review of attachment 223670 [details] [review]: > > ACN with the comments below fixed. > > ::: js/ui/boxpointer.js > @@ +313,3 @@ > if (this._arrowSide == St.Side.TOP) { > + if (skipTopLeft) { > + cr.moveTo(x1, y1 + borderRadius); > > Y here has to be (y2 - borderRadius) because in the end the bottom left arc > ends there and we would end up without the vertical left line. Thanks for spotting it. Fixed. > @@ +330,3 @@ > > + if (!skipTopRight) { > + // top-right corner > > These comments are kind of needless now that we have these variables here. I > think we can drop them. Fixed. > @@ +458,1 @@ > + let halfMargin = margin / 2; > > This can also move below. Fixed.
commit 00f15c10754fc327802fdec9318b8a07491df7b4 Author: Debarshi Ray <debarshir@gnome.org> Date: Sat Sep 1 16:42:50 2012 +0200 boxpointer: Avoid malformed boxpointer arrow If the arrow's origin is so close to the edge that the arrow will not be isosceles, we try to compensate as follows: - We skip the rounded corner and settle for a right angled arrow as as shown below. |\_____ | | - If the arrow was going to be acute angled, we move the position of the box to maintain the arrow's accuracy. https://bugzilla.gnome.org/show_bug.cgi?id=680077
commit 10da35cbef64441d0c92c831d5a651d7e447f253 Author: Debarshi Ray <debarshir@gnome.org> Date: Thu Sep 6 20:59:58 2012 +0200 boxpointer: Clean up https://bugzilla.gnome.org/show_bug.cgi?id=680077