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 768317 - Fix various glitches in OSD window
Fix various glitches in OSD window
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 768330 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-07-02 15:54 UTC by Florian Müllner
Modified: 2016-07-05 18:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
osdWindow: Fix level bar width (1.72 KB, patch)
2016-07-02 15:54 UTC, Florian Müllner
committed Details | Review
osdWindow: Fix blurry level bar (1.07 KB, patch)
2016-07-02 15:54 UTC, Florian Müllner
committed Details | Review
osdWindow: Use a constraint to enforce ratio (4.67 KB, patch)
2016-07-02 15:54 UTC, Florian Müllner
committed Details | Review

Description Florian Müllner 2016-07-02 15:54:36 UTC
Some fallout from bug 758471 ...
Comment 1 Florian Müllner 2016-07-02 15:54:42 UTC
Created attachment 330792 [details] [review]
osdWindow: Fix level bar width

Commit 9b07ce1d0d changed the OSD window's level bar to be a regular
actor instead of a custom drawn bar. The bar actor's width depends on
both the configured level (e.g. 40%) and the available width, however
the width is currently only updated when the configured level changes.
Fix that by properly considering changes to the parent's width as well.
Comment 2 Florian Müllner 2016-07-02 15:54:48 UTC
Created attachment 330793 [details] [review]
osdWindow: Fix blurry level bar

ClutterActor:width is a floating point property, so it will not be
automatically rounded to non-fractional values that properly align
to pixels. To fix the resulting blurriness, add explicit rounding.
Comment 3 Florian Müllner 2016-07-02 15:54:54 UTC
Created attachment 330794 [details] [review]
osdWindow: Use a constraint to enforce ratio

Commit 9b07ce1d0d broke the code that keeps the OSD window square.
Use that opportunity to move away from the hack of setting the
min-height style property from code and adjusting the width on
allocate, and implement a proper constraint instead.
Comment 4 Florian Müllner 2016-07-03 10:20:19 UTC
*** Bug 768330 has been marked as a duplicate of this bug. ***
Comment 5 Rui Matos 2016-07-05 15:55:37 UTC
Review of attachment 330793 [details] [review]:

::: js/ui/osdWindow.js
@@ +41,2 @@
         let alloc = this.actor.get_allocation_box();
+        let newWidth = Math.floor((alloc.x2 - alloc.x1) * this._level / 100);

why not round() ? don't we take the risk of ending up 1 pixel short of a full bar when the level is 100 this way?
Comment 6 Rui Matos 2016-07-05 15:55:50 UTC
Review of attachment 330792 [details] [review]:

looks fine
Comment 7 Rui Matos 2016-07-05 16:38:18 UTC
Review of attachment 330794 [details] [review]:

got a bit puzzled until realizing the MonitorConstraint applies to the parent of the box this constraint applies to

anyway, looks good

::: js/ui/osdWindow.js
@@ +65,3 @@
+        // so add it to our minimum size
+        let minSize = this._minSize + actor.margin_top + actor.margin_bottom;
+        let [width, height] = actorBox.get_size();

this is the min-width/min-height set from the osd-window css class right? perhaps a small comment here to make that obvious
Comment 8 Rui Matos 2016-07-05 16:45:00 UTC
Review of attachment 330794 [details] [review]:

hang on, this patch actually makes the level bar blurry on this 1366x768 monitor, not sure why though
Comment 9 Rui Matos 2016-07-05 16:57:50 UTC
Review of attachment 330794 [details] [review]:

now it doesn't anymore, glitch in the matrix I suppose
Comment 10 Florian Müllner 2016-07-05 17:17:24 UTC
(In reply to Rui Matos from comment #5)
> why not round() ? don't we take the risk of ending up 1 pixel short of a
> full bar when the level is 100 this way?

I don't think so, if this._level is 100, we should multiply by one either way. But then I neither think that round() is wrong, so changed.
Comment 11 Florian Müllner 2016-07-05 17:22:05 UTC
(In reply to Rui Matos from comment #7)
> this is the min-width/min-height set from the osd-window css class right?

At least, but likely more on most displays. It's something like [Math.min(min-width, icon-size + horizontal padding), Math.min(min-height, icon-size + vertical padding)] (where the icon size depends on the monitor dimensions, see _monitorsChanged).
Comment 12 Florian Müllner 2016-07-05 18:50:20 UTC
Attachment 330792 [details] pushed as 775187b - osdWindow: Fix level bar width
Attachment 330793 [details] pushed as 424fa01 - osdWindow: Fix blurry level bar
Attachment 330794 [details] pushed as 128697d - osdWindow: Use a constraint to enforce ratio