GNOME Bugzilla – Bug 656335
One-pixel border bugs in the overview
Last modified: 2011-08-29 23:22:29 UTC
The positioning and picking of windows in the overview is incorrect with a large draggable border width. Unfortunately, this bug is harder to solve than I'd like. MetaWindowActor needs to be in sync with the frame X window is. The easiest way to do this is to assume that the padding is in the calculations, so that the top-left of the actor is the top-left of the invisible border section. But this competes with the overview's needs. What I'd really want is a way to crop the excess padding by directly setting a property on the WindowClone's actor: { 10, 10, width-20, height-20 } I've tried various combinations of get_preferred_width/height vfuncs and the anchor point, but they've all failed. Additionally, you can click *outside* of the visible area, and it will treat it as a click on the window.
Created attachment 194754 [details] [review] MetaWindowActor: Accommodate for invisible borders blah this needs a better commit message Just a starting point, here. This fixes MWA sizing in the overview, but causes trails to the right and bottom of the window.
Created attachment 194755 [details] [review] workspace: Clone the MetaWindowActor, not its MetaShapedTexture Because of the effects invisible borders had upon MetaWindowActor and MetaShapedTexture, to get proper positioning and sizing we need to use the MetaWindowActor instead of its MetaShapedTexture.
Created attachment 194851 [details] [review] MetaWindowActor: Accommodate for invisible borders This corrects the sizing and positioning in the overview, and makes the MetaWindowActor's bounds effectively the same as the outer rect of the window -- that is, the full X window rect minus the invisible borders. corrected the artifacts
Created attachment 195079 [details] [review] MetaWindowActor: Accommodate for invisible borders This corrects the sizing and positioning in the overview, and makes the MetaWindowActor's bounds effectively the same as the outer rect of the window -- that is, the full X window rect minus the invisible borders.
Created attachment 195081 [details] [review] MetaWindowActor: Don't use uninitialized frame borders Just a minor memory fixup.
Review of attachment 195079 [details] [review]: Looks good to me, except that the commit message "This corrects the sizing and positioning in the overview" doesn't make much sense for a mutter patch. And I don't really understand the Subject either. Subject: MetaWindowActor: size the actor to the visible region Exclude the invisible border from the area that the Clutter actor occupies; this makes handling MetaWindowActor more convenient for users of libmutter.
Review of attachment 195081 [details] [review]: Yeah
Created attachment 195086 [details] [review] overview: Compensate for the window's invisible borders let's try this
Comment on attachment 195081 [details] [review] MetaWindowActor: Don't use uninitialized frame borders Attachment 195081 [details] pushed as 8c74ad1 - MetaWindowActor: Don't use uninitialized frame borders
Review of attachment 195086 [details] [review]: Doesn't this have to handle the size changing?
Created attachment 195129 [details] [review] overview: Compensate for the window's invisible borders
Review of attachment 195129 [details] [review]: In addition to the use of prefs.get_draggable_border_width() noted on irc couple of other small things: ::: js/ui/workspace.js @@ +99,3 @@ + let clone = new Clutter.Clone({ source: realWindow.get_texture(), + x: -border, + y: -border }); I'd like to see a comment above the whole thing, along the lines of: /* The MetaShapedTexture that we clone has a size that includes * the invisible border; this is inconvenient; rather than trying * to compensate all over the place we insert a ClutterGroup into * the heirarchy that is sized to only the visible portion. */ @@ +190,3 @@ + let rect = realWindow.metaWindow.get_outer_rect(); + this.actor.set_size(rect.width, rect.height); + this.emit('size-changed'); Maybe update the position of of the child here too to be a little more resiliant about changes in the invisible border width at runtime? (Not very concerned about being 100% correct for that in all cases.)
Created attachment 195131 [details] [review] overview: Compensate for the window's invisible borders
Created attachment 195134 [details] [review] overview: Compensate for the window's invisible borders
Review of attachment 195134 [details] [review]: Just a few comments ::: js/ui/workspace.js @@ +108,3 @@ + + let outerRect = realWindow.meta_window.get_outer_rect(); + this.actor = new Clutter.Group({ reactive: true, Would still like to see a comment here somewhere describing what this group is about - something like the comment in my last review. @@ +191,3 @@ + // We need to adjust the position of the actor because of the + // consequences -- in reality, the texture has an extra set + // of "padding" around it that we need to trim down. I'm not sure what "the consequences" are here, doesn't make sense to me. We need to adjust the position of the actor to exclude invisible parts of the MetaShapedTexture that are there only so the user can drag on them for resizing. Maybe? @@ +193,3 @@ + // of "padding" around it that we need to trim down. + + // The outer rect paradoxially is the smaller rectangle, paradoxically @@ +208,3 @@ + let [borderX, borderY] = this._getInvisibleBorderPadding(); + let outerRect = this.metaWindow.get_outer_rect(); + this.actor.set_size(rect.width, rect.height); outRect/rect
Created attachment 195135 [details] [review] overview: Compensate for the window's invisible borders
Attachment 195135 [details] pushed as e5bc3a2 - overview: Compensate for the window's invisible borders Closing bug, but I'd really like to get the other patch into mutter for "correctness". Not today, though.