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

Add manual help viewer based on Qt Help Framework #3047

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Holzhaus
Copy link
Member

This allows reading the manual inside Mixxx instead of opening webbrowser (which requires an internet connection) or opening a PDF reader. If the help files are missing, Mixxx will fall back to open the requested manual page in a webbrowser.

@Holzhaus Holzhaus added this to the 2.4.0 milestone Aug 27, 2020
@Holzhaus Holzhaus force-pushed the qthelp-manual branch 7 times, most recently from 5d5bada to c015d02 Compare August 27, 2020 23:25
@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

I had to install the following additional packages to resolve config errors:

libqaccessibilityclient-qt5-dev
libqaccessibilityclient-qt5-0
qttools5-dev

It still gives me a config warning FATALqcollectiongenerator not found, documentation collection will not be generated.
(which is FATAL but not colored in the cmake log stream)
Which package is needed?

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

There are some glitches in the help view.
For example the keyboard mapping image is drawn off-center behind the text.
image
Once I scrolled further down, or mark text it disappears partially/completely
image
Mysterious: if I press Ctrl+Print to select a screen region for copying it returns..

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

I can't search within the manual, the Index tab has the same content as 'Glossary'

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

For example the keyboard mapping image is drawn off-center behind the text.

I'm trying to fix this. How can I make the built process recognize updated (local) manual ressources?

@Holzhaus
Copy link
Member Author

Holzhaus commented Aug 28, 2020

For example the keyboard mapping image is drawn off-center behind the text.

I'm trying to fix this. How can I make the built process recognize updated (local) manual ressources?

What do you mean? Should I make the branch and git repo customizable?

EDIT: Does it happen in Qt assistant as well? You can test via:

$ sphinx-build -b qthelp source build/qthelp
$ qhelpgenerator -o build/qthelp/Mixxx.qhc build/qthelp/Mixxx.qhcp
$ assistant-qt5 -collectionFile  build/qthelp/Mixxx.qhc

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

Should I run those commands in the mixxx top dir?

@Holzhaus
Copy link
Member Author

Should I run those commands in the mixxx top dir?

No, in the manual git root dir.

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

Unfortunately I'm running into python 2/3 issues again. (still need python2* for apps like gimp it seems)
sphinx-build -b qthelp source build/qthelp states Running Sphinx v1.8.5 even though I have Sphinx v2.something` installed with pip3.

@Holzhaus
Copy link
Member Author

Ooof, even though Python is EOL since January 1st? Anyway, replace sphinx-build with python3 -m sphinx

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

Thanks!

python3 -m sphinx -b qthelp source build/qthelp
qcollectiongenerator build/qthelp/Mixxx.qhcp
assistant -collectionFile build/qthelp/Mixxx.qhc

gives me a full help view with the image placed correctly (strange styling though):
image

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

...compared to the 'reduced view' of this branch
image

@Holzhaus
Copy link
Member Author

There are some glitches in the help view.
For example the keyboard mapping image is drawn off-center behind the text.
image
Once I scrolled further down, or mark text it disappears partially/completely
image
Mysterious: if I press Ctrl+Print to select a screen region for copying it returns..

The HTML code looks okay. We either need to work around this in the manual itself or wait for Qt to release a fix. I didn't implement the HTML renderer, this just uses a regular QTextBrowser.

@ronso0
Copy link
Member

ronso0 commented Aug 28, 2020

I looked at the code, too. the onl, difference is that the next correctly drawn image has a sub-chapter line right preceeding. Also when browsing the built manual it seems all other images are also correct

@Holzhaus
Copy link
Member Author

Maybe because it's an SVG image?

@Holzhaus
Copy link
Member Author

Holzhaus commented Aug 28, 2020

Do the other parts work fine?

  • I worked around the bug that broke the index list, so double clicking a keyword should now open the appropriate keyword in the glossary
  • The manual language depends on the application language so you can use LANG=de ./mixxx and get the German manual. It always falls back to the English version if the language is not supported.
  • I added a full text search tab
  • if you click the help button in the preferences it opens the correct section in the help viewer
  • if the files in res/help are missing, it opens the manual page in web browser instead

@ronso0
Copy link
Member

ronso0 commented Aug 30, 2020

Tests are failing.
Locally I get stuck at another point though:

/home/ronso/Downloads/mixxx_source/src/master/src/widget/helpviewer.cpp: In constructor ‘HelpViewer::HelpViewer(const QFileInfo&, QWidget*)’:
/home/ronso/Downloads/mixxx_source/src/master/src/widget/helpviewer.cpp:29:20: error: ‘class QHelpEngine’ has no member named ‘setUsesFilterEngine’
   29 |     m_pHelpEngine->setUsesFilterEngine(false);
      |                    ^~~~~~~~~~~~~~~~~~~
make[3]: *** [CMakeFiles/mixxx-lib.dir/build.make:6524: CMakeFiles/mixxx-lib.dir/src/widget/helpviewer.cpp.o] Error 1

@Holzhaus
Copy link
Member Author

Ah, that method is probably too new, I can just remove it.

@ronso0
Copy link
Member

ronso0 commented Aug 30, 2020

yeah *buntu 20.04 has just Qt5.12.8

@ronso0
Copy link
Member

ronso0 commented Aug 30, 2020

Alright, no I can resize the sidebar, this has tabs Contents | Index | Search

How would I tell cmake about a custom manual dir/repo now?
HELP_GIT_URL=https://github.com/ronso0/manual ?

@Be-ing
Copy link
Contributor

Be-ing commented Aug 30, 2020

This builds but the help viewer shows no contents for me.

There is a crash on shutdown:

Debug [Main]: 979 ms deleting EngineMaster
Debug [Main]: 1050 ms deleting HelpViewer
Debug [Main]: 1095 ms deleting DlgPreferences
Process 318228 stopped
* thread #1, name = 'mixxx', stop reason = signal SIGSEGV: invalid address (fault address: 0xc1)
    frame #0: 0x00000000000000c1
error: memory read failed for 0x0
(lldb) bt
* thread #1, name = 'mixxx', stop reason = signal SIGSEGV: invalid address (fault address: 0xc1)
  * frame #0: 0x00000000000000c1
    frame #1: 0x00007ffff519422d libQt5Core.so.5`QObject::~QObject() + 13
    frame #2: 0x000000000092ce70 mixxx`MixxxMainWindow::finalize() at mixxx.cpp:779:12
    frame #3: 0x000000000092e194 mixxx`MixxxMainWindow::~MixxxMainWindow() at mixxx.cpp:190:13
    frame #4: 0x000000000054af90 mixxx`main at main.cpp:43:41
    frame #5: 0x00007ffff4762042 libc.so.6`__libc_start_main + 242
    frame #6: 0x000000000056b29e mixxx`_start + 46

@Holzhaus
Copy link
Member Author

Holzhaus commented Sep 4, 2020

Can't we provide an other solution, something like a git repository that has the files as an unpacked version.
This would allow to make partial updates and ensures that the correct version of the manual is build.

That was the intial version. But it requires Python + installing python dependencies and it painful to maintain.

I don't want to add binary compiled files

Why are the files so big if that are already the compiled files. Is there room optimization?
Will the Mixxx download, currently ~15 MB grow to 245 MB?
That's sounds like an issue to me if you are connected with a low bandwidth.

It's the complete documentation including all images and in all languages. Maybe I can bring that size down by compiling all languages into a single qthelp file, but I gotta test that.

In any case the installer size will grow, but I think it's worth it.

@Be-ing
Copy link
Contributor

Be-ing commented Sep 5, 2020

I'm not so concerned with the package size, but I share @daschuer's concern at needing to do a big download before compiling Mixxx by default. Perhaps it could only download the manual for the locale of the build environment by default? For our official packages of course let's ship all translations.

@Holzhaus Holzhaus mentioned this pull request Oct 18, 2020
@Holzhaus
Copy link
Member Author

I managed to bring down the filesize for all languages to ~75 MiB. That's the best I can do, and I doubt it can be reduced further without removing all images.

@Holzhaus
Copy link
Member Author

If you want to test: Here's are the precompiled help files: https://transfer.sh/QdYrg/manual.zip
Unpack them into the res/help directory.

@Holzhaus
Copy link
Member Author

There are some glitches in the help view.
For example the keyboard mapping image is drawn off-center behind the text.
image
Once I scrolled further down, or mark text it disappears partially/completely
image
Mysterious: if I press Ctrl+Print to select a screen region for copying it returns..

I found this: pbek/QOwnNotes#1772 (comment)

Could we convert the SVGs to SVG Tiny 1.2? Maybd that would fix the issue.

@Be-ing Be-ing changed the base branch from master to main October 23, 2020 23:11
@github-actions
Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Jan 22, 2021
@poelzi
Copy link
Contributor

poelzi commented Feb 2, 2021

I would like to see a offline manual that certain help button can jump to.
I think we could either render the SVG in the build process to PNG and include that in the manual.
What if we instead use QWebEngineView instead ?

@github-actions github-actions bot removed the stale Stale issues that haven't been updated for a long time. label Feb 3, 2021
@Holzhaus
Copy link
Member Author

Holzhaus commented Feb 4, 2021

What if we instead use QWebEngineView instead?

That would be whole new dependency. It's basically the chrome backend IIRC.

I gotta check if there are any decent SVG to SVG tiny converters, that would solve all issues ;-)

@github-actions
Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Aug 11, 2021
@ferdnyc
Copy link

ferdnyc commented Sep 30, 2021

@Holzhaus

I have no small amount of experience with QtSVG, thanks to OpenShot, so I wrote this long-winded thing all about SVG Tiny that I'll leave at the end of this comment. However...

The layout issues you're seeing have nothing to do with SVG Tiny compatibility. I was able to get a figure using the original image file to lay out correctly (well, except for the fact that it doesn't properly center), just by making these adjustments to the .rst source in your branch:

diff --git a/source/chapters/controlling_mixxx.rst b/source/chapters/controlling_mixxx.rst
index 73cb920..b622d93 100644
--- a/source/chapters/controlling_mixxx.rst
+++ b/source/chapters/controlling_mixxx.rst
@@ -47,15 +47,22 @@ can perform actions by pointing and clicking with your mouse.
 Using a Keyboard
 ================
 
+.. container::
+
+   ..
+
 .. figure:: ../_templates/keyboard_mapping_sheet.svg
    :align: center
-   :width: 100%
+   :width: 782
+   :height: 240
    :figwidth: 100%
    :alt: Keyboard shortcuts
    :figclass: pretty-figures
 
    Mixxx Keyboard shortcuts (for en-us keyboard layout)
 
+.. only:: builder_html
+
    :download:`Download the image <../_static/Mixxx-220-Keyboard-Mapping.png>`
 
 Controlling Mixxx with a keyboard is handy. Unlike mouse control, the keyboard
@@ -98,6 +105,11 @@ real mixer and turntables or :term:`CDJ`.
 
 Loading a controller mapping
 ----------------------------
+
+.. container::
+
+   ..
+
 .. figure:: ../_static/Mixxx-200-Preferences-Controllers.png
    :align: center
    :width: 75%

(The .. only:: wrapping the download link isn't strictly necessary, it's just a good idea because those links are broken in QtHelp.)

The explicit :width: and :height: keep the browser from ignoring the image and laying text out right through it, and the empty .. container:: before the figure is to separate it from the preceding heading. Otherwise, it was being placed inline to the right of the heading, instead of in a block element following it.

As you can see I had to do the same thing before the PNG figure farther down, since it was also being inlined after the last line of the preceding paragraph heading. (But it also happens with figures following body paragraphs.) At a minimum, that adjustment (addition of an empty .. container::) will likely be necessary for every figure.

Still, it does work:

Screenshot from 2021-09-30 19-43-06

All of these layout issues come down to one — no, two simple facts:

  1. Since the deprecation of QtWebKit, Assistant has used QTextBrowser to display HTML content.
  2. QTextBrowser is a terrible HTML renderer.

In fact, people have at times gone to great lengths to display QtHelp content using a decent renderer, such as QtWebEngine.

As with QtSVG (see below), Qt implemented just barely enough HTML support in QTextBrowser to be able to display their very basic help formatting, then they immediately declared it "good enough" and stopped working on it.

So now QTextBrowser handles HTML rendering for QtHelp, something the sphinx qthelp builder was clearly never updated for. The HTML it generates is perfectly fine HTML, but it's utterly broken when rendered by QTextBrowser. It would take a lot of cleaning up and sanitizing to generate a useful QtHelp document, I suspect, and the HTML rendering would probably suffer quite a bit along the way.

SVG Tiny

Switching gears, here's what I can tell you about SVG Tiny:

  1. SVG-Tiny-1.2 is a poorly-supported subset of SVG-1.1-Full that mostly tosses away features for expediency:
    1. No filters
    2. Only basic text rendering (just linear layout and rotation, no deformations or effects)
    3. Limited animation (none at all, I think, in Qt's implementation)
    4. Hugely limited scripting
    5. No vendor-extensibility "stuff"
    6. etc.
  2. Because it's a strict subset, any SVG-1.1 file is also an SVG-Tiny-1.2 file as long as it uses no features that aren't supported by SVG-Tiny-1.2. As such, there aren't really too many libraries/applications that write "SVG Tiny" as a distinct format. (There are some. Just not any of the ones you'd initially think of and probably have access to.) However, exporting with e.g. Inkscape's "Optimized SVG" will typically produce an SVG Tiny-compatible file as long as the SVG doesn't apply any filters. (Not even blur, which is the one that usually trips most people up. Blur is flippin' handy to have!)
  3. There are some adjustments that help a lot. A big one is doing away with any CSS, so all properties are set directly as tag attributes rather than using style= and/or class=. (That adjustment is part of the Optimized SVG exporter, and it defaults to enabled.)
  4. If a file does employ unsupported features, the SVG Tiny renderer is supposed to just silently drop them and continue rendering. What kind of results this gives vary greatly, but usually you can see something and it at least passably resembles the image you expected. Sometimes, in fact fairly often in the beginning, you don't notice any difference at all. Then one day you load a file with some prominent visual application of a blur filter or a text effect, and from then on you'll start noticing the subtly missing elements in every file QtSVG renders.
  5. SVG Tiny isn't actually "tiny". SVG-Tiny-1.2 files are no smaller than SVG-1.1 files except by virtue of the bytes they save by not making use of as many language features. The "tiny" in the name refers to the size of the language definition vs. SVG-1.1, not any actual data sizes.
  6. The best SVG to hand to QtSVG tends to be... not hand-coded exactly, but... hand-"checked". When I work on files I know it'll be rendering, these days I use Optimized SVG to export from Inkscape, then I load the flle into a text editor and just sort of... "strategically massage" the results. If it's an icon I'll manually adjust all of the coordinates to whole-number positions, even for things like gradient stops where there's no actual reason they can't be floating-point values (which is why Inkscape is disinclined to lock itself down to whole numbers for those parameters).
  7. Without having seen the actual code, I suspect the layout issues you're seeing are most likely not an issue of SVG Tiny compatibility. It's possible there is an issue with the SVG file, like a missing bounding box or something like that, but that could equally be the case if it were an SVG-Tiny-1.2 file. Whatever's going on is likely throwing off QtSVG, which is (not merely by virtue of being an SVG-Tiny-1.2 renderer) extremely limited and basic. Qt implemented just enough to render SVG icons in the user interface and immediately gave up working on it, AFAICT.

I was right and wrong about those last bits: As I said, the layout issues were nothing to do with the SVG, but the SVG is also far too complex for SVG Tiny. (It has like a dozen filters on it! For no good reason!) In addition, many of the key cap labels are multi-line/line-wrapped text, which Tiny cannot handle.

Honestly the only way that image would ever display correctly in QtSVG is if all of the text is converted to paths, like the text on the original keycaps. By doing that in Inkscape, then re-exporting as Optimized SVG, I was able to finally get a decent rendering of that doc (except for the figure centering):

Screenshot from 2021-09-30 19-41-58

The size of the SVG file increased from ~240K to 570K (even with optimization!), but that's the price you pay to appease QtSVG.

@github-actions github-actions bot removed the stale Stale issues that haven't been updated for a long time. label Oct 1, 2021
@Be-ing Be-ing removed this from the 2.4.0 milestone Nov 14, 2021
@github-actions
Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build stale Stale issues that haven't been updated for a long time. ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants