Differential D11076

Comics Project Reader...
ClosedPublic
Actions

Authored by ragnarb on Mar 5 2018, 9:48 PM.

Details

Reviewers
woltherav
Commits
R37:b19acfb39811: CPMT: Improve comics viewer to allow reading through the whole comic.
Summary

Implementation of most of T8139.

More or less replaces the comic_project_page_viewer, with one that lets you flip through the pages of your comic, and view them as spreads, as in a printed book.

It's functional, but I still want to add a few things to it. I'm uploading this patch now, as I'm a bit stuck on some basic layout stuff, mostly due to very little experience with PyQt/Qt. I really want the pages to expand to the size of the window, but also have the two pages of the double spread be next to each other in the center of the window...and I just can't seem to figure it out. Any tips much appreciated.

Other things that I want to add includes:

  • Add support for reading from right to left
  • Add translation support, or alternately...
  • ...replace the text buttons, with arrows and icons that are overlaid on top of the image, and only show when you move the mouse. This would work well with full screen mode too, but might be problematic for touch screens that don't have a hover cursor.

Known bugs:

  • Doesn't gracefully handle being called if the comic project you have open has no pages in it.
Test Plan
    • Open a comic in the Comic Project Docker
    • Right click a page, and choose View Page in Window
    • Browse through your comics either using the buttons at the bottom, clicking the pages themselves or using the following keyboard shortcuts.
    • Home - Go to first page
    • Back Arrow - Go to previous page
    • Forward Arrow - Go to next page
    • Space - Go to next page
    • End - Go to last page
    • There is no keyboard shortcut to switch between single page view and double page spread
  • Also try resizing the window, to see how the pages scale

The viewer can actually also be run standalone outside Krita. This was mostly for testing when developing, but could be a handy feature. To run it as standalone (at least under Linux), run:

python3 comics_project_page_viewer.py /path/to/your/comicConfig.json

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
ragnarb requested review of this revision.Mar 5 2018, 9:48 PM
ragnarb created this revision.
woltherav added a comment.Mar 5 2018, 10:25 PM

Ah, for forcing the two sides towards the middle, see if you can add a stretcher before and after the two pages: https://doc.qt.io/qt-5/qboxlayout.html#addStretch

We haven't quite figured out how we want to handle translations ourselves. There's a i18n( ) function that you can stick around strings and Krita won't complain, but I am not sure if they're already translated or whether the translations get used already.

ragnarb added a comment.Mar 6 2018, 2:39 PM

I tried adding addStretch to either end of the surrounding QHBoxLayout that the pages are in, but it seems to not work with the size policy MinimimumExpanding or Expanding. With addStretch on either side, they simply stop expanding when they reach their SizeHint. I've attached a simpler example, which I've used to try out different solutions, but nothing I've done so far has worked correctly.

page_layout_test.py2 KBDownload

ragnarb updated this revision to Diff 28873.Mar 6 2018, 10:44 PM

Fixed the alignment issue, so it now works correctly. The issue was that there was a QImage within a QPushButton, and the image needed to be aligned within the button.

Secondly, I've added reading direction support. This involved a minor change in comics_metadata_dialog.py, as the reading direction set there, was not being stored in the .json file.

This patch should be more or less ready to roll now.

woltherav added a comment.EditedMar 7 2018, 6:54 PM

Okay, this is working quite well!

The only bug I found is that if you're on an even-paged comic, and then on the last page, while the comic viewer is in "spread" mode, going to single mode will push the viewer into a page that doesn't exist(so for a 4 page comic, it will show page 5).

Other than that, I added a suggestion to use QAction instead of seperate shortcuts and buttons. That, with QMenu/QToolbar might make the code a lot simpler.

EDIT: I went ahead and pushed a proper fix for the reading direction storage. The prime reason for me to use the enum is readability :p

plugins/python/comics_project_management_tools/comics_project_page_viewer.py
121

It might be an idea to, instead of using seperate shortcuts and pushbuttons, make QActions, assign a shortcut to those, and then make a QToolbutton that the QAction becomes the default action off. OR, even simpler, make a QMenu or QToolbar where you just set the layoutDirection instead of making the long list of removes and adds.

https://doc.qt.io/qt-5/qaction.html#shortcut-prop
https://doc.qt.io/qt-5/qwidget.html#layoutDirection-prop
https://doc.qt.io/qt-5/qmenu.html

ragnarb updated this revision to Diff 29035.Mar 8 2018, 7:42 PM

Use setLayoutDirection to flip the direction of the whole dialog, instead of adding and removing widgets, this flips both the pages and the buttons, which is perfect, and makes the code much simpler. The arrow keyboard shortcuts are pretty much the same as before.

Fixed the bug where you would get no pages at the end, and sometimes at the beginning, when switching from double page spread, to single page.

Updated one line in the comics_metadata file, to get the reading direction to be stored in the json file.

woltherav accepted this revision.EditedMar 8 2018, 9:49 PM

Alright working good here!

This is your third accepted patch, so that makes you eligible for a contributor's account: https://community.kde.org/Infrastructure/Get_a_Developer_Account

Note me down as the person that vouches for you, and when you get access, see if you can get it pushed :)

(You can always ask for help in Krita on freenode, we have an irc client on the website if you're having battles with irc for whatever reason)

EDIT: Also, add your name/email to the license header, you've rewritten so much of it :)

This revision is now accepted and ready to land.Mar 8 2018, 9:49 PM
ragnarb added a comment.Mar 9 2018, 10:38 PM

That's great, but could you please push it for me. I don't know how much more development I'll be doing on Krita this time around, as I've got a 110 page comic book waiting for me to pencil and ink, alongside full time employment, which will likely eat up most of my time. I do plan to come more back to Krita once I'm done with the comic, but that will likely be in around a years time or so. :)

woltherav added a comment.Mar 10 2018, 12:26 AM

Add yourself to the license header then, or give me a line of text to add to the license header first then. It'll remind that you made the viewer into its current incarnation :)

ragnarb updated this revision to Diff 29136.Mar 10 2018, 8:04 AM

Added my name to the licensing info.

Closed by commit R37:b19acfb39811: CPMT: Improve comics viewer to allow reading through the whole comic. (authored by Ragnar Brynúlfsson <me@ragnarb.com>, committed by woltherav). · Explain WhyMar 10 2018, 3:04 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 29035

View Options

plugins/python/comics_project_management_tools/comics_metadata_dialog.py

Loading...
View Options

plugins/python/comics_project_management_tools/comics_project_manager_docker.py

Loading...
View Options

plugins/python/comics_project_management_tools/comics_project_page_viewer.py

Loading...