I agree...but i think it will be only after release.

Created attachment 34907 [details]
Patch against SVN Rev. 989103. Improved "show cover" dialog.

Hi,
Hi,

I reimplemented the dialog, it now automatically centers on the screen and it's size fits the cover's size. You can zoom out and in on the cover with the mouse wheel. The window size automatically adapts on the new cover size and centers on the screen everytime the zoom factor has changed. If the cover's original size is greater than the desktop's resolution, it is scaled to fit onto the desktop before the dialog pops up. In this implementation, there is no more need for scroll bars, so i removed them...

For suggestions or criticism please contact me, I would love to help further in amarok development :)

Created attachment 34909 [details]
corrected previous patch

- corrected formatting (tabs to spaces, indentation, length of lines, etc...)
- changed names of instance variables according the conventions (m_xxxx)
- patch can now be applied inside the amarok directory

Created attachment 34911 [details]
some more improvements...

the implementation for covers larger than the desktop resolution wasn't good. I corrected it now...

SVN commit 989288 by nhnielsen:

Improvements to the "show cover" dialog.

- Auto center on screen
- Size matches cover size (if it fits on screen, scales down cover otherwise)
- Zooming/resizing using mouse wheel
- No more scrollbars

Thanks to Pascal Pollet <pascal@bongosoft.de> for the patch! :-)

BUG: 175901



 M  +92 -31    PixmapViewer.cpp  
 M  +18 -11    PixmapViewer.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=989288

Whoops! :-)

Pascal, I missed your most recent patch with the changes to the handling of large covers as I had already gotten the previous patch ready for comitting before I had to leave for a while. I committed it when I got back and did not see that you had created yet another one. Could you please update from svn, and create a patch with just the last changes? With any luck, svn should correctly merge stuff if you just update without reverting your local changes.

Hello Nikolaj,

yes, I was a bit chaotic with my many patches ;) Thank's for having
reformatted the code! I'll make a new patch on this basis, it's not much
work because it were only 2 or 3 lines of code that changed...

bye
Pascal



Nikolaj Hald Nielsen schrieb:
> https://bugs.kde.org/show_bug.cgi?id=175901
>
>
> Nikolaj Hald Nielsen <nhnFreespirit@gmail.com> changed:
>
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |nhnFreespirit@gmail.com
>
>
>
>
> --- Comment #7 from Nikolaj Hald Nielsen <nhnFreespirit gmail com>  2009-06-29 23:24:11 ---
> Whoops! :-)
>
> Pascal, I missed your most recent patch with the changes to the handling of
> large covers as I had already gotten the previous patch ready for comitting
> before I had to leave for a while. I committed it when I got back and did not
> see that you had created yet another one. Could you please update from svn, and
> create a patch with just the last changes? With any luck, svn should correctly
> merge stuff if you just update without reverting your local changes.
>
>

Created attachment 34935 [details]
better handling of multiple screens. Better handling of large covers.

Created attachment 34939 [details]
[patch] multiple screens: Cover should popup on the same screen than the parent window.

Please test it, I don't have multiple screens here :)

like reported by testers the placement of the window with multiple monitors doesn't seem to work like intended now. The window should open on the same monitor than the main window of amarok. I will try to fix this issue later when I'm sitting on a PC with multiple monitors...

could the repaint flickering when zooming be reduced somehow? its a bit choppy because first the window is moved and then resized, can this be made smoother?

it's a good point Simon. I removed a repaint() call that was too much, and a little bit of code in the paintEvent function that wasn't needed anymore, and now it resizes much more smoothly.

I wait to send the patch until I solved the "multiple screens" problems, it let me go crazy yesterday evening, Qt4 seems to be a mess regarding the handling of more than one screen :(

Created attachment 34977 [details]
[patch] resizing is much smoother now. Removed centering of window.

hmmm, no matter what I'm programming, the "move" command will always move the window to the secondary screen on a dualview setup, I really tried everything! I followed exactly the instructions in the Qt4 API, but there is really no way to control to which monitor the window will be moved... I'm quite sure it is a bug in the Qt4 API, nobody could really help me in the Qt-IRC... 

I now completely removed the code for the "move" command, so that the window doesn't get centered on the screen anymore... But at least, it will stay on the same monitor as the parent window. If someone is interested in the not working code for moving the window to try to debug it, please ask me...

in return, the resizing of the window is much smoother now :)

SVN commit 990288 by nhnielsen:

More "Show Cover" improvements by Pascal Pollet <pascal@bongosoft.de>
- Much smoother zooming.
- No longer try to center on screen as that causes issues on multi screen setups.

CCBUG: 175901



 M  +7 -1      covermanager/CoverFetchingActions.cpp  
 M  +6 -3      covermanager/CoverViewDialog.h  
 M  +14 -30    widgets/PixmapViewer.cpp  
 M  +1 -1      widgets/PixmapViewer.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=990288