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 786145 - Folders' subicons not rendered properly on Hi DPI displays
Folders' subicons not rendered properly on Hi DPI displays
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: overview
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2017-08-11 09:02 UTC by Mario Sánchez Prada
Modified: 2017-08-11 10:49 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Explicitly set the width and height for subicons' containers in folders (1.53 KB, patch)
2017-08-11 09:08 UTC, Mario Sánchez Prada 		none Details | Review
Explicitly set the width and height for subicons' containers in folders (1.88 KB, patch)
2017-08-11 10:18 UTC, Mario Sánchez Prada 		accepted-commit_now Details | Review

Description Mario Sánchez Prada 2017-08-11 09:02:50 UTC 
On High DPI displays where the current resolution changes the value of scaling-factor to something > 1 (i.e. 2 in my 13" 3200x1800 display), the 4 subicons that are part of the folders are incorrectly rendered:

Instead of taking 40% of the folder icon's real state each, they are rendered so that each of them take 100% of the folder icon's real state, meaning that only one is rendered "inside" the folder, and the other ones are rendered outside of it.
Comment 1 Mario Sánchez Prada 2017-08-11 09:08:48 UTC 
Created attachment 357398 [details] [review]
Explicitly set the width and height for subicons' containers in folders

This is the patch we landed in Endless OS and fixed the problem for us there. Unfortunately, I don't have upstream GNOME running on a High DPI display so I could not fully test this locally.

However, I did `gsettings set org.gnome.desktop.interface scaling-factor 2` on my Fedora 26 VM to test the patch and it seems to fix the very same problem there, so I'm proposing it here as well, as it's a simple patch anyway.
Comment 2 Florian Müllner 2017-08-11 09:56:50 UTC 
Review of attachment 357398 [details] [review]:

::: js/ui/appDisplay.js
@@ +1176,3 @@
             if (i < numItems) {
                 let texture = this._allItems[i].app.create_icon_texture(subSize);
+                bin = new St.Bin({ child: texture, width: subSize, height: subSize });

Mmh, I really would expect the bin to pick up the child's size in this case. But anyway, if we are already setting an explicit size, the code could be rewritten more concisely as:

  let bin = new St.Bin({ width: subSize, height: subSize });
  if (i < numItems)
      bin.child = this._allItems[i].app.create_icon_texture(subSize);
  layout.attach(bin, ...);
Comment 3 Mario Sánchez Prada 2017-08-11 10:18:13 UTC 
Created attachment 357402 [details] [review]
Explicitly set the width and height for subicons' containers in folders

(In reply to Florian Müllner from comment #2)
> Review of attachment 357398 [details] [review] [review]:

Thanks for the review, see the newly attached patch addressing your comments (which I should have done already)
Comment 4 Florian Müllner 2017-08-11 10:23:25 UTC 
Review of attachment 357402 [details] [review]:

LGTM