Before this commit folder with no direct comics (only subfolders) didn't have a cover image to be displayed. Now updating the children info is done recursively and subfolders are taken into account.
This affects the iOS client remote browser so far, but it is also needed for the future browser update (display folders as a grid instead of using EmptyFolderWidget)
Otherwise we can end with huge cover files if the source content has abnormally tall covers. This large files could end exhausting RAM in the iOS client.
Creating a folder in search mode selects it and makes the UI look
half-way between Normal and Searching navigation statuses.
An alternative fix is to disable addFolderAction in search mode. But
this is more difficult to implement and inconsistent with the other
always-enabled folder and reading list actions.
ConcurrentQueue is currently used only by two classes and a test, but
modifying concurrent_queue.h requires recompiling 30 source files. None
of the member functions is so lightweight as to make it worth inlining.
An alternative to `@note ConcurrentQueue is unable to execute jobs if
@p threadCount == 0.` is `assert(threadCount != 0);`. But this would
force classes that contain a ConcurrentQueue data member to always start
a thread, even if they detect at runtime that they are never going to
enqueue a job.
Add Job type alias to avoid repeating the type.
Use default member initializers instead of the member initializer list
to make it clear [to the reader of the header] that no data member is
left uninitialized.
There is a bunch of QButton and similar widget connnections which
cannot be converted to new slot syntax automatically.
Fix them by hand and bundle them for testing.
When a key not handled by Grid view is pressed while it has focus, the
following warning appears in Library's output
qrc:/qml/GridComicsView.qml:776: Error: Cannot assign [undefined] to int
AND the top-left cover is selected. The added early return fixes both
issues.
The Ctrl+Q shortcut is assigned to Quit action in most applications on
GNU/Linux. Command+Q is used on macOS. The added shortcut should be
automatically mapped to Command+Q on macOS judging by the following
quote from QKeySequence class documentation:
Note: On macOS, references to "Ctrl", Qt::CTRL, Qt::Key_Control and
Qt::ControlModifier correspond to the Command keys on the Macintosh
keyboard
QKeySequence::Quit could be used as the default key sequence in place of
`Qt::CTRL | Qt::Key_Q`. This would leave the shortcut unassigned by
default on Windows. But YACReader doesn't use QKeySequence::StandardKey
anywhere, so perhaps this shortcut should be hard-coded too.
The shortcut is particularly useful when Close to tray option is
enabled, because in this case closing the Library window with a system
window manager shortcut simply hides it.
QWidget::sizeHint() is const-qualified, so Clang warns that non-const
sizeHint() member functions merely hide the virtual function of the base
class.
664dac3401 and
9f53ae6efc introduced these member
functions in 2014 without const qualifiers. QWidget::sizeHint() was
const-qualified even in Qt 3. Since these member functions have never
had any effect, they should be removed rather than const-qualified to
preserve the long-standing behaviors of the two classes.
Add a TODO for a similar but less straightforward issue with
PropertiesDialog::sizeHint().
Focusing the current comics view allows to use keyboard arrow keys to
choose among the visible comics.
The shortcut for this new action should not be a single character
without modifiers because it won't work when the search line has focus.
The Qt::FocusReason parameter in ComicsView::focusComicsNavigation()
allows to reuse this function for other keyboard navigation features.
For instance the search line can transfer focus to comics navigation
when the user presses Return or Enter key. In this case
Qt::OtherFocusReason can be used (an application-specific reason).
The Ctrl+F shortcut gives focus to a search bar in many applications.
In this case it allows to search the library without touching a mouse.
YACReaderMacOSXSearchLineEdit::setFocus() will have to be implemented to
make the shortcut work on macOS.
Qt OpenGL in Qt5 is a deprecated module that is discouraged for
new code usage. We have been including this module in our builds
despite not relying on its functionality for a long time now -
probably an oversight from porting to the newer functions.
Time to remove it.
IMPORTANT INFORMATION: In Qt6, a lot of functionality that was
provided by Qt GUI was moved into the 'new' Qt6 Qt OpenGL module.
Thus, even if it makes perfectly sense to remove it for Qt5 builds
we will likely have to restore it for Qt6 builds at a later time.
In the past, translation files provided by the community
mostly came as pregenerated .qm files missing the corresponding
.ts sources. This has led to a situation where the translations
are out of sync with the sources and the sources have not been
updated for several release iterations.
To improve the situation, this commit syncs all .qm files back to
the sources by using the lconvert tool to create .ts files and
updating these files against our sources using lupdate.
For future updates, a CI solution would be preferable.
Before this commit starting a search when an empty folder or an empty
reading list was selected left all comics actions disabled. Fixes#213.
When search mode is exited, we always call either
YACReaderNavigationController::loadFolderInfo() or
YACReaderNavigationController::loadListInfo(). Both of them call
LibraryWindow::disableComicsActions(), so the enabled/disabled state of
the comics actions stays up-to-date at all times.
Currently these objects are created once at program startup and are
never destroyed. Printing debug messages in the models' destructors
confirms the leaks and proves that with this fix the objects are
destroyed at Library exit.
When Back or Forward action was triggered, the toolbar title was not
updated to match the reselected entry.
I am calling LibraryWindow::setToolbarTitle() from
selectedIndexFromHistory() rather than loadIndexFromHistory(), because
the latter is also called from
YACReaderNavigationController::loadPreviousStatus(), which in turn is
called only from LibraryWindow::setSearchFilter() when the search line
text becomes empty. The toolbar title is already correct and does not
have to be updated in this case.
My code analysis and experiments have revealed that YACReader code never
creates YACReaderLibrarySourceContainer objects of type None. This type
could be removed altogether along with YACReaderLibrarySourceContainer's
default constructor, but for Q_DECLARE_METATYPE macro's requirement. So
YACReaderNavigationController::loadIndexFromHistory() now simply prints
an error message instead of introducing a failure condition by returning
false when the type is None.
finished() signal of both FoldersRemover and ComicsRemover was not
connected to their QThread's quit() slot. So the thread kept running
after the deletion completed. The QThread's parent is LibraryWindow.
Thus LibraryWindow's ~QObject() invokes the QThread's destructor.
As a result, when the user exited YACReader Library after deleting at
least one folder or comic, it printed the following FATAL message and
crashed at exit: "QThread: Destroyed while thread is still running".
Extract signal-slot connections between a remover and a QThread into
moveAndConnectRemoverToThread() to reduce code duplication.
Remove always true (thread != NULL) checks.
The function doesn't use data members or other member functions. It
could even be put into an unnamed namespace in the cpp file, but that
would require more changes and complicate turning it back into a member
function if need be in the future.
InfoComicsView constructor is the only function that connects to
FlowView's currentCoverChanged signal. Neither of the slots connected to
this signal handles the argument value index==-1. So when FlowView emits
this signal with index==-1, YACReaderLibrary crashes. Returning early
from either ComicsView::updateInfoForIndex() or
InfoComicsView::setCurrentIndex() when index==-1 is not sufficient - the
crash happens in the other slot then. Let us skip emitting the signal in
FlowView if index==-1 rather than return early from both slots.
Steps to reproduce 1:
1. Launch YACReaderLibrary version that matches the version of the
default library database. Alternatively, select a compatible library
after starting the application.
2. If InfoComicsView is not active, switch to it.
3. (optional) Switch to another comics view out of InfoComicsView.
4. Quit YACReaderLibrary. The application crashes during exit - after
the "YACReaderLibrary closed with exit code : 0" message is printed.
Steps to reproduce 2:
1. Launch a YACReaderLibrary version newer than the version of the
default library database.
2. Click the "No" button in the "Update needed" dialog that pops up.
3. Change between comics views until InfoComicsView becomes active. If
this view was active at the beginning, switch through all the views to
get back to it. At this point YACReaderLibrary crashes.
This way we can tell the app that a folder contains mangas so the user doesn't have to constantly set comics as manga when new issues are added. And it should be easier to set all the content in a folder as manga from the folder tree.
List initialization ended using movable constructors which surprisingly caused data troubles in release mode, at least in VC2019 compiler. The tree being messed up caused crashes while SQL was generated.
I have no explanation for it.
When setting ports, temporary or for good, we need to go via the config
files and not QTcpServer or we get undefined behavior. To support temp
ports, we need to back up the fixed port in the settings.
In a later commit WorkerThread should also replace classes similar to
ImageLoader: PageLoader, ImageLoaderGL and ImageLoaderByteArrayGL.
Bugs fixed:
1. Eliminated a data race between ImageLoader::run() and
ComicFlow::updateImageData()->ImageLoader::result(). Specifically when
ImageLoader::busy() returns false, then ImageLoader::run() sets
ImageLoader::working to true, loads the image and starts assigning it to
ImageLoader::img, while ImageLoader::result() is accessed without
locking from updateImageData().
Making ImageLoader::working atomic is clearly insufficient to eliminate
this data race. The fix is to set 'working' to true immediately and
synchronously as soon as a new task is assigned to the worker.
2. Replaced thread termination with graceful thread exit. ComicFlow
destructor called QThread::terminate(), using which is discouraged by Qt
documentation. The application exited without errors in Release mode.
In Debug mode, however, it received the SIG32 signal on exit and printed
the following warning - "QWaitCondition: mutex destroy failure:
Device or resource busy".
The loop in WorkerThread::run() is no longer endless. The worker thread
properly ends and is joined in WorkerThread destructor.
Design decisions:
1. WorkerThread could emit a signal when it completes a task.
Thus updateTimer could be removed from ComicFlow and GoToFlow. However,
there is no obvious way to use this new signal in the two GL classes.
Also I don't know whether updateTimer is just an inefficient polling
substitute for notification or an intentional animation mechanism.
2. The index variable is no longer stored in the worker class, but in
ComicFlow directly. Thing is, this data member was never actually
accessed by the worker, but ComicFlow went so far as to lock worker's
mutex to "protect" access to the index.
3. The common ImageLoader implementation turned out to be very general.
So I converted it into the WorkerThread class template that is not
restricted to producing QImage results and can be reused elsewhere.
4. I used standard classes (such as std::thread) instead of their Qt
equivalents (e.g. QThread) because they are more thoroughly documented.
The standard classes should also be more efficient as they were more
carefully designed and provide much fewer unnecessary features.
5. Release-Acquire ordering is safe for the WorkerThread::working
use case and is more efficient than the std::atomic-default
Sequentially-consistent ordering.
6. condition.notify_one() is called while the mutex is unlocked
to improve performance. This is safe in both cases:
a) if the worker thread exits due to a spurious wakeup just before
the condition.notify_one() call in WorkerThread destructor, so much the
better;
b) if a spurious wakeup lets the worker thread finish the task and
start waiting on the condition again just before the
condition.notify_one() call in WorkerThread::performTask(), the second
waking will be ignored by the worker thread as 'working' and 'abort'
will be false then.
Manual editing of a config file for setting a port is not ideal.
Solution: add a set-port command to save a port and also a
--port option to allow setting a temporary port during startup
Qt's database and query model requires that both the queries and the database
objects are out of scope before a database connection can safely be removed.
Solution: Properly encapsulate databases and queries in "{ }" and use a string
to cache the connection name for out-of-scope removal.
- Adapt server code for QtWebapp namespace 'stefanfrings'
- Implement custom modifications needed by v1 controller
via template engine
- Unify iphone and ipad templates
server_config_dialog.cpp:57:30: error: member access into incomplete type 'struct sockaddr'
if (ifa->ifa_addr->sa_family == AF_INET) { // check it is IP4
^
/usr/local/include/qt5/QtNetwork/qhostaddress.h:50:8: note: forward declaration of 'sockaddr'
struct sockaddr;
^
server_config_dialog.cpp:61:27: error: use of undeclared identifier 'AF_INET'
inet_ntop(AF_INET, tmpAddrPtr, addressBuffer, INET_ADDRSTRLEN);
^
server_config_dialog.cpp:64:37: error: member access into incomplete type 'struct sockaddr'
} else if (ifa->ifa_addr->sa_family == AF_INET6) { // check it is IP6
^
/usr/local/include/qt5/QtNetwork/qhostaddress.h:50:8: note: forward declaration of 'sockaddr'
struct sockaddr;
^
server_config_dialog.cpp:68:27: error: use of undeclared identifier 'AF_INET6'
inet_ntop(AF_INET6, tmpAddrPtr, addressBuffer, INET6_ADDRSTRLEN);
^
Stopped QThreads don't process events, so cleanup signals get lost.
Prevent this from happening by keeping the threads alive and the comic
inside the thread (as we already do in the viewer). Cleanup happens by
connecting the comic's destroyed() signal to the thread's quit() slot.
Adds vertical navigation (up down) to FlowView. This corresponds visually to the File Name list display -- especially when the flow pane is hidden, pressing up-down is the intuitive way to navigate up-down in the list.
There were many run-time warnings in YACReaderLibrary built in Debug
mode with hardware acceleration disabled and with ClassicComicsView
as the active comics view:
QWidget::paintEngine: Should no longer be called
QPainter::begin: Paint device returned engine == 0, type: 1
The ComicFlowWidgetSW::paintEvent() implementation now corresponds to
ComicFlowWidgetGL::paintEvent(), which has been fixed earlier.
QWidget::repaint() calls paintEvent() immediately.
PictureFlow::paintEvent() calls d->renderer->paint(). So the
d->renderer->paint() call in PictureFlow::updateMarks() was redundant.
