Skip to content
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

gui: Port Bitcoin Intro class (implement the ability to choose a data directory via the GUI) #1978

Merged

Conversation

jamescowens
Copy link
Member

@jamescowens jamescowens commented Nov 27, 2020

This PR is a port of Bitcoin's Intro class and the associated form to support the choice of a data directory via the GUI. This PR also has implemented stateful management of that choice in the optionsModel, similar to the Bitcoin approach. This should make the choice of non-default data directories much easier for people with GUI wallet installations, especially the MacOS, where it is difficult to supply program arguments to the underlying command line in the icon.

I have also solved the silent exit if the gridcoinresearch.conf file is corrupted/badly formatted. The solution uses a simplified form of the newer Bitcoin config file parsing functions, which needed to be ported anyway to suppport Intro. (Closes #1303.)

Most of the issues with scaling I think I have solved with this PR. (Closes #1952 and closes #1953.)

Also implements an early check for the ability to lock the data directory. Closes #1872.

@jamescowens jamescowens self-assigned this Nov 27, 2020
@jamescowens jamescowens added this to the Hilda milestone Nov 27, 2020
@jamescowens jamescowens force-pushed the optionsModel_datadir branch 6 times, most recently from 2b796d2 to 4caa7f7 Compare November 27, 2020 21:14
@jamescowens
Copy link
Member Author

I think I have solved many of the issues in #1926 also with this PR.

This was linked to issues Nov 27, 2020
@jamescowens jamescowens force-pushed the optionsModel_datadir branch 4 times, most recently from 29ca167 to 8a83aae Compare November 28, 2020 01:19
@jamescowens jamescowens marked this pull request as ready for review November 28, 2020 04:02
@jamescowens
Copy link
Member Author

jamescowens commented Nov 28, 2020

I have done quite a bit of testing and "futzing" with this on several different platforms.

The appearance looks good on my hidpi openSUSE Linux box, my hidpi W10 box, my old Windows 2008 32 bit server VM (not hidpi), and my ARM Odroid. I have reduced the default size of the window, and implemented memorization of the window geometry (size and position).

Functionality testing:

  1. Fresh start of Gridcoin with missing data dir, the dialog comes up.
  2. Snapshot download functionality still works correctly.
  3. Stateful storage of data directory works as it should. Verified on Windows with unicode path.
  4. Window geometry is memorized correctly
  5. -choosedatadir override argument correctly re-presents the Intro dialog box to choose a new data directory
  6. Specifying -datadir as an argument overrides the setting in the GUI config file/registry entry area and does not disturb the entry in the GUI config.
  7. On the config file silent exit problem. Bad config files now produce a dialog box that warns that config file cannot be parsed correctly and the application exits rather than exiting silently.

@jamescowens jamescowens force-pushed the optionsModel_datadir branch 3 times, most recently from 70bf5ac to 0854d57 Compare November 28, 2020 16:43
@jamescowens
Copy link
Member Author

I still don't like the look of the intro dialog on Windows hidpi displays. It is too narrow. @cyrossignol maybe you can take a look. Windows is doing something funky with the logicalDpi I think.

@jamescowens
Copy link
Member Author

I am not fooling with the dialog size anymore. It is pretty narrow on Windows hidpi displays, but it doesn't cut any of the text off. Good enough for right now.

This commit implements the datadir state management in the
optionsModel to support the Intro class.
This commit ports certain Bicoin utility functions to support
the implementation of the Intro dialog box class (the ability
to specify a data directory with the GUI and have it saved
in the options model file).
Copy link
Member

@cyrossignol cyrossignol left a comment

Choose a reason for hiding this comment

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

Looks pretty good from an initial glance... building to test and play with different DPIs...

@nathanielcwm
Copy link
Contributor

I'll take a look soon...

@jamescowens
Copy link
Member Author

Please take a look. I don't want this PR to sit around too much longer.

@nathanielcwm
Copy link
Contributor

nathanielcwm commented Dec 4, 2020

idk don't really see an appreciable difference between this pr and 5.1.0 in terms of the font rendering.
image

Datadir choice box looks fine though.

@jamescowens
Copy link
Member Author

@nathanielcwm What resolution are you running and what scale factor do you have set for the GUI?

src/qt/intro.cpp Outdated Show resolved Hide resolved
This commit ports the Bicoin intro.h, .cpp,and .ui files,
which contain the implementation of the GUI Intro class. This
provides the implementation support of the intro ui form (dialog
box), which allows the user to specify a data directory on a
brand new installation of Gridcoin and have it remembered in the
options model (Qt GUI config file).
@nathanielcwm
Copy link
Contributor

nathanielcwm commented Dec 6, 2020

@jamescowens 1280x1024
It's at 100%

@jamescowens
Copy link
Member Author

jamescowens commented Dec 6, 2020

@nathanielcwm did you resize the application window? This is the default size with 1280x1024 and 100% scaling in Windows 10 (with German localization active). It looks good to me, and different from yours above.

VirtualBox_W10-2_05_12_2020_22_16_41

It also appears the same as the 5.1.0.0 release on Windows.

@nathanielcwm
Copy link
Contributor

nathanielcwm commented Dec 6, 2020

Weird...
Nope I didn't except I think that screenshot was maximised.
Could you send me a compiled executable to test, just in case I messed up building lol.

I'm using the native theme but it looks like your screenshot might be with the light theme? @jamescowens

@jamescowens
Copy link
Member Author

It was light. But native is no different.

VirtualBox_W10-2_05_12_2020_23_07_29

@jamescowens
Copy link
Member Author

If you maximize the window, of course the layout is going to look sparse. The text is sized for the window to take up 40% to 60% of the x and y dimensions of the screen. It is 40% for hidpi screens and increases to more like 60% of low dpi screens because of minimum size settings for some of the widgets. (It used to be 60% plus in each dimension for all, but I changed it to 40%, because 60% is really too big for today's aesthetic.)

The text does not change scale even if the wallet is maximized, and is not meant to. Most people have been asking for a smaller window layout anyway.

@jamescowens
Copy link
Member Author

If we think this font is too squashed, maybe we should change to a different open source sans serif with a little wider layout for a given point size.

This commit implements necessary modifications to Gridcoin's
GUI startup to support the ported Bitcoin Intro class. Note that
Bitcoin's startup has diverged significantly from Gridcoin over
the last few years, so I had to make some adjustments/simplifications
to avoid rewriting the whole thing.

Also enables hidpi pixmap scaling and persist window geometry.
Also corrects unused variable warning on Linux compilation
This does an early check for the ability to lock the data directory
and corrects a segfault condition if the lock is not obtainable.
@nathanielcwm
Copy link
Contributor

@jamescowens well my issue isn't it looking sparse it's the text being squished

Just built on 04d0c77 and it looks like i'm still experiencing squished text

image

Maybe it's just an issue with my Windows installation, especially since it doesn't look like anyone else is reporting this issue.

@jamescowens
Copy link
Member Author

I think it depends on your definitiion of squished. :) That text looks ok to me. It is a little "tight" but many modern sans-serif fonts are that way.

@jamescowens
Copy link
Member Author

Cy has been too busy to finish testing this on MacOS. It appears to be working well now. I am going to go ahead and merge this so that folks on testnet with MacOS can try it out easily.

@jamescowens jamescowens merged commit 3f976d4 into gridcoin-community:development Dec 12, 2020
@jamescowens jamescowens deleted the optionsModel_datadir branch January 4, 2021 00:28
jamescowens added a commit that referenced this pull request Mar 1, 2021
 Added
 - gui: Add RAC column to wizard summary page projects table #1951 (@cyrossignol)
 - rpc: clean up the superblocks function and add magnitude to getmininginfo #1966 (@jamescowens)
 - rpc: Add transaction size to RPC output #1971 (@cyrossignol)
 - voting: Add user-facing support for poll response types #1976 (@cyrossignol)
 - gui: Port Bitcoin Intro class (implement the ability to choose a data directory via the GUI) #1978 (@jamescowens)
 - gui: Port Bitcoin MacOS app nap manager #1991 (@jamescowens)
 - mining, rpc: Implement staking efficiency measure and improve SelectCoinsForStaking and CreateCoinStake #1992 (@jamescowens)
 - accrual, rpc: Implement auditsnapshotaccruals #2001 (@jamescowens)
 - docs: add doxygen support #2000 (@div72)
 - beacon: Specialized beacon storage in leveldb #2009 (@jamescowens)
 - rpc: Add a call to dump contracts in binary form #2011 (@div72)
 - rpc: Add boolean option to report active beacons only in beaconreport #2013 (@jamescowens)
 - consensus: Set Hilda mainnet hardfork height to 2197000 #2022 (@jamescowens)

 Changed
 - refactor: [Memory optimization] Block index duplicate PoS state #1945 (@cyrossignol)
 - refactor: [Memory optimization] Block index superblock and contract flags #1950 (@cyrossignol)
 - refactor: [Memory optimization] Remove stake modifier checksums #1954 (@cyrossignol)
 - refactor: [Memory optimization] Block index allocation overhead #1957 (@cyrossignol)
 - refactor: [Memory optimization] Remove block index subsidy fields #1960 (@cyrossignol)
 - refactor: [Memory optimization] Separate chain trust from the block index #1961 (@cyrossignol)
 - refactor: [Memory optimization] Eliminate padding between block index fields #1962 (@cyrossignol)
 - beacon, gui: Add check for presence of beacon private key to updateBeacon() #1968 (@jamescowens)
 - util: Enhance ETTS calculation #1973 (@jamescowens)
 - refactor: Use new clamp in util.h #1975 (@jamescowens)
 - gui: Redo global status for overview #1983 (@jamescowens)
 - util: Improvements to MilliTimer class and use in the miner and init #1987 (@jamescowens)
 - rpc: Move rpc files to directory #1995 (@Pythonix)
 - rpc: Enhance consolidateunspent and fix fee calculation #1994 (@jamescowens)
 - contract: Double the lookback scope of contract replay #1998 (@jamescowens)
 - net: Don't rely on external IP resolvers #2002 (@Tetrix42)
 - beacon: Change beacon map to pointers #2008 (@jamescowens)
 - gui: Update bitcoin_sv.ts #2014 (@sweede-se)
 - util: Update snapshot URLs and add accrual directory #2019 (@jamescowens)
 - beacon: Tweak BeaconRegistry::Revert #2020 (@jamescowens)
 - rpc, qt: bump fees @2023 (@div72)

 Removed
 - researcher: Remove automatic legacy beacon key import #1963 (@cyrossignol)
 - util: Revert "Close LevelDB after loading the block index" #1969 (@cyrossignol)
 - ci: Fix python symlink issue & remove travis #1990 (@div72)
 - ci: remove python workaround #2005 (@div72)

 Fixed
 - gui: fix mandatory/leisure detection of upgrade check #1959 (@Pythonix)
 - voting: Fix title in "gettransaction" RPC for legacy poll contracts @1970 (@cyrossignol)
 - gui: Fix missing menu items on macOS #1972 (@scribblemaniac)
 - rpc: Fix answer offset in "votedetails" #1974 (@cyrossignol)
 - voting: Implement missing try-catch in VotingVoteDialog::vote #1980 (@jamescowens)
 - scraper: Add check for minimum housekeeping complete in scraper #1977 (@jamescowens)
 - voting: Fix nonsense vote weights for legacy polls #1988 (@cyrossignol)
 - voting: Fix incorrect field returned in ResolveMoneySupplyForPoll() #1989 (@cyrossignol)
 - consensus, accrual: Fix newbie accrual #2004 (@jamescowens)
 - log: grammar correction #2016 (@nathanielcwm)
 - wallet: Correct nMinFee fee calculation in CreateTransaction #2021 (@jamescowens)
 - rpc, miner: Correct GetLastStake #2026 (@jamescowens)
 - wallet: Fix bug in CreateTransaction causing insufficient fees #2029 (@jamescowens)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment