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

Allow compilation by disabling cmake flag WITH_QTWEBENGINE #2006

Merged
merged 2 commits into from
Nov 4, 2021

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Nov 2, 2021

Description

On some platforms (ARM based) the QtWebEngine seems to be missing. The CMAKE flag WITH_QTWEBENGINE can now be set to false on those to disable the webbrowser view internal to the OnlineQueries plugin. The websites will instead be queried inside the system's default web browser.

  • Currently this flag has to be manually set. It may be possible to set the flag as consequence of cmake being unable to find the QtWebEngine modules, and emit a warning to the developer during compilation.

  • If there is some style guide for cmake, I surely did something wrong! Advice?

Fixes #2004, #1982

Screenshots (if appropriate):

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update

How Has This Been Tested?

Test Configuration:

  • Operating system: Windows 10; Raspberry Pi 4.
  • Graphics Card: irrelevant.

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@gzotti gzotti added enhancement Improve existing functionality state: confirmed A developer can reproduce the issue importance: critical Real showstopper, program fails here qt Issues, related to Qt framework labels Nov 2, 2021
@gzotti gzotti added this to the 0.21.3 milestone Nov 2, 2021
@gzotti gzotti self-assigned this Nov 2, 2021
@gzotti gzotti added this to To do in GUI via automation Nov 2, 2021
@gzotti gzotti added this to To do in OS: macOS via automation Nov 2, 2021
@gzotti gzotti added this to To do in Plugin via automation Nov 2, 2021
@gzotti gzotti added this to To do in Hardware: ARM via automation Nov 2, 2021
@gzotti gzotti added this to To do in Interoperability of Stellarium via automation Nov 2, 2021
@github-actions github-actions bot requested a review from alex-w November 2, 2021 13:36
@github-actions
Copy link

github-actions bot commented Nov 2, 2021

Great PR! Please pay attention to the following items before merging:

Files matching guide/**:

  • Did you remember to update screenshots to match new updates?
  • Did you remember to grammar check in changed part of documentation?

This is an automatically generated QA checklist based on modified files

@github-actions
Copy link

github-actions bot commented Nov 2, 2021

Hello @gzotti! Thank you for this enhancement.

@gzotti gzotti linked an issue Nov 2, 2021 that may be closed by this pull request
Copy link
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

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

Please update BUILDING.md also

@alex-w
Copy link
Member

alex-w commented Nov 2, 2021

This PR fixed issue #1982 partially, because the latest one contains 2 problems: no module on Apple Silicon chips and regress in Qt 5.15.x.

@gzotti
Copy link
Member Author

gzotti commented Nov 2, 2021

Do you know how to unset the flag if cmake cannot find the QtWebEngine module? This setting should emit a well-visible warning as well, so that it can be installed if available and the user just had forgotten.

What kind of regress has happened in 5.15? Mac only or all platforms? (I have not tried building this on 5.15 yet.)

@alex-w
Copy link
Member

alex-w commented Nov 2, 2021

Do you know how to unset the flag if cmake cannot find the QtWebEngine module? This setting should emit a well-visible warning as well, so that it can be installed if available and the user just had forgotten.

First problem: standard cmake cannot check the architecture of target platform (see solution here), the second problem - on some platforms (e.g. linux) you may just not installed it in system, but it exists as package in repository or it just not installed from standard Qt installer. Of course you may change the logic of your cmake script to enable/disable flag WITH_QTWEBENGINE (see FIND_PACKAGE docs + you may add MESSAGE(STATUS ...) to inform the user).

What kind of regress has happened in 5.15? Mac only or all platforms? (I have not tried building this on 5.15 yet.)

Right now I've tested it on linux and mac and I can reproduce the issue on mac only. It may be related to deprecation brige Qt-OpenGL on the Mac or changes the requirement of OpenGL version (3.2 + enable core profile).

@alex-w
Copy link
Member

alex-w commented Nov 2, 2021

Important note: the regress on Mac may be not related to the QtWebEngine module!

@gzotti
Copy link
Member Author

gzotti commented Nov 2, 2021

Ad (1) The logic may be:

if QtWebEngine module not found:
disable WITH_QTWEBENGINE
emit warning message to the user who tries to compile: "It seems you have not installed QtWebEngine, or it may not be available on this platform. Disabling its use."

This warning should not rush by but be visible as one of the last messages.

Ad (2) please try to compile this on a Mac with 5.15 to see if QtWebEngine changed to the worse. Else, this OpenGL deprecation may hit us. It will do so sooner or later, at least on this platform.

@alex-w
Copy link
Member

alex-w commented Nov 2, 2021

Building with option WITH_QTWEBENGINE=Off has no issues on Mac with Qt 5.15.2. Maybe this is related to OpenGL context (I didn't check details for implementation of independence the window of OnlineQuery plugin)?

Copy link
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

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

Thanks!

Plugin automation moved this from To do to In progress Nov 4, 2021
@alex-w alex-w merged commit 8390b42 into master Nov 4, 2021
Interoperability of Stellarium automation moved this from To do to Done Nov 4, 2021
@alex-w alex-w deleted the allow-no-qtwebengine branch November 4, 2021 01:33
OS: macOS automation moved this from To do to Done Nov 4, 2021
GUI automation moved this from To do to Done Nov 4, 2021
Hardware: ARM automation moved this from To do to Done Nov 4, 2021
Plugin automation moved this from In progress to Done Nov 4, 2021
@alex-w alex-w added the state: fixed The bug has been fixed label Nov 14, 2021
@github-actions
Copy link

Hello @gzotti! Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w
Copy link
Member

alex-w commented Nov 14, 2021

Just for note: same problem (missing build dependencies) for architectures s390x and ppc64el

@gzotti
Copy link
Member Author

gzotti commented Nov 14, 2021

Yes, I have also received the flood from canonical. The complete solution for CI would be to let cmake detect the presence of qtwebengine-dev, and if not found react with disabling the feature, plus emitting a warning (in case the human compiling on a "good" platform has forgot to install the available package).

To be realistic: How many instance of Stellarium are running on s390x and ppc64el?

@gzotti gzotti mentioned this pull request Nov 19, 2021
18 tasks
gzotti added a commit that referenced this pull request Nov 20, 2021
- dynamic creation of our web view, dependent on Qt package availability
- detect QtWebEngine package availability
- Exclude QtWebEngine from builds on WSL.

Co-authored-by: Alexander V. Wolf <alex.v.wolf@gmail.com>
@rayw000 rayw000 mentioned this pull request Nov 28, 2021
@alex-w alex-w removed the state: fixed The bug has been fixed label Dec 25, 2021
@github-actions
Copy link

Hello @gzotti! Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality importance: critical Real showstopper, program fails here qt Issues, related to Qt framework state: confirmed A developer can reproduce the issue
Projects
GUI
  
Done
Hardware: ARM
  
Done
OS: macOS
  
Done
Plugin
  
Done
Development

Successfully merging this pull request may close these issues.

Cannot build OnlineQueries on Raspberry Pi Fresh build, stuck in black screen on startup
2 participants