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 image comparison features to details dialog #683

Merged
merged 70 commits into from
Oct 28, 2020

Conversation

glubsy
Copy link
Contributor

@glubsy glubsy commented Jul 2, 2020

  • Add splitter in order to hide the details table.
  • Add a toolbar to the Details Dialog window to allow for better image
    comparisons: zoom in/out, swap pixmaps in place, best-fit-to-viewport.
    Scrollbars and viewports are synchronized.

Notes about the patch:

  • We only use the ScrollAreaImageViewer class, and the ScrollAreaController here, the other image viewer classes (QWidgetImageViewer and GraphicsViewImageViewer) and their corresponding controllers (QWidgetController and GraphicsViewController) have been left in in case we need them later and for study purposes.
  • The QWidget-based image viewer is the simplest implementation of all, but lacks scrollbars and also has some issues when scaling up and panning around. Probably something fixable but I have not pursued it further.
  • The GraphicsView-based image viewer is more unnecessarily full-fledged at this point and I decided not to used it. The ScrollArea version is more than enough and works slightly better too (there is some minor margin problem with the GraphicsView version which I have not worked on fixing)
  • The QScrollArea-based image viewer is the most balanced of the three and works just as well while not carrying all the "bloat" of the QGraphicsView widget-based implementation.

Screencast video demonstration of the implemented features.

glubsy added 29 commits July 2, 2020 22:36
The controller singleton acts as a proxy to relay
signals from each widget to the other
It should help encapsulating things better if we need to
use a different class for image viewers in the future.
* setPixmap() now disables the QWidget automatically if the pixmap passed is null.
* the controller relays repaint events to the other widget
When swapping images, use getters to hopefully get a reference to
each pixmap and swap them within a single slot.
Using a QWidget inside the QScrollArea mostly works
but we only move around the pixmap inside the QWidget,
not the QWidget itself, which doesn't update scrollbars.
Need a better implementation.
* Work around flickering of scrollbars due to
GridLayout resizing on odd pixels by disabling
the scrollbars while BestFit is active
* Also setting minimum column width to work around
the issue above.
* Avoid updating scrollbar values twice by using a
simple boolean lock
* Also fix scaled down pixmap when updating pixmap in the same group
* Fix ignoring mouse wheel event when max scale has been reached
* Fix toggle scrollbars when asking for normal size
* Needed to ignore the scrollbar changes in the disabled
panel, sine a null pixmap would reset the bars to 0 and affect
the selected viewer.
* Keep view as same scale accross entries from the same group.
* Moved the QToolbar into the image viewer's  translation unit.
* QAction are still attached to the dialog window for shortcuts to work
* Add splitter in order to hide the details table.
* Add a toolbar to the Details Dialog window to allow for better image
comparisons: zoom in/out, swap pixmaps in place, best-fit-to-viewport.
Scrollbars and viewports are synchronized.
* Fix ME and SE versions of details dialog not displaying their content properly after change to QDockWidget
* Add option to toggle titlebar and orientation of titlebar in preferences dialog
* Fix setting layout on PE details dialog window while layout already set, by removing the self (parent) reference in constructing the QSplitter
* For some reason the table's height is a few pixel longer on Windows so we work around the issue by adding a small offset to the maximum height hint.
* No idea about MacOS yet but this might need the same treatment.
* Use custom icons on platforms which do not provide theme
* Old zoom icons credits to "schollidesign" from icon pack Office and Entertainment (GPL licence).
* Exchange icon credit to Jason Cho (Unknown license).
* Use hack to resize viewers on first show() as well
* Jason Cho gave his express permission to use the icon (it was made 10 years ago and he doesn't have the source files anymore)
* Used waifu2x to upscale the icon
* Used GIMP to draw dark outline around the icon
* Source files are included
@glubsy glubsy force-pushed the details_dialog_improvements branch from b71936f to 8827209 Compare July 15, 2020 19:30
glubsy added 5 commits July 30, 2020 15:16
* If details dialog failed to be created for some reason, avoid crashing by dereferencing a null pointer
* Restore the details dialog dock position if it was previously docked (i.e. not floating).
* Since the details_dialog instance was not deleted after closing by default, the previous instances were still saving their own geometry. We now delete them explicitely if we have to recreate a new instance to avoid the signal triggering the callback to save the geometry.
* Since restoreGeometry() and saveGeometry() are only called in our QDockWidget, it should be safe to modify the methods for the Preferences class (in qtlib).
* Avoid attempting to add a QLayout to DetailsDialog which already has a layout by removing superfluous layout setup.
@glubsy
Copy link
Contributor Author

glubsy commented Jul 31, 2020

Seems to work fine in MacOS Catalina:
simplescreenrecorder-2020-08-01_01 54 00

* On Linux, even with 1 pixel size, the button is filled entirely with the color selected
* On MacOS, the color pixmap stays at 1 pixel so we hard code the size to 16x16
@arsenetar
Copy link
Owner

@glubsy, the tab PR is merged so you can work out the conflicts here when you get a chance.

glubsy added 2 commits August 1, 2020 16:42
* We use GroupBoxes to group items together and surround them in a frame
* Remove separator lines to avoid cluttering
* Adjust columns and their stretch factors for better alignment of buttons
QFormLayout should adhere to each platform's style better. It also simplifies the code a bit since we don't have to setup the labels, etc.
@glubsy
Copy link
Contributor Author

glubsy commented Aug 1, 2020

The display tab in the preference dialog should look slightly better now:
2020-08-01_18-54-33
2020-08-01_18-55-07
Edit: updated versions

glubsy added 3 commits August 1, 2020 18:29
* It seems that stretching the last section automatically is a bit inconvenient on MacOS as it will grow beyond the window border.
* Keep it as it was before for now until a better solution is devised.
* Use QGroupBox to keep items together on the display tab in the preference dialog just like for the other options.
* It is probably not be necessary to keep these as class members
@glubsy
Copy link
Contributor Author

glubsy commented Aug 1, 2020

I think we're good here, not much to add anymore for now. We might need to start considering releasing 4.0.5 soon. ;)

glubsy added 6 commits August 1, 2020 19:02
* The close button on custom tabs cannot be hidden on MacOS for some reason.
* Prevent the directories tab from closing if the close button was clicked by mistake
* Whenever the Result Window already existed and its tab was in second position, and if the ignore list tab was in 3rd position, asking to show the Result window through the View menu would add a new tab and push the Result tab to the third position (ignore list tab would then become 2nd position).
* Fix view menu Directories entry not switching to index "0" in custom tab bar.
* When right clicking on image viewers while they are docked, the context menu of the Results window showed up.
* This also enables capture of right click and middle click buttons to drag around images, which solves a conflict with some theme engines that enable left mouse button click to drag a window's position regardless of where the event happens, hence blocking the panning.
* Probably unnecessary to check which button is released.
Copy link
Owner

@arsenetar arsenetar left a comment

Choose a reason for hiding this comment

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

Reviewed these changes, had a few minor items.

qtlib/preferences.py Outdated Show resolved Hide resolved
qt/se/details_dialog.py Outdated Show resolved Hide resolved
qt/se/details_dialog.py Outdated Show resolved Hide resolved
qt/me/details_dialog.py Outdated Show resolved Hide resolved
qt/me/details_dialog.py Outdated Show resolved Hide resolved
qt/dg.qrc Show resolved Hide resolved
qt/pe/details_dialog.py Outdated Show resolved Hide resolved
qtlib/about_box.py Outdated Show resolved Hide resolved
* Moved credits to CREDITS file
* Updated exchange icon with higher hue contrast for better visibility on dark backgrounds
@arsenetar arsenetar merged commit 8d9933d into arsenetar:master Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants