New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows: Add testnet link to installer #8285

Merged
merged 2 commits into from Jul 1, 2016

Conversation

Projects
None yet
4 participants
@laanwj
Member

laanwj commented Jun 29, 2016

Implements #8251

testnet

@jonasschnelli I'm not sure what tooling you used in 4c23743, but for some reason icotool, even though I've used the same sizes/bit depths as you, manages to produce a huge icon file:

  57964 Feb 18  2015 bitcoin.ico
 295606 Jun 29 13:01 bitcoin_testnet.ico
$ icotool -l bitcoin.ico > /tmp/bitcoin.lst
bitcoin.ico: clr_important field in bitmap should be zero
bitcoin.ico: clr_important field in bitmap should be zero
bitcoin.ico: clr_important field in bitmap should be zero
$ icotool -l bitcoin_testnet.ico > /tmp/bitcoin_testnet.lst
$ diff -du /tmp/bitcoin.lst /tmp/bitcoin_testnet.lst
[no output]
$ cat /tmp/bitcoin.lst
--icon --index=1 --width=48 --height=48 --bit-depth=4 --palette-size=16
--icon --index=2 --width=32 --height=32 --bit-depth=4 --palette-size=16
--icon --index=3 --width=16 --height=16 --bit-depth=4 --palette-size=16
--icon --index=4 --width=48 --height=48 --bit-depth=8 --palette-size=256
--icon --index=5 --width=32 --height=32 --bit-depth=8 --palette-size=256
--icon --index=6 --width=16 --height=16 --bit-depth=8 --palette-size=256
--icon --index=7 --width=256 --height=256 --bit-depth=32 --palette-size=0
--icon --index=8 --width=48 --height=48 --bit-depth=32 --palette-size=0
--icon --index=9 --width=32 --height=32 --bit-depth=32 --palette-size=0
--icon --index=10 --width=16 --height=16 --bit-depth=32 --palette-size=0

Comparing binary, it looks like it's not using PNG compression inside the icon.

It also looks like it's picking a smaller and lower-depth icon than it should.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 29, 2016

Member

utACK.
An optimized version of the testnet icon can be found here: jonasschnelli@6473d73

I'm using the OSX "Icon Slate" app.

Member

jonasschnelli commented Jun 29, 2016

utACK.
An optimized version of the testnet icon can be found here: jonasschnelli@6473d73

I'm using the OSX "Icon Slate" app.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 29, 2016

Member

Gah, seems I did something wrong while changing the color:
testnet2

What I did was take the 256x256 image and re-create all the other levels from that. But it looks like the smaller levels have relatively different amounts of whitespace around them? at least that would explain the differently-sized icon.

Member

laanwj commented Jun 29, 2016

Gah, seems I did something wrong while changing the color:
testnet2

What I did was take the 256x256 image and re-create all the other levels from that. But it looks like the smaller levels have relatively different amounts of whitespace around them? at least that would explain the differently-sized icon.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 29, 2016

Member

@laanwj: hmm..
I just re-created the testnet icon with the same properties like the mainnet icon: jonasschnelli@d188c85

Maybe this one is more in align.

Member

jonasschnelli commented Jun 29, 2016

@laanwj: hmm..
I just re-created the testnet icon with the same properties like the mainnet icon: jonasschnelli@d188c85

Maybe this one is more in align.

@MarcoFalke MarcoFalke added this to the 0.13.0 milestone Jun 29, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 29, 2016

Member

Unfortunately that one has a white block around it, and seems even smaller :(

testnet3

Member

laanwj commented Jun 29, 2016

Unfortunately that one has a white block around it, and seems even smaller :(

testnet3

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 29, 2016

Member

Pushed an icon that has the right properties and size - rotated the hue with 70 degrees for every level of the icon individually.

testnet4

Could use file size optimization though :)

Member

laanwj commented Jun 29, 2016

Pushed an icon that has the right properties and size - rotated the hue with 70 degrees for every level of the icon individually.

testnet4

Could use file size optimization though :)

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 30, 2016

Member

Tested ACK (on Windows 10).
bildschirmfoto 2016-06-30 um 14 52 36

Member

jonasschnelli commented Jun 30, 2016

Tested ACK (on Windows 10).
bildschirmfoto 2016-06-30 um 14 52 36

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jun 30, 2016

Member

Concept ACK, and the screenshots look good.

However, I don't understand how the second .lnk file is associated with the new icon.

Member

sipa commented Jun 30, 2016

Concept ACK, and the screenshots look good.

However, I don't understand how the second .lnk file is associated with the new icon.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 30, 2016

Member

Tested ACK (on Windows 10).

The new icon still needs optimization, would you be willing to do this?

However, I don't understand how the second .lnk file is associated with the new icon.

It's told to grab icon index 1 from the exe (index 0 is the default icon):

    CreateShortcut "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, @WINDOWS_BITS@-bit).lnk" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" "-testnet" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" 1

This icon is added in the resource:

IDI_ICON2 ICON DISCARDABLE "icons/bitcoin_testnet.ico"
Member

laanwj commented Jun 30, 2016

Tested ACK (on Windows 10).

The new icon still needs optimization, would you be willing to do this?

However, I don't understand how the second .lnk file is associated with the new icon.

It's told to grab icon index 1 from the exe (index 0 is the default icon):

    CreateShortcut "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, @WINDOWS_BITS@-bit).lnk" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" "-testnet" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" 1

This icon is added in the resource:

IDI_ICON2 ICON DISCARDABLE "icons/bitcoin_testnet.ico"
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 30, 2016

Member

The new icon still needs optimization, would you be willing to do this?

Sure. I guess this one should be fine:
https://bitcoin.jonasschnelli.ch/bitcoin_testnet.ico

Member

jonasschnelli commented Jun 30, 2016

The new icon still needs optimization, would you be willing to do this?

Sure. I guess this one should be fine:
https://bitcoin.jonasschnelli.ch/bitcoin_testnet.ico

windows: Add testnet icon for testnet link
Overhauled testnet icon by Jonas Schnelli
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 30, 2016

Member

Sure. I guess this one should be fine:

Added. Tested ACK, looks good now, and both icons are ~60kB.

Member

laanwj commented Jun 30, 2016

Sure. I guess this one should be fine:

Added. Tested ACK, looks good now, and both icons are ~60kB.

@laanwj laanwj merged commit 975a41d into bitcoin:master Jul 1, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jul 1, 2016

Merge #8285: windows: Add testnet link to installer
975a41d windows: Add testnet icon for testnet link (Wladimir J. van der Laan)
0ce8e99 windows: Add testnet link to installer (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #8285: windows: Add testnet link to installer
975a41d windows: Add testnet icon for testnet link (Wladimir J. van der Laan)
0ce8e99 windows: Add testnet link to installer (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Dec 27, 2017

Merge #8285: windows: Add testnet link to installer
975a41d windows: Add testnet icon for testnet link (Wladimir J. van der Laan)
0ce8e99 windows: Add testnet link to installer (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Dec 27, 2017

Move bitcoin.ico/png and bitcoin_testnet.ico/png one dir up
This also overwrites/dashifies icons/bitcoin_testnet.ico which was
introduced in Bitcoin #8285.

The icons were previously located in the drkblue theme directory while the
path used in dash.qrc was poining to the non-theme icons directory. Also,
the icons were never ported to the other themes. This commit moves them one
level up until someone actually ports these to the other themes (if ever
needed).

codablock added a commit to codablock/dash that referenced this pull request Dec 28, 2017

Merge #8285: windows: Add testnet link to installer
975a41d windows: Add testnet icon for testnet link (Wladimir J. van der Laan)
0ce8e99 windows: Add testnet link to installer (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Dec 28, 2017

Move bitcoin.ico/png and bitcoin_testnet.ico/png one dir up
This also overwrites/dashifies icons/bitcoin_testnet.ico which was
introduced in Bitcoin #8285.

The icons were previously located in the drkblue theme directory while the
path used in dash.qrc was poining to the non-theme icons directory. Also,
the icons were never ported to the other themes. This commit moves them one
level up until someone actually ports these to the other themes (if ever
needed).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment