Created attachment 49418 [details]
patch fixing this issue here

Thanks for the patch, I'll look into it. However removing the ->show() call introduces other issues, which are not noticeable in the first sight: Tooltips will have a wrong size dependent on the previous state of a tooltip without this.

Well, I've read your comment on that code (suggesting wrong tooltip size) but I haven't observed it's actually true. Tooltips look well, layout inside is fine as well and enabling kwin shadow effect (which, I guess would expose wrong tooltip size) doesn't produce any artifacts. So I'm definitely unable to reproduce here the issue you're mentioning.

SVN commit 1153521 by ppenz:

Fix visual artefacts for tooltips, if compositing has been disabled. Thanks to Maciej Mrozowski for analyzing the root cause! Also a minor issue is fixed with the upper/left position: It may never overlap with the icon area.

BUG: 245491


 M  +6 -3      tooltipmanager.cpp  


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

SVN commit 1153522 by ppenz:

Fix visual artefacts for tooltips, if compositing has been disabled. Thanks to Maciej Mrozowski for analyzing the root cause! Also a minor issue is fixed with the upper/left position: It may never overlap with the icon area.

CCBUG: 245491


 M  +6 -3      tooltipmanager.cpp  


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

Polluting secondary screen:
1. Set up multiscreen (TwinView, XRandR, etc)
2. Enable dolphin tooltips
3. Open dolphin on first screen and hver over dolphinpart items to show
tooltips. Tooltips will be visually rendered on second screen (which is not
expected).

This issue is still present (compositing disabled or enabled - doens't matter).
my setup - latest KDE SC 4.5 branch, Qt-4.6.3, nvidia-drivers-173.14.27

Please provide information about your setup and preferably screenshot showing that those tooltips have *actually* wrong size. So that:
if (KWindowSystem::compositingActive()) {
        const QRect desktop = QApplication::desktop()->screenGeometry(m_itemRect.bottomRight());
        m_fileMetaDataToolTip->move(desktop.bottomRight());
        m_fileMetaDataToolTip->show();
}

would be justified. Because from my observation this is some attempted hack, (which additionally doesn't work).

To reproduce the wrong size issue:
- Enable Nepomuk
- Add an extreme long comment to one file without spaces
- hover file 1 with comment (tooltip is shown on the left, as the text will not get wrapped)
- hover file 2 with no comment

Result:
Left position of tooltip for file 2 is wrong, as the width from the previous tooltip state is taken.

Also trying to open a tooltip near the bottom left of the screen requires to have a 100 % valid size, otherwise the upper left position cannot be calculated and the tooltip will appear/disappear...

Would it be possible for you to check whether using desktop.topLeft() - tooltipSize might do the trick? We just have to find a position, where we can be sure that it is not visible. Thanks in advance!

ah, there are some types on the comment above:
- "Add an extreme long comment to one file without spaces" -> "Add an extreme long comment without spaces to one file"
- "Also trying to open a tooltip near the bottom left..." -> "Also trying to open a tooltip near the bottom right..."

Created attachment 49463 [details]
another try

Indeed, issue is there.
But I hereby refuse to work around bad layout using off-screen rendering.

Here's what i did:
- simplified FileMetaDataToolTip - don't use additional widget between QHBoxLayout and QVBoxLayout. This ensures all "invalidate layout" requests are properly propagated to child layouts so now m_fileMetaDataToolTip->adjustSize(); actually works.
- place tolltip not on the screen containg item, but screen containing pointer - it's more logical imho
- restrict size of tooltips so that they fit on screen in question
- (not important - use "public" kde headers + some accidental trailing whitespace removal)

Hmm, wait.
Now when I think about it, when layout invalidation is now properly propagated, the code:
+    // Restrict tooltip size to rough estimation (excluding borders and actual file preview size)
+    // of available space on current screen.
+    const QRect screen = QApplication::desktop()->screenGeometry(QCursor::pos());
+    m_fileMetaDataWidget->setMaximumSize(0.9f * (screen.width() - 256), 0.9f * (screen.height() - 100));

can be actually moved to TooltipManager (which should give us more knowledge about destination screen and tooltip size). Let me experiment.

Thanks a lot for investigating into this. I did not have the time yet to test your patch, but I would be very happy to apply it to get rid of those ugly outside-screen-rendering hacks...

Created attachment 49474 [details]
dolphin-tooltips-on-multiscreen

Ok, that's more like it.

Changes (against branch/trunk):
* fix layout in FileMetaDataToolTip:
  - remove unnecessary stretchWidget (use Qt::AlignTop instead)
  - nest textLayout (QVBoxLayout) directly inside tipLayout (QHBoxLayout(this) - main)
    to get proper invalidation propagation so that adjustSize on FileMetaDataToolTip works
  - align KFileMetaDataWidget to left (looks nicer imho - to check - show tooltip for
    some file with long name)
  - do not set SetFixedSize constraints yet, we'll manage it in ToolTipManager
* ToolTipManager changes:
  - make tooltips appear on screen containing the pointer
  - remove off-screen tooltip rendering in favour of 3 steps below
  - restrict tooltips size to screen size in 3 steps
    1) let layout calculate fixed sizeHint for tooltip widget
    2) trim requested tooltip dimensions to screen dimensions if necessary
    3) set tooltip geometry
  - fix tooltip positioning in two cases
* Minor/accidental cleanup (trailing whilespaces, use public headers)

Possible enhancement:
- trim tooltip size to actually available space on screen (respecting m_itemRect, previously and currently such tootips won't be shown at all)

-    
-    int x = 0;
-    int y = 0;
+
+    int x, y;

One (yet) undocumented change - to make compiler warn when any of those are unhandled in code.

*** Bug 245725 has been marked as a duplicate of this bug. ***

The patch looks good, but sadly it does not resolve the issue and still the size of the tooltips depends on the previous size. To reproduce:
- Hover a directory with no content (this results in a tooltip with a quite small height)
- Hover an image (this results in more meta data enries, as e. g. width and height are shown)

Result: The height is as large as the previous tooltip and items are clipped (see attachment).

I'll try to use your patch as base now and hopefully have some time to investigate further...

Created attachment 49492 [details]
Too small height

Attached is a patch based on your patch and simplifies some other things in the tooltipmanager: 2 timers and 2 boolean flags have been removed. The earlier version tried to be "too clever" and wanted to show a default icon until the preview has been received (however this results into possible size changes and just makes the code unnecessary complex).

I found out the root cause of the wrong size and am a very annoyed about myself: KFileMetaDataWidget::setItems() loads the items *asynchronously*. Dependent on whether this request has been fulfilled before the tooltip will be opened or not, the size from the previous state is taken.

I'll have a look on this during the next days, it might be necessary to also adjust KFileMetaDataWidget...

Created attachment 49496 [details]
Updated patch

@Maciej: I could fix the issue now based on your patch :-) I needed to add a signal to KFileMetaDataWidget of kdelibs, so I need to wait until next Monday until I can commit the patch. However I'm very glad, as because of your patch those ugly hacks could be removed and the code got a lot simpler.

Interestingly enough I couldn't reproduce your 'previous height' issue when using my latest attached patch (no other changes against 4.5 branch either in kdelibs or dolphin). But as it's loaded asynchronously it may be indeed tricky to reproduce.
Your patch (the one using mine as a base and removing some slots) actually introduces your issue for me but i suppose it's expected now :)
I agree that code should be kept as simple as possible, so I'm staying tuned.
I can take a look as well, but cannot promise anything, those parts with signals and timers are black magic to me for now.

err, .^ (the one using mine as a base and removing some *timers*) ^

> Your patch (the one using mine as a base and removing some
> slots) actually introduces your issue for me but i suppose
> it's expected now :)

Yes, my adapted patch makes this issue easier to reproduce, but I could also reproduce it with your original patch :-) It even should be reproducible with the original "open the tooltip offscreen" hack, but I guess I was just lucky there to get the meta data before the tooltip was made visible.

Well, I'll continue to test my local patch during this week before merging it, but I'm very confident that it should fix this issue in a clean way.

SVN commit 1158057 by ppenz:

Remove the workaround to show the tooltip temporary on a hidden position, to have a valid layout. Based on Maciej Mrozowski's patch this is now done by postponing the showing of the tooltip until the meta-data and the preview has been received.

CCBUG: 245491


 M  +15 -18    tooltips/filemetadatatooltip.cpp  
 M  +13 -1     tooltips/filemetadatatooltip.h  
 M  +119 -104  tooltips/tooltipmanager.cpp  
 M  +12 -27    tooltips/tooltipmanager.h  
 M  +1 -1      viewextensionsfactory.cpp  


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

Thanks, working well!

Also a note for those willing to try it out in 4.5 (fogetting about BC), corresponding kdelibs commit is:

http://websvn.kde.org/trunk/KDE/kdelibs/kio/kfile/kfilemetadatawidget.cpp?r1=1152656&r2=1158055

I'll backport this to 4.5 next Monday (will be released in 4.5.1 then). I've submitted the related patch at http://reviewboard.kde.org/r/4777/, to get an official "go". David Faure has not answered yet, but the go from Michael Pyne is OK too.

SVN commit 1160677 by ppenz:

Backport of SVN commit 1158055: Emit a signal, if the meta-data request has been finished. This e. g. allows tooltips to postpone the opening of the tooltip, until all data could be received.
CCBUG: 245491


 M  +2 -1      kfilemetadatawidget.cpp  
 M  +9 -0      kfilemetadatawidget.h  


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

SVN commit 1160679 by ppenz:

Backport of SVN commit 1158057: Remove the workaround to show the tooltip temporary on a hidden position, to have a valid layout. Based on Maciej Mrozowski's patch this is now done by postponing the showing of the tooltip until the meta-data and the preview has been received.

BUG: 245491
FIXED-IN: 4.5.1



 M  +15 -18    tooltips/filemetadatatooltip.cpp  
 M  +15 -3     tooltips/filemetadatatooltip.h  
 M  +122 -107  tooltips/tooltipmanager.cpp  
 M  +12 -27    tooltips/tooltipmanager.h  
 M  +1 -1      viewextensionsfactory.cpp  


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