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

[DO NOT MERGE] Onboarding views + OpenGL #126

Closed
wants to merge 13 commits into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jun 3, 2022

This PR is for testing purpose only to provide designers with CI built binaries for supported platforms.

It combines #33 and #124.

Windows
macOS
Android

hebasto and others added 13 commits June 3, 2022 12:15
Changes:
- Added option to not wrap the text in Header.qml
- Added option to bold the text and right align the text in
  TextButton.qml
- These changes are the prerequisite to the forthcoming commits.
- This is an object similar to Setting.qml
- Instead of a switch button this allows to have the text as the
  option's value.
- The text could also double as an hyperlink if the link is given.
- Adds the onboarding01.qml file and its 3 subpages
	- onboarding01a.qml : Introduction
	- onboarding01b.qml : About
	- onboarding01c.qml : Developers Options
- Adds the About and DeveloperOptions component which facilitates
  working of the pages 01b and 01c respectively.

Co-authored-by: jarolrod <[email protected]>
- This corresponds to the second page, which talks about Strengthening
  Bitcoin

Co-authored-by: jarolrod <[email protected]>
- The page correponds to the block clock page in design file.
- The block clock is temporarily replaced with the Bitcoin core app icon
- This page corresponds to the "Storage location" page in design file.
- This commit also adds a component file: StorageLocations.qml to
  facilitate the working of the onboarding04.qml file.

Co-authored-by: jarolrod <[email protected]>
- This includes the onboarding05.qml file and it subpages:
	- onboarding05a.qml : "Storage"
	- onboarding05b.qml : "Storage settings"
- This commit also includes a components file to facilitate the
  onboarding05b.qml file

Co-authored-by: jarolrod <[email protected]>
- This includes the main onboarding06.qml file along with its subpages:
	- onboarding06a.qml : "Connection"
	- onboarding06b.qml : "Connection settings"
- This commit also updates the ConnectionSettings.qml component to
  better reflect the design file.

Co-authored-by: jarolrod <[email protected]>
Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

  • Tested this PR on a Xiaomi Note 5 Pro, with Android 9 Pie
  • I successfully downloaded the app and ran it on my phone.

Observations:

Note: These are the things I observed while running the app. They are not intended as a set of suggestions for any specific PR.

  1. The Bitcoin core icon is not shown when installing the app but is shown when the app is downloaded.

Frame 2

Of the onboarding views:
Frame 1

  1. Text on all the pages overflows.
  2. OptionButton switches only when they are tapped at the border. Without touching the edge, taping in the center does not change the selected OptionButton.
  3. The color of the Bitcoin icon is bright green, corresponding to testnet. Don't know if that's a deliberate choice.

@jarolrod
Copy link
Member

jarolrod commented Jun 3, 2022

@shaavan thank you for testing on android, that's actually one thing we wanted to quickly confirm about the current master branch in general. Seems like this should be feedback on #124 in consideration of mobile devices.

This, in considersation of this repo, is about fixing the stuttering issues by moving from 2d rendering to opengl rendering. Please also test on desktop comparing this branch and master to see if there's a considerable difference in how things are rendered in switching through pages + resizing the window

@shaavan
Copy link
Contributor

shaavan commented Jun 3, 2022

Seems like this should be feedback on #124 in consideration of mobile devices.

Yes. And AFAIK, fixing the layout according to the design guide (for example, using max-width instead of width) shall fix most of these issues. I shall soon update PR #124 with the layout set.

Please also test on desktop comparing this branch and master to see if there's a considerable difference in how things are rendered in switching through pages + resizing the window

I shall soon post my observation regarding this.

@shaavan
Copy link
Contributor

shaavan commented Jun 3, 2022

I could not observe any visible improvement on my desktop using OpenGL.

Specification:

Ubuntu 22.04 with Qt version 5.15.3

Demo:

With 2D Rendering (PR#124) With OpenGL(PR#126)
Without OpenGL(1) With OpenGL

Edit 1:

For the above testing, I dynamically built the PR. It is possible that the PR might not have been built correctly. I will try building this PR with static builds to see if I observe any difference in the smoothness and rendering of the GUI.

Edit 2: This comment has been outdated. See this review for a better analysis of the PR.

@hebasto
Copy link
Member Author

hebasto commented Jun 3, 2022

I could not observe any visible improvement on my desktop using OpenGL.

Specification:

Ubuntu 22.04 with Qt version 5.15.3

Do logs confirm OpenGL usage?

@shaavan
Copy link
Contributor

shaavan commented Jun 3, 2022

I could not find any statement while configuring that talks about using OpenGL.

However, I found a seemingly relevant statement in the debug.log

image

I used the dynamic build for building this PR. Let me give a try building with static builds as well.

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

I successfully compiled this PR by doing a static build using depends.

System Specification:

Ubuntu 22.04 with Qt version 5.15.3

Steps used to compile

From ../gui-qml folder

cd depends
make -j8
cd ..
./autogen.sh
CONFIG_SITE=\$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure --without-bdb --with-qml
make -j8

I observed considerable improvements with using OpenGL Rendering over 2-D Rendering.

Demo:

With 2D Rendering (PR#124) With OpenGL(PR#126)
Without OpenGL(1) Screencast from 05-06-22 01 58 18 AM IST

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

Concept ACK

I have tested this and compared to master using static builds. There is a huge difference, as expected. I am not including a video demonstration as I'm not sure it would accurately convey the difference due to compression and file size limit constraints.

OpenGL support is needed for implementing several UI elements as specified in the design files, it's widely superior to 2d rendering for general UI fluidity, and is needed for proper wayland support.

hebasto added a commit that referenced this pull request Jun 16, 2022
745c797 build, qt: Enable OpenGL for Linux static builds (Hennadii Stepanov)
02f5f83 build, qt: Add libglvnd package to support OpenGL on Linux (Hennadii Stepanov)
e66f0bd build, qt: Enable OpenGL for Windows static builds (Hennadii Stepanov)
27a2a12 build, qt: Enable OpenGL for macOS static builds (Hennadii Stepanov)

Pull request description:

  Software renderer has [limitations](https://doc.qt.io/QtQuick2DRenderer/qtquick2drenderer-limitations.html). Therefore, it'd better to avoid it.

  Usability feedback collected in #126:
  - #126 (review)
  > I observed considerable improvements with using OpenGL Rendering over 2-D Rendering.
  - #126 (review)
  > There is a huge difference, as expected.

  ---

  #### Guix builds:
  ```
  $ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  0ab9d27f38b042a2b76c54115976381998932e21a14c9705bf1b5bb1ce9b0623  guix-build-745c797a64ba/output/aarch64-linux-gnu/SHA256SUMS.part
  456b6bf902711aeeecb2ea107f3ef5ef55eae13ba404f86f9b45e7a18ec4b7cd  guix-build-745c797a64ba/output/aarch64-linux-gnu/bitcoin-745c797a64ba-aarch64-linux-gnu-debug.tar.gz
  3ae2b6a95d03df60eb910c39978539aa791b9d74291955ea83f75daf0db25dec  guix-build-745c797a64ba/output/aarch64-linux-gnu/bitcoin-745c797a64ba-aarch64-linux-gnu.tar.gz
  42e7379ac9f4f6a0f805cbca00da9fc2b9f33b9a374dfa7f0c9fc01d6bc9e072  guix-build-745c797a64ba/output/arm-linux-gnueabihf/SHA256SUMS.part
  18cbf0619483fcb1a550cf7e66944bf2779e8af129c6f5d4cc256bcc8f658e6f  guix-build-745c797a64ba/output/arm-linux-gnueabihf/bitcoin-745c797a64ba-arm-linux-gnueabihf-debug.tar.gz
  f5aab05ed5be4d0955dd33cbfec5e1bfe8e943751c8495b93cd1d2c5ec343f91  guix-build-745c797a64ba/output/arm-linux-gnueabihf/bitcoin-745c797a64ba-arm-linux-gnueabihf.tar.gz
  375a2981a1721a937a260992c76978d220a554d65da3aca6805f33b2bdd101f8  guix-build-745c797a64ba/output/arm64-apple-darwin/SHA256SUMS.part
  b5e2336c669efbc3ca7782e7a0b7cf94ee9047e6b02c2fc6304393b9e9019f2f  guix-build-745c797a64ba/output/arm64-apple-darwin/bitcoin-745c797a64ba-arm64-apple-darwin-unsigned.dmg
  8d1c2e3680ce4902cd2e6798709f44193682c00225e9b9d049ac7401bafbb711  guix-build-745c797a64ba/output/arm64-apple-darwin/bitcoin-745c797a64ba-arm64-apple-darwin-unsigned.tar.gz
  18aa1676cb65f73c79eac01781bca86987247bb6565cd6774332e1b0db9959d8  guix-build-745c797a64ba/output/arm64-apple-darwin/bitcoin-745c797a64ba-arm64-apple-darwin.tar.gz
  455a3c80e32505ffb7eb008985579a7b087ea6c4d4915d87b019ac7375a9ecde  guix-build-745c797a64ba/output/dist-archive/bitcoin-745c797a64ba.tar.gz
  ed6fa2b071769d39641c51980b986b56b42670f4121294589caf438e46f79f37  guix-build-745c797a64ba/output/powerpc64-linux-gnu/SHA256SUMS.part
  6260f3fa1c0955c2ecc2c507a43188bfb29e456c888f8619d615aa0db46be780  guix-build-745c797a64ba/output/powerpc64-linux-gnu/bitcoin-745c797a64ba-powerpc64-linux-gnu-debug.tar.gz
  160ab587612d5b5fee25fbab29d8ee8a64ce2847f441c60f5f7c3006a521ba85  guix-build-745c797a64ba/output/powerpc64-linux-gnu/bitcoin-745c797a64ba-powerpc64-linux-gnu.tar.gz
  de995da04349c4edf5df2eb97e4bfad5c6425bee5a96a0b9d4fb6a851c878c5c  guix-build-745c797a64ba/output/powerpc64le-linux-gnu/SHA256SUMS.part
  c780b674a030b56efa536b6d9d67579f8682461af020634a4f41899b6a714ad5  guix-build-745c797a64ba/output/powerpc64le-linux-gnu/bitcoin-745c797a64ba-powerpc64le-linux-gnu-debug.tar.gz
  04ae6d80cec6474db5ed3a55d7e9462c2e32f22558fe483254c6742470362ad4  guix-build-745c797a64ba/output/powerpc64le-linux-gnu/bitcoin-745c797a64ba-powerpc64le-linux-gnu.tar.gz
  bb22ee16007c5c30d281a328854e8247456a0101c3aa89f38a42dff3b6c12feb  guix-build-745c797a64ba/output/riscv64-linux-gnu/SHA256SUMS.part
  186e6de5c8fff822297c2ba89f33a12ac7a50afd3347f2bf083f69c01d5ca0e7  guix-build-745c797a64ba/output/riscv64-linux-gnu/bitcoin-745c797a64ba-riscv64-linux-gnu-debug.tar.gz
  13f4e0833b5d68bf8025b8115fefaf4c8aedccbb3b29cc35e8458051df776d90  guix-build-745c797a64ba/output/riscv64-linux-gnu/bitcoin-745c797a64ba-riscv64-linux-gnu.tar.gz
  aea2a10fd06e85ba7c314a9e82380d3b72e564a9d1d3663f82280be687fcf17f  guix-build-745c797a64ba/output/x86_64-apple-darwin/SHA256SUMS.part
  ae54514b6c2a5c8bb6f524f7a21985a78cdffd78a7dfaea76aa2b9ba0dc51bd9  guix-build-745c797a64ba/output/x86_64-apple-darwin/bitcoin-745c797a64ba-x86_64-apple-darwin-unsigned.dmg
  1c41f1bd7c5d4ff926613eaecfb61eabbf28e79a2ebdefe67c7831242ccfba3e  guix-build-745c797a64ba/output/x86_64-apple-darwin/bitcoin-745c797a64ba-x86_64-apple-darwin-unsigned.tar.gz
  39d858b14bf2b7c93f00512343c9266eca906b0cc5ae05f3dad66d9e3beb5bd3  guix-build-745c797a64ba/output/x86_64-apple-darwin/bitcoin-745c797a64ba-x86_64-apple-darwin.tar.gz
  241a6fa2699cd157862ec7b6bcdbd1f4f8febef9cfe4c641ba74bbb7e75a2fc6  guix-build-745c797a64ba/output/x86_64-linux-gnu/SHA256SUMS.part
  4aab1141502884c253a2944094bdf8e432a8570f6cc7d713963f84823aed1917  guix-build-745c797a64ba/output/x86_64-linux-gnu/bitcoin-745c797a64ba-x86_64-linux-gnu-debug.tar.gz
  c998f3a855470f90e5837aecf3f33deb994cb00519c49eb62fcc190b243502e8  guix-build-745c797a64ba/output/x86_64-linux-gnu/bitcoin-745c797a64ba-x86_64-linux-gnu.tar.gz
  1be28babb8ff03621669a0d2b8c31d9d5d2d031700271bcd1e16ada0b56dd092  guix-build-745c797a64ba/output/x86_64-w64-mingw32/SHA256SUMS.part
  4536a24c9d13dbe974f641c004e1492cb76401126757e5b0815e2c1652ab5b95  guix-build-745c797a64ba/output/x86_64-w64-mingw32/bitcoin-745c797a64ba-win64-debug.zip
  b3d161d0f4161ced106f8acccc06f9627f35dadb2437a11a1b47b7d3d97c6912  guix-build-745c797a64ba/output/x86_64-w64-mingw32/bitcoin-745c797a64ba-win64-setup-unsigned.exe
  40e331efcf49bc43817c941164c829dcd78d46eee0167d98970076261e4e1bd3  guix-build-745c797a64ba/output/x86_64-w64-mingw32/bitcoin-745c797a64ba-win64-unsigned.tar.gz
  1a7ba46ee9ae086f982fbe2afc4e3b0be01ebc02c093c5c9f703b07dc48de0e1  guix-build-745c797a64ba/output/x86_64-w64-mingw32/bitcoin-745c797a64ba-win64.zip
  ```

  [![Windows](https://img.shields.io/badge/OS-Windows-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/insecure_win_gui.zip?branch=pull/33)
  [![macOS](https://img.shields.io/badge/OS-macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/insecure_mac_gui.zip?branch=pull/33)
  [![Android](https://img.shields.io/badge/OS-Android-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/insecure_android_apk.zip?branch=pull/33)

Top commit has no ACKs.

Tree-SHA512: ab36076968af924c9510ba0611551f1fd05e7fe864ccae9903d1abe0dc499087e44826817368371d9bd55b309484702344186b6ff01244e5c5e373510fe3bdfc
@jarolrod
Copy link
Member

This can be closed now

@hebasto hebasto closed this Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants