Qt: HiDPI (retina) support for splash screen #5228

jonasschnelli commented at 3:37 PM on November 6, 2014: contributor

Currently the splash screen is not optimized for retina/HiDPI screens. This changeset will enable high-resolution background images and text drawing. This should also work on other platforms than OSX as soon as they support HiDPI.

in src/qt/bitcoin.cpp:None in 9a2d4709df outdated

 438 | @@ -439,6 +439,7 @@ void BitcoinApplication::initializeResult(int retval)
 439 |          {
 440 |              window->show();
 441 |          }
 442 | +        sleep(3);

luke-jr commented at 3:43 PM on November 6, 2014:

?

jonasschnelli commented at 3:49 PM on November 6, 2014:

You know how to review.. :) This is/was for testing purposes only. Will Amend-Commiting this away right now.

jonasschnelli force-pushed on Nov 6, 2014

laanwj commented at 9:05 AM on November 7, 2014: member

ACK on the concept.

However: 2x as well as 1x splash images? We could save a lot of executable space by doing a bit of scaling. (same could be said for recoloring for the testnet/mainnet case, but ok)

jonasschnelli force-pushed on Nov 7, 2014

jonasschnelli commented at 9:12 AM on November 7, 2014: contributor

@laanwj With the 2x splash screen i followed apples recommendation. They prefer higher binaries with less cpu usage (by down-scaling) and downscaling can sometime gives bad result. Good would be to dynamically build the splash screen with the already available icons. I try now to get this working without loosing quality. And you might remember: the splash screens background is not a #fff, there is some slightly gradient. :)

laanwj commented at 9:42 AM on November 7, 2014: member

Nah, downscaling an image is very fast, especially if it's by a factor of two. I prefer executable size over less CPU usage in this case. This is a cross-platform project and we're not following Apple's guidelines to the letter.

laanwj added the label GUI on Nov 7, 2014

jonasschnelli force-pushed on Nov 7, 2014

jonasschnelli commented at 8:30 PM on November 7, 2014: contributor

@laanwj did some changes: the splashscreen now get completely puzzled together. I had to place higher resolution PNGs for mainnet and testnet icons. Needs testing on Win. Testen on OSX 10.10 and Ubuntu 10.4.

theuni commented at 8:56 PM on November 7, 2014: member

@jonasschnelli Is it not possible to set some color mask rather than duplicating the PNGs? Edit: Heh, @laanwj already mentioned that. +1 to the idea.

jonasschnelli commented at 9:13 PM on November 7, 2014: contributor

@theuni: i did research but then stopped because i though the tray and window icon might need a file based icon. But your right, it should be possible. Maybe the colorize quality is not as good as it comes from Photoshop but still okay for devs (who else needs the testnet!)

Let me try...

jonasschnelli force-pushed on Nov 9, 2014

jonasschnelli force-pushed on Nov 10, 2014

in src/qt/splashscreen.cpp:None in 53be4a6a59 outdated

  47 | +    // create a bitmap according to device pixelration
  48 | +    QSize splashSize(480*devicePixelRatio,320*devicePixelRatio);
  49 | +    pixmap = QPixmap(splashSize);
  50 | +
  51 | +#if QT_VERSION > 0x050100
  52 | +    // change to HiDPI if it makes sens

fanquake commented at 1:25 PM on November 10, 2014:

s/sens/sense

jonasschnelli commented at 1:26 PM on November 10, 2014:

thanks! My english is just so bad.

in src/qt/splashscreen.cpp:None in 53be4a6a59 outdated

  42 | @@ -38,12 +43,34 @@ SplashScreen::SplashScreen(Qt::WindowFlags f, const NetworkStyle *networkStyle)
  43 |      QString titleAddText    = networkStyle->getTitleAddText();
  44 |  
  45 |      QString font            = "Arial";
  46 | +    
  47 | +    // create a bitmap according to device pixelration

fanquake commented at 1:25 PM on November 10, 2014:

s/pixelration/pixelratio

jonasschnelli force-pushed on Nov 10, 2014

Qt: HiDPI (retina) support for splash screen

- remove splash screen images (reduce binary size)
- dynamicly draw splash screen with available icon.
- remove testnet icon
- dynamicly colorize testnet icon

c9b0ed4552

jonasschnelli force-pushed on Nov 10, 2014

jonasschnelli commented at 1:39 PM on November 10, 2014: contributor

After hard working, i got the splashscreen and also the tray/window icon working by dynamically changing the color of the original icon. With this commit, we get rid of the splashscreen PNG (both net) and the testnet icon. Together we might save around 150kb in the distributed binary.

Tested on OSX10.10 and Ubuntu 10.4 It would help if someone could test this on windows.

in src/qt/networkstyle.cpp:None in c9b0ed4552 outdated

  10 | @@ -11,22 +11,22 @@
  11 |  static const struct {
  12 |      const char *networkId;
  13 |      const char *appName;
  14 | -    const char *appIcon;
  15 | +    const int iconColorHueShift;

laanwj commented at 4:48 PM on November 10, 2014:

Ha iconColorHueShift, very cool

laanwj commented at 5:07 PM on November 10, 2014: member

Works for me (Ubuntu 14.04)

in src/qt/networkstyle.cpp:None in c9b0ed4552 outdated

 108 | +                scL[x] = col.rgba();
 109 | +            }
 110 | +        }
 111 | +
 112 | +        //convert back to QPixmap
 113 | +        pixmap.convertFromImage(img);

laanwj commented at 12:09 PM on November 13, 2014:

Shouldn't we cache these or such? Looks to me that calling this every time results in unnecessary overhead.

laanwj commented at 12:11 PM on November 13, 2014:

BTW you are reimplementing part of the QIcon functionality here. See QIcon.pixmap(QSize). Apart from the hue-shift part, but that could be done once on instantiation?

jonasschnelli commented at 12:25 PM on November 13, 2014:

1)Cache do you think kind of a file cache for the resized image? The hue shifted testnet icon could also be cached, but it's kinda dev only things where some used cputicks maybe are tolerated. But filecaching it brings us a little bit back to the apple recommended @2x strategy. Maybe the distributed binary is not unnecessary bloated, but we use disk space (very minor indeed). I think the rescaling from 1024 to 256 does perform very fast and implementing a cache with correct invalidating mechanism (for possible upcoming icon changes) might need some time. But tell me if I'm sounding lazy, then i go behind it.

Reimplementing QIcon.pixmap Ah. Now i see what you mean. Yes. I try to optimize this. It was more or less "organically grown" and than i oversaw the quick way resizing the standard icon. Thanks for that.

laanwj commented at 12:37 PM on November 13, 2014:

@jonasschnelli Nonono, a file cache shouldn't be necessary. I just like to avoid doing the same scaling multiple times within the same invocation of Bitcoin-Qt. I think QIcon already implements such functionality to maintain the image at multiple sizes, so there would be no need for us to do anything complex here.

jonasschnelli commented at 12:47 PM on November 13, 2014:

Ah, right. On windows and posix there are two invocations with the same size (windowIcon, tryIcon). Let me implement a simple mem-cache or see what Qt provides here.

laanwj commented at 12:01 PM on November 14, 2014: member

@jonasschnelli simply using QIcon simplifies the code a lot, see here: https://github.com/laanwj/bitcoin/tree/2014_11_jonasschnelli_hidpi

jonasschnelli commented at 12:17 PM on November 14, 2014: contributor

@laanwj nice. Yes. I try to adapt this. I went for this solution because the window and tray icon cannot be greater than 256x256. I increased the size of the bitcoin.png icon to 1024x1024. Setting this size as tray or window icon will end up in having no icon on my ubuntu vm. And i thought scale first, than change HSL (cpu whise). Your solution looks clean. I take it and change it to ensure that it also works for tray and window icon.

laanwj commented at 1:35 PM on November 14, 2014: member

Your solution looks clean. I take it and change it to ensure that it also works for tray and window icon.

That's weird. I've had no problems with the tray and window icon here on Ubuntu. The whole purpose of QIcon is that any size can be requested, the client code shouldn't request 1024x1024 if it can't handle that. Maybe MacOSX specific.

Qt: Go back to using QIcon functionality for scaling 572832239b

resize tray icon because a 1024x1024 icon won't show in ubuntu (bug) b74fa7cad9

in src/qt/bitcoingui.cpp:None in 99899d8070 outdated

 494 | @@ -495,7 +495,9 @@ void BitcoinGUI::createTrayIcon(const NetworkStyle *networkStyle)
 495 |      trayIcon = new QSystemTrayIcon(this);
 496 |      QString toolTip = tr("Bitcoin Core client") + " " + networkStyle->getTitleAddText();
 497 |      trayIcon->setToolTip(toolTip);
 498 | -    trayIcon->setIcon(networkStyle->getAppIcon());
 499 | +
 500 | +    QIcon resizedIcon(networkStyle->getAppIcon().pixmap(QSize(256,256)));
 501 | +    trayIcon->setIcon(resizedIcon);

jonasschnelli commented at 4:23 PM on November 14, 2014:

for me it only works with this resize. Otherwise i have the following tray icon in Ubuntu:

unbenannt-2

laanwj commented at 10:13 AM on November 19, 2014:

Wow :/ I wonder if this could be solved by explicitly adding a 256x256 scaled bitmap to the icon in the NetworkStyle::NetworkStyle constructor using void QIcon::addPixmap ( const QPixmap & pixmap ). That at least moves the code away from here.

jonasschnelli force-pushed on Nov 19, 2014

jonasschnelli commented at 1:27 PM on November 19, 2014: contributor

Updated once again with keeping the 256x256 optimized icon in the NetworkStyles.

in src/qt/networkstyle.cpp:None in b74fa7cad9

  79 | +        //convert back to QPixmap
  80 | +        pixmap.convertFromImage(img);
  81 | +    }
  82 | +
  83 | +    appIcon             = QIcon(pixmap);
  84 | +    trayAndWindowIcon   = QIcon(pixmap.scaled(QSize(256,256)));

laanwj commented at 1:47 PM on November 19, 2014:

It doesn't work if you add both of these pixmaps to the same QIcon, as I suggested?

jonasschnelli commented at 8:42 AM on November 21, 2014:

I tested the following some minutes ago:

Addad a 256,256 pixmap to the appIcon with: appIcon.addPixmap(pixmap.scaled(QSize(256,256)));

But this does not fix the Ubuntu-No-Tray-Icon-Error (see screenshot above).

laanwj commented at 8:56 AM on November 21, 2014:

This is really strange. Anyhow, current solution is fine then, thanks for testing!

jonasschnelli commented at 12:13 PM on November 26, 2014: contributor

As #5219 it might be flagged with 0.11 milestone.

laanwj added this to the milestone 0.11.0 on Nov 26, 2014

laanwj referenced this in commit bb3c75bdf4 on Dec 16, 2014

laanwj commented at 10:16 AM on December 16, 2014: member

Merged via bb3c75bdf48cf04f97fa84f494f7366fac99e379

laanwj closed this on Dec 16, 2014

jonasschnelli referenced this in commit 270f42d39f on Dec 16, 2014

laanwj referenced this in commit 28a274e6ce on Dec 16, 2014

MarcoFalke locked this on Sep 8, 2021