new splash screen by jonasschnelli · Pull Request #2495 · bitcoin/bitcoin

jonasschnelli · 2013-04-09T13:26:09Z

why:

the current splash-screen has no referring to official images on https://en.bitcoin.it/wiki/Promotional_graphics
the current splash screen only exists in a low res jpg
current splash screen looks dark and "hackish"
new splash screen should generate positive, "trust-emotions".
new splash screen gives the user infos about the running client.
new splash screen can handle long messages (in a lot of languages the text is cropped in current release)
example: https://dl.dropbox.com/u/7383846/new_bitcoin_splash.png
new size (x2) 400x312
contains textual information about the client
textinfos are dynamicly written to the image
when -testnet is switch on, the splashscreen will show the bitcoin logo in testnet-color (as well as a text [testnet])

jonasschnelli · 2013-04-09T13:27:25Z

If the new splash screen will be accepted, i will update the build process so that the splash.png text (version, copyright) gets automatically "written" with imagemagick like the gui about screen.

laanwj · 2013-04-09T14:13:52Z

I like the splash screen. However we should add the version and copyright
information text programmatically from qt. It does not need to be in the
image. Otherwise there's yet another place to update copyright and version
numbers.

Diapolo · 2013-04-09T15:00:12Z

src/qt/bitcoin.qrc

Please don't remove that tag.

Oops. was auto-removed by Qt Creator. Now readded in d33ff50.

Diapolo · 2013-04-09T15:02:20Z

@laanwj ACK, I thought the same. We don't want to update that screen with every new year or relase.

@jonasschnelli Looks very nice, I like it.

jonasschnelli · 2013-04-09T19:06:52Z

@laanwj @Diapolo as a coder, i would also recommend the option of placing the text on the splashscreen by code. As a designer i would avoid this. Why: text placed by Qt will look much more sharp and somehow crispy. Text placed as image on a template image by imagemagick (or other command-line capable gfx tool) will look much better and can use non-standard fonts.

It might sounds crazy for you (coders), but in my eyes, the splash screen is the first contact with the enduser and when it come to the point where the Bitcoin-Qt client gets "mainstream", first contact is very important. That's why i would go with the pre-generated png in the build process with imagemagick.

It's more work for us, but more quality for the enduser. And i kind of like this.

laanwj · 2013-04-09T19:47:14Z

Qt supports various text rendering options as well. And TBH it's our time that is very limited at the moment, I really don't mind text quality to be somewhat less for that.

laanwj · 2013-04-09T19:50:37Z

Maybe there's another option: make building the image part of Bitcoin-Qt's build process. That makes it automatic, which is fine with me as well.
Non-standard fonts be a problem in any case though: everyone needs to be able to build the image, so it can't rely on certain fonts to be installed.

jonasschnelli · 2013-04-09T19:56:48Z

@laanwj yes. Include into bitcoin-qt's build process.
Font: i would just place a ttf or otf file (open source fonts) into the qt/src folder. The font must not be installed on the build-system.

laanwj · 2013-04-09T20:04:04Z

@Diapolo would calling imagemagick in the build process work on windows? I suppose it'd be more difficult...

jonasschnelli · 2013-04-10T06:39:05Z

@laanwj i thought the same. Imagemagick is probably a overkill. Has also huge dependencies (Ghostscript, freetype). And yes: windows user would hate me. ;)

I think I try to create a solution with qt only (runtime).
Let me try to play with http://qt-project.org/doc/qt-5.0/qtgui/qrawfont.html#alphaMapForGlyph.

Will push something soon.

gmaxwell · 2013-04-10T06:43:56Z

meh. I love the image, but it seems like such a small thing to need to invoke imagemagick for... I'm sure that QT can be made to do this.

laanwj · 2013-04-10T07:13:48Z

Agreed @ gmaxwell. Especially as Qt has a quite advanced rendering backend
(QPainter). I daresay it can do most that imagemagick can do,
rendering-wise.

Let's do this the proper way :)

Diapolo · 2013-04-10T10:50:42Z

@laanwj I've never heard of imagemagick, but as we want to do it the Qt way I won't even google it ^^.

jonasschnelli · 2013-04-10T14:00:54Z

Just pushed some code which places the infos by Qt. In development, the version string is much longer and a smaller font will be used. Releases would look like this (1:1 screen from the splash screen drawing by qt):

Question:

Where should we place the 2013 (© end year)?
Is there a nice way to get the app name dynamically?

Could someone test this on Qt4.7/4.8?

Diapolo · 2013-04-10T14:07:49Z

@jonasschnelli You could use QApplication::applicationName() for the name, but be careful, as it would be Bitcoin-Qt-testnet for testnet currently.

Diapolo · 2013-04-10T14:14:46Z

src/qt/bitcoin.cpp

Currently we have ABOUTDIALOG_COPYRIGHT_YEAR in aboutdialog.cpp, which perhaps could be moved to another location for this to be usable.

Yes. I saw it. Is there a nice location for this const?

Perhaps it could be added to clientversion.h, but I'm not sure if we should use that directly in bitcoin.cpp. @laanwj What do you think?
If we would include it, you also had options to generate the version string without the client model (which seems bad, because we HAVE it in clientmodel to not access that stuff directly AFAIK).

Edit: Is it possible to use tr("Copyright") + QString(" © ") + tr("2009-%1 The Bitcoin developers").arg(2013), because we should allow translation and had problems with that copyright sign, if we don't use the HTML tag for it :).

Agreed. Clientversion.h sounds like the right place, then all the version
and year stuff can be updated in the same place.

@jonas won't including two large splash images, grow the executable quite a
lot? If so, it's not really worth it for the testnet imo; and putting
"testnet" in the text is enough.

jonasschnelli · 2013-04-10T14:29:41Z

App name is now dynamic ("-testnet" will be replaced with " (tn)" for a better style).
When on testnet, the splash screen contains also a "green" bitcoin logo.

Diapolo · 2013-04-10T14:42:12Z

@jonasschnelli We currently use [testnet] appended to Bitcoin-Qt to indicate testnet usage (see bitcoingui.cpp -> setWindowTitle(windowTitle() + QString(" ") + tr("[testnet]"));), which I would like to keep (with equality and translation stuff in my mind).

Diapolo · 2013-04-10T14:43:00Z

src/qt/bitcoin.qrc

You also once more will need to mention them in the assets file :D.

Oh.. yeah. Comes also.

jonasschnelli · 2013-04-10T14:52:51Z

@laanwj the testnet splash png is 45.37kb. When you compare it against the blockchain size... but yes: it will increase the bin size. I still recommend to have it (the new testnet splash).

Diapolo · 2013-04-10T14:58:35Z

I also like the new testnet splash :) and 46KB is fine with me!

sipa · 2013-04-10T15:01:49Z

I'm fine with 46kB extra.

laanwj · 2013-04-10T15:33:57Z

46k is much less than I had estimated, I'm fine with that.

qubez · 2013-04-10T17:51:49Z

splash screen in <3k:

(I <3 Bitcoin)

Diapolo · 2013-04-11T06:46:14Z

@jonasschnelli Can you please squash all commits into one after we have the final ACK for this :).

Diapolo · 2013-04-11T07:05:32Z

src/qt/bitcoin.cpp

Can you try if QString(" ") + _("[testnet]") instead of QString(" [testnet]") works please. We have [testnet] already translated [testnet] is a NEW string and for that reason currently untranslated.

For sure. " [testnet]" is a bad label. Will change that.

why: - the current splash-screen has no referring to official images on https://en.bitcoin.it/wiki/Promotional_graphics - the current splash screen only exists in a low res jpg - current splash screen looks dark and "hackish" - new splash screen should generate positive, "trust-emotions". - new splash screen gives the user infos about the running client. - new splash screen can handle long messages (in a lot of languages the text is cropped in current release) - example: https://dl.dropbox.com/u/7383846/new_bitcoin_splash.png - new size (x2) 400x312 - contains textual information about the client - textinfos are dynamicly written to the image - when -testnet is switch on, the splashscreen will show the bitcoin logo in testnet-color (as well as a text [testnet]) Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>

jonasschnelli · 2013-04-11T12:56:20Z

squashed and ready to test on Qt4.8.
Anyone?

jonasschnelli · 2013-04-11T12:56:44Z

Pulltester might also do a check...

…lash_screen Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch> Conflicts: doc/assets-attribution.txt

fanquake · 2013-04-11T13:27:01Z

@jonasschnelli I can test for you. Have one comp running 10.7.5

jonasschnelli · 2013-04-11T13:28:58Z

@fanquake do you can build from the source (take master and pull from jonasschnelli/bitcoin new_splash_screen)?

laanwj · 2013-04-11T13:52:45Z

src/qt/bitcoin.cpp

I don't think \uXXXX will work in non-gcc compilers such as MSVC. Better use QChar(0xA9) + QString(" ...") .

sorry for my lack of non-gcc experience. Fixed now.

Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>

BitcoinPullTester · 2013-04-11T15:02:59Z

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/66fa5cbaefc7bbf0a5ac48303d72818088c9f04e for binaries and test log.

This could happen for one of several reasons:

It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (you can find the patches applied at test-time at http://jenkins.bluematt.me/pull-tester/files/patches/)
It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
It does not build on either Linux i386 or Win32 (via MinGW cross compile)
The test suite fails on either Linux i386 or Win32
The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

jonasschnelli · 2013-04-11T15:06:57Z

Oh...
Hmm.. "src/qt/bitcoin.cpp:205: error: 'COPYRIGHT_YEAR' was not declared in this scope"
Wasn't it merged in a PR to the master in #2503?

Diapolo · 2013-04-11T15:12:22Z

You can rebase to current master and update this pull, perhaps @BitcoinPullTester was doing it's work with a not up-to-date version :).

Diapolo · 2013-04-11T15:13:36Z

doc/assets-attribution.txt

Looks like an indention via tab ;).

sipa · 2013-04-11T17:32:34Z

I like the splash screen image :)

Diapolo · 2013-04-11T17:53:07Z

Same here, I really love it, great looking! Don't think my many comments lower that feeling :) @jonasschnelli .

laanwj · 2013-04-12T03:44:47Z

src/qt/bitcoin.cpp

Code-wise, I would prefer if you move this stuff to a function or class. The main is already pretty cluttered. Personally I prefer a main function that consists of function/method calls, not logic and rendering code in itself.

Let me inherit the QSplashScreen and write it more proper to reduce bitcoin.cpp size.

jonasschnelli · 2013-04-12T08:45:36Z

@Diapolo my contributions are for the blockchain and not for my ego. :) so keep on finding details to make it better!

sipa · 2013-04-12T20:58:43Z

Seems to work fine. I haven't checked the code changes, but can you at least squash them?

jonasschnelli · 2013-04-13T11:56:56Z

@sipa: will finish the splash screen soon (1-2 days) then i try to squash. I once pulled/updates from master, ... i think i can't squash over the merge of the master? Can i?

laanwj · 2013-04-13T12:37:46Z

@jonasschnelli it is possible, but more difficult, and not simply with git rebase -i (I think. That's why you should ideally not merge in these cases, but always rebase); easiest may be to start from a new branch with master, then git cherry-pick the non-merge commits.

…lash_screen

jonasschnelli · 2013-04-14T09:44:58Z

fresh and clean pull request is #2524

sipa · 2013-04-14T09:59:45Z

You can just do a force push to the branch associated with the old pullrequest. You don't have to create a new one.

jonasschnelli · 2013-04-14T11:54:11Z

@sipa: i did a --force push but the "old commits" where still in the commit list. The branch on my github repo was completely different to what i had localy (and pushed with --force). That's why i then restarted the whole thing. Hope you excuse. :)

laanwj · 2013-04-14T14:35:47Z

Yes it's not a problem, he's just saying that in general you never need to re-create a pull request. Force pushing something new will override anything that is currently in the pull request. Sometimes, the old commits stay there for a while ~ 5 minutes but that's just github in the process of updating.

Diapolo reviewed Apr 9, 2013
View reviewed changes

Diapolo reviewed Apr 10, 2013
View reviewed changes

Diapolo mentioned this pull request Apr 10, 2013

new COPYRIGHT_YEAR macro in clientversion.h #2503

Merged

Diapolo reviewed Apr 11, 2013
View reviewed changes

Merge branch 'master' of git://github.com/bitcoin/bitcoin into new_sp…

21d0963

…lash_screen Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch> Conflicts: doc/assets-attribution.txt

laanwj reviewed Apr 11, 2013
View reviewed changes

copyright char as proper QChar

66fa5cb

Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>

Diapolo reviewed Apr 11, 2013
View reviewed changes

doc/assets-attribution.txt

Copy link
Copy Markdown

Diapolo Apr 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an indention via tab ;).

laanwj reviewed Apr 12, 2013
View reviewed changes

Jonas Schnelli added 2 commits April 14, 2013 09:47

Merge branch 'master' of git://github.com/bitcoin/bitcoin into new_sp…

ab08de4

…lash_screen

own class for splashscreen

e7467cc

jonasschnelli closed this Apr 14, 2013

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Conversation

jonasschnelli commented Apr 9, 2013

Uh oh!

jonasschnelli commented Apr 9, 2013

Uh oh!

laanwj commented Apr 9, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Diapolo commented Apr 9, 2013

Uh oh!

jonasschnelli commented Apr 9, 2013

Uh oh!

laanwj commented Apr 9, 2013

Uh oh!

laanwj commented Apr 9, 2013

Uh oh!

jonasschnelli commented Apr 9, 2013

Uh oh!

laanwj commented Apr 9, 2013

Uh oh!

jonasschnelli commented Apr 10, 2013

Uh oh!

gmaxwell commented Apr 10, 2013

Uh oh!

laanwj commented Apr 10, 2013

Uh oh!

Diapolo commented Apr 10, 2013

Uh oh!

jonasschnelli commented Apr 10, 2013

Uh oh!

Diapolo commented Apr 10, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Apr 10, 2013

Uh oh!

Diapolo commented Apr 10, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Apr 10, 2013

Uh oh!

Diapolo commented Apr 10, 2013

Uh oh!

sipa commented Apr 10, 2013

Uh oh!

laanwj commented Apr 10, 2013

Uh oh!

qubez commented Apr 10, 2013

Uh oh!

Diapolo commented Apr 11, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Apr 11, 2013

Uh oh!

jonasschnelli commented Apr 11, 2013

Uh oh!

fanquake commented Apr 11, 2013

Uh oh!

jonasschnelli commented Apr 11, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment