Destroying and creating a scan popup instance twice in
MainWindow::editPreferences() is wasteful.
2b9dd55804 added the unconditional second
makeScanPopup() call below but didn't remove the existing call,
probably by mistake.
When scan popup is configured to appear without any key modifiers
pressed and is active on X11, it interferes with selecting text inside
the scan popup window (or inside the main window if "Send translated
word to main window" option is enabled). It also makes searching text
inside article definition impossible - in the main window and
even more so in the scan popup window.
However, when scan popup is configured to appear only when some keys are
pressed, or when the scan flag feature is enabled, it may work fine
inside Goldendict windows.
It is possible to automatically decide whether to show scan popup when
selection or clipboard inside Goldendict changes. But such logic might
be unsuitable for some use cases. For example, invoking scan popup by
selecting article definition text in the main window works fine.
Therefore this commit makes ignoring selection and clipboard changes
inside Goldendict itself optional. This commit implements one of two
feature requests in issue #606.
This new option could have effect on non-X11 platforms if the hidden
trackClipboardChanges option is enabled. But it is much less useful on
these platforms because scan popup without key modifiers is unusable
there (at least under Windows). Let us show and use the option only on
X11 to avoid cluttering Preferences UI on other platforms.
This option has effect even when scan popup functionality is disabled -
when the Ctrl+C+C hotkey is triggered. So the scanToMainWindow checkbox
should not be disabled when enableScanPopup is unchecked. Fixes #716.
qobject_cast() implementations in Qt4 and Qt5 until version 5.4
unconditionally dereference the argument to access a static member.
This is undefined behavior. When Goldendict is compiled with GCC's
-fsanitize=undefined option in Release mode and launched with
<useInternalPlayer>1</useInternalPlayer> in config, the application
crashes right away with the following message:
runtime error: member call on null pointer of type 'struct AudioPlayer'
When the current audio player instance changes, the destroyed player's
playback must be stopped for consistency between implementations and to
avoid surprises. Note that this residual playback could be stopped only
by switching back to the Ffmpeg player or by exiting the application.
My tests in many desktop environments and window managers indicate that
no single configuration works perfectly in all environments. There are
also behavior differences between Qt::Popup and Qt::Tool flags, which
are not exactly bugs, so I suppose users might subjectively prefer
different options.
Customizing the flags allows the user to prevent unpinned scan popup
window flickering with Qt5 on Linux. In a way adding these options fixes
issue #645, which is: the scan popup window blinks rapidly, barely
noticeably in some applications, such as Calibre ebook-viewer
and Chromium. In this case the scan popup window usually ends up hidden
when selection ends, unless it was finished with a jerk.
I have tested the new options in 9 desktop environments and window
managers: at least one configuration for each eliminates #645 and makes
the scan popup window work the same as with Qt4 in this regard:
the popup window remains visible, text in the popup's translation line
keeps up with the text selection in the external application,
and the selected text is being translated on the fly.
Moreover, for each tested DE/WM, at least one configuration makes
the scan popup window work perfectly as far as I am concerned.
This issue was partially worked around with a 200ms scan popup delay
timer in the recent commit 58e41fe3ce
for the duplicate issue #854. However the timer solution is incomplete
because it requires the user to select text quickly and without delays.
If global mouse selection does not change for 200ms while the left mouse
button is held down, the user will likely not see the scan popup when
(s)he finishes selection, and will have to try selecting again -
hopefully faster this time.
The 200ms delay is no longer critically important after this commit,
but it is still beneficial: the lookup query changes less often,
which in turn reduces article definition update frequency.
So the delay improves the UI (perhaps subjectively) and performance.
I have verified in numerous desktop environments and window managers
that the workaround does not work with Qt5.
Let us disable it in Qt5 builds to prevent potential issues
and eliminate the small performance overhead.
* add config and GUI support for internal player back end switching;
* make FFmpeg player disabling option consistent with other similar
qmake options by using CONFIG;
* add a new qmake option that disables Qt Multimedia player. This is
useful for GNU/Linux distributions where Qt WebKit and Qt Multimedia
packages depend on different GStreamer versions and don't work
correctly when combined in one application.
The existing FFmpeg+libao internal player back end has a relatively
low-level implementation, which is difficult to understand and improve.
There are at least 3 open internal player issues:
1) many GNU/Linux users have to edit their libao configuration file to
make Goldendict's internal player work (issue #412);
2) libao's pulseaudio plugin does not support 32-bit audio, which
means that many MediaWiki pronunciations don't work with the most
popular GNU/Linux audio driver (issue #949);
3) Ffmpeg::DecoderContext uses deprecated FFmpeg APIs, which causes
compiler warnings and means that this internal player back end
may not compile with a future FFmpeg library version (issue #978).
The Qt Multimedia back end implementation uses the highest-level
Qt audio API and is very simple.
This new back end works flawlessly on my GNU/Linux machine.
I'm not making it the default back end because I don't know how well
it will work on other platforms with different configurations.
This change substantially reduces audio player configuration code
complexity. The single remaining bool option - useInternalPlayer -
is enough for the binary audio player implementation choice.
Removing the useExternalPlayer option doesn't change the application GUI
behavior except for minor differences in one or two corner cases, which
could be classified as bugs fixed by this commit. For example, before
this commit, if the configuration file contained the following lines
<useExternalPlayer>0</useExternalPlayer>
<useInternalPlayer>0</useInternalPlayer>
an external player would be actually used for audio playback, but
neither of the two audio radio buttons would be checked in
Preferences GUI, and audioPlaybackProgram line edit would be disabled.
Whereas, after this commit, an external player is still used for
audio playback, but useExternalPlayer radio button is checked
in Preferences GUI, and the audioPlaybackProgram line edit is enabled.
External and internal audio players work similarly now. Fixes #950.
* inherit a new ExternalAudioPlayer class from AudioPlayerInterface;
* use an existing ExternalViewer class to implement ExternalAudioPlayer;
* take (const char *, int) instead of std::vector<char> in
ExternalViewer constructor to fit into AudioPlayerInterface;
* extend ExternalViewer API to let ExternalAudioPlayer stop superseded
audio player processes;
* make AudioPlayerInterface::play() return an error message string to
allow reporting immediate failures from derived classes;
* Document AudioPlayerInterface API;
* Document AudioPlayerFactory::player();
* use the common audio interface exclusively in ArticleView.
* add a new interface class AudioPlayerInterface;
* inherit a new proxy class Ffmpeg::AudioPlayer from it;
* partially switch ArlticleView to using the interface;
* expose MainWindow's AudioPlayerInterface instance to all ArticleView
instances;
* add a new AudioPlayerFactory class responsible for creating instances
of concrete classes derived from AudioPlayerInterface depending on
relevant Config::Preferences values;
* increase minimum supported Qt version from 4.5 to 4.6 in README
in order to use QScopedPointer introduced in Qt 4.6.
The result of a downcast to a wrong dynamic type is undefined according
to the C++ standard.
Wrong casts were detected at goldendict start by GCC's
-fsanitize=undefined option. This commit fixes #974.
