Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Some basic improvements & fixes #185

Closed
wants to merge 8 commits into from
Closed

Conversation

luebking
Copy link
Contributor

@luebking luebking commented Mar 8, 2015

Thanks for allowing me to get rid of gwenview.

I don't know why this took place,
but it's *very* prone to infinite
looping (which occurred here on
changing views)
copy & paste stuff and forgotton safe
the function does that anyway and keeping an
extra setting is just prone to cause out-of-sync
states
nor restoreState/Geometry again from cliFileName mode

This seems to be/have been some workaround for something,
but it's wrong - esp. because the dock widths are NOT
correct when the dock was never shown for the cliFileName
mode

No problem spotted w/ simplified code so far
This allows to copy/move files into a filebrowser
or onto the desktop, mail attachments and whatnot

NOTICE:
there's a severe DnD bug in Qt 5.4.0 - it doesn't
work at all (regardless of this or any DnD implementation)
* can resolve ~/
* autotrails a / on completion
* use lazy childcount (interested in dirs only anyway)
@oferkv
Copy link
Owner

oferkv commented Mar 9, 2015

Hi! Sorry about the delay, will integrate ASAP!

@oferkv
Copy link
Owner

oferkv commented Mar 9, 2015

Hi luebking,
Thanks for the code, unfortunately, it causes (commit d7522d1) an old bug to re-appear, the bug is when you open an image and close it when in full screen mode (try multiple times), the dock sizes change (one on top is enlarged and the other is shrunk) when coming back to thumbnails.
The whole mess with these(that you removed): pvDock->setMaximumHeight(QWIDGETSIZE_MAX);
what what keeping it OK... What do you think?

@luebking
Copy link
Contributor Author

luebking commented Mar 9, 2015

Indeed.

This is actually unrelated to the fullscreen mode and caused by ::setDocksVisibility(bool)
The docks are shown first and then space is required by the status/toolbars, which is taken from the docks in an uneven way.

As long as tool & statusbars are in a fixed position, the could be covered by showing them before resp. hiding them after the docks, but that solution doesn't scale, nor is very elegant.

I'll look for some better fix.

prevents resizes of docks when changing view modes
@luebking
Copy link
Contributor Author

luebking commented Mar 9, 2015

Is the commit da0cef3 automagically added to the pull request? (I've never done this before ;-)

@oferkv
Copy link
Owner

oferkv commented Mar 10, 2015

Hey, yes it was, but it doesn't fix it...

@luebking
Copy link
Contributor Author

sigh
Yes, and that one's fullscreen related.

Background:
::hideViewer() calls showNormal() what will withdraw the fullscreen hint from the window.
At "some point in the near future"™ the WM will receive that event, process it and restore the window size - that point is however very likely faaaar beyond the continuation of ::hideViewer() - this may be the cause for the even processing loop?

However, the docks etc. are shown while the window has still its fullscreen size and when its resized, the docks will shrink with it - unevenly.

Right now, I've no idea how to do this "correctly" w/o splitting the function (ie. showNormal() first and continue hideViewer() when the window state actually changed) - altering the order in show/hideViewer won't work since it just alters the problem location to showViewer(), saving and restoring the state will still encounter a resize... that could take a while.

before showing docks.
This is a bit hackish, but better solutions require X11 dependency
@oferkv
Copy link
Owner

oferkv commented Apr 18, 2015

Hi luebking;

Sorry for the delay, I merged the following commit, thanks:
@luebking fix bookmark visibility handling … 9ad0568

Regarding this one, I tested it dragging an image from the thumbviewer to my XFCE deskop, or from Thunar to the thumbviewer, and it does not work:
@luebking support drag and drop of url mime … 836314d

Regarding this, do you actually have a use case where endless loop occurs her? (it was done on purpose, because some refreshing was not complete unless doing so...)
@luebking don't processEvents in a loop …

Regarding these, as you saw these are sensitive... once you have a solution without degradation we will talk:
@luebking rely dock/toolbar on QMainWindow::restoreState() … c1b7727
@luebking do not fool around with max geometries … d7522d1

Regarding this, I was not able to see any difference in user experience, what are the benefits of this?
@luebking have a more sophisticated dircompleter … 7df944a

Thanks!
Ofer

@luebking
Copy link
Contributor Author

Dragging INTO thumbviewer isn't implemented (nor suggested) - phototonic is no filemanager.
I don't know whether the xfce desktop supports DnD of wallpapers at all, tested on plasma-desktop and be::shell, also filemanagers like dolphin (no, that's not a Qt thing - I expect dragging FROM phototonic INTO thunar to work as well)


QT5 build, run phototonic, show doubleclick an image (shows fullscreen) press escape -> Phototonic maxes out one core forever, Phototonic::hideViewer() never finishes. 100% reproducible.

Stacktrace:

#0 0xb60d66a3 in call_pselect6 () from /usr/lib/libc.so.6
#1 0xb60cebbd in pselect () from /usr/lib/libc.so.6
#2 0xb65bbd3a in qt_safe_select(int, fd_set
, fd_set
, fd_set_, timespec const_) () from /usr/lib/libQt5Core.so.5
#3 0xb65bd763 in QEventDispatcherUNIXPrivate::doSelect(QFlagsQEventLoop::ProcessEventsFlag, timespec_) () from /usr/lib/libQt5Core.so.5
#4 0xb65bdd16 in QEventDispatcherUNIX::processEvents(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib/libQt5Core.so.5
#5 0xb127cddf in ?? () from /usr/lib/qt/plugins/platforms/libqxcb.so
#6 0xb6565a7d in QCoreApplication::processEvents(QFlagsQEventLoop::ProcessEventsFlag) () from /usr/lib/libQt5Core.so.5
#7 0x08080eaa in Phototonic::hideViewer() ()
#8 0x080a37ee in Phototonic::qt_static_metacall(QObject_, QMetaObject::Call, int, void**) ()
#9 0xb6598291 in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib/libQt5Core.so.5
#10 0xb65988dd in QMetaObject::activate(QObject_, QMetaObject const_, int, void*) () from /usr/lib/libQt5Core.so.5
#11 0xb6e324e9 in QAction::triggered(bool) () from /usr/lib/libQt5Widgets.so.5
#12 0xb6e34d77 in QAction::activate(QAction::ActionEvent) () from /usr/lib/libQt5Widgets.so.5
#13 0xb6e354e3 in QAction::event(QEvent
) () from /usr/lib/libQt5Widgets.so.5
#14 0xb6e3d6ea in QApplicationPrivate::notify_helper(QObject_, QEvent_) () from /usr/lib/libQt5Widgets.so.5
#15 0xb6e42ec1 in QApplication::notify(QObject_, QEvent_) () from /usr/lib/libQt5Widgets.so.5
#16 0xb65659ea in QCoreApplication::notifyInternal(QObject_, QEvent_) () from /usr/lib/libQt5Core.so.5
#17 0xb68afa7e in QShortcutMap::dispatchEvent(QKeyEvent_) () from /usr/lib/libQt5Gui.so.5
#18 0xb68afc08 in QShortcutMap::tryShortcutEvent(QObject_, QKeyEvent_) () from /usr/lib/libQt5Gui.so.5
#19 0xb6e42ce7 in QApplication::notify(QObject_, QEvent_) () from /usr/lib/libQt5Widgets.so.5
#20 0xb65659ea in QCoreApplication::notifyInternal(QObject_, QEvent*) () from /usr/lib/libQt5Core.so.5

Whatever the purpose was, it's wrong.
Processing events in a loop means that you expect an event to post more events and that means your heading for an infinite loop.

As the commit says, the dircompleter resolves "~" (eg. to /home/ofer), adds trailing "/" on completion (so you can type on w/o having to finish the completed path) and does saves I/O by not inspecting the dir to get its childcount (because we only list dirs anyway)


commits 592e79b & da0cef3 should resolve issues caused by c1b7727
& d7522d1 - and provide slightly better performance as well.

@oferkv
Copy link
Owner

oferkv commented Apr 18, 2015

I see, merged the following, great work:
@luebking support drag and drop of url mime … 836314d
@luebking have a more sophisticated dircompleter …

As for the following, disabled the first one, and added a limit to the second one with a magic number (did some checking), credit is given, thanks:
@luebking don't processEvents in a loop …

@oferkv
Copy link
Owner

oferkv commented May 9, 2015

Hi luebking;

Regarding your comment:

"commits 592e79b & da0cef3 should resolve issues caused by c1b7727
& d7522d1 - and provide slightly better performance as well."

I am afraid that there issues with these still, one: there is degradation after returning from full screen, you can not right click on tool bars, and two, the "hack" code looks really nasty :)
If you wish to fix, please rebase and submit a new pull.
Closing this pull, thanks again for this fantastic work!

@oferkv oferkv closed this May 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants