Skip to content

windows: Add testnet link to installer #8285

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 1, 2016
Merged

windows: Add testnet link to installer #8285

merged 2 commits into from
Jul 1, 2016

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Jun 29, 2016 •
edited
Loading

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
Copy link
Contributor

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

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

@laanwj
Copy link
Member Author

laanwj commented Jun 29, 2016 •
edited
Loading

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
Copy link
Contributor

@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.

@maflcko maflcko added this to the 0.13.0 milestone Jun 29, 2016
@laanwj
Copy link
Member Author

laanwj commented Jun 29, 2016

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

testnet3

@laanwj laanwj force-pushed the 2016_06_testnet_link_windows branch from be7cd90 to e3bc4ed Compare June 29, 2016 16:34
@laanwj
Copy link
Member Author

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
Copy link
Contributor

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

@sipa
Copy link
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
Copy link
Member Author

laanwj commented Jun 30, 2016 •
edited
Loading

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
Copy link
Contributor

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

@laanwj
windows: Add testnet icon for testnet link
975a41d 
Overhauled testnet icon by Jonas Schnelli
@laanwj laanwj force-pushed the 2016_06_testnet_link_windows branch from e3bc4ed to 975a41d Compare June 30, 2016 15:18
@laanwj
Copy link
Member Author

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
laanwj added a commit that referenced this pull request Jul 1, 2016
@laanwj
Merge #8285: windows: Add testnet link to installer
da50997 
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)
@laanwj laanwj mentioned this pull request Jul 1, 2016
Closed
codablock pushed a commit to codablock/dash that referenced this pull request Dec 28, 2017
@laanwj @codablock
Merge bitcoin#8285: windows: Add testnet link to installer
dcf36df 
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
@codablock
Move bitcoin.ico/png and bitcoin_testnet.ico/png one dir up
f8a7a2c 
This also overwrites/dashifies icons/bitcoin_testnet.ico which was
introduced in Bitcoin 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).
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
@codablock @andvgal
Move bitcoin.ico/png and bitcoin_testnet.ico/png one dir up
18807bc 
This also overwrites/dashifies icons/bitcoin_testnet.ico which was
introduced in Bitcoin 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).
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Build system Windows
Projects
None yet
Milestone
0.13.0
Development

Successfully merging this pull request may close these issues.
4 participants
@laanwj @jonasschnelli @sipa @maflcko