Bug 324560 - Shadows are not fully drawn for tooltips which changed size
Summary: Shadows are not fully drawn for tooltips which changed size
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Unclassified
Component: compositing (show other bugs)
Version: 4.11.0
Platform: unspecified Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-05 18:04 UTC by Ruslan Kabatsayev
Modified: 2013-09-23 23:12 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 4.11.2


Attachments
qdbus output as requested (4.31 KB, text/x-log)
2013-09-05 18:55 UTC, Ruslan Kabatsayev
Details
actual fix (1.33 KB, patch)
2013-09-11 21:29 UTC, Thomas Lübking
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ruslan Kabatsayev 2013-09-05 18:04:04 UTC
See screenshot: http://i0.simplest-image-hosting.net/picture/screenshot-09052013-082452-pm.png

Reproducible: Always

Steps to Reproduce:
1. Open this page in Chromium: http://jurjenbokma.com/ApprenticesNotes/turing_off_overcommit.html
2. Move cursor to third code rectangle (with "cat <<EOF...")
3. After a "Step 3" tooltip appears, move it to light background
4. See the result as in screenshot
Actual Results:  
Shadow is not fully drawn to the right of the shadow of original-sized tooltip

Expected Results:  
The shadow should update to reflect new tooltip size
Comment 1 Thomas Lübking 2013-09-05 18:43:18 UTC
please post the output of
   qdbus org.kde.kwin /KWin supportInformation

GUI style is oxygen?
Comment 2 Ruslan Kabatsayev 2013-09-05 18:55:29 UTC
Created attachment 82180 [details]
qdbus output as requested

Yes, style is oxygen and gtk style used by Chromium is oxygen-gtk2.
Comment 3 Thomas Lübking 2013-09-05 19:04:24 UTC
(In reply to comment #2)
> Created attachment 82180 [details]
> qdbus output as requested

> Qt Graphics System: native
Does it also happen with the raster graphicssystem (which should be faster in that setup anyway)

> glPreferBufferSwap: 0
Does it also happen with "tearing prevention" enabled?

> Yes, style is oxygen and gtk style used by Chromium is oxygen-gtk2.
I take this does /only/ occur with chromium?
Comment 4 Ruslan Kabatsayev 2013-09-05 19:11:15 UTC
(In reply to comment #3)
> Does it also happen with the raster graphicssystem (which should be faster
> in that setup anyway)
Yes

> > glPreferBufferSwap: 0
> Does it also happen with "tearing prevention" enabled?
Doesn't happen with "Full scene repaints", does happen with anything else

> > Yes, style is oxygen and gtk style used by Chromium is oxygen-gtk2.
> I take this does /only/ occur with chromium?
I remember experiencing similar things with dolphin and some other apps (and this was since about 4.7 or earlier), but they were hard to reproduce. Chromium appeared to give a 100% reproducible case, so now I actually made a report.
Comment 5 Martin Flöser 2013-09-11 14:21:54 UTC
> > > glPreferBufferSwap: 0
> > 
> > Does it also happen with "tearing prevention" enabled?
> 
> Doesn't happen with "Full scene repaints", does happen with anything else
hmm sounds like an update is missing or using the wrong region. Assuming it's 
an Unmanged:
addWorkspaceRepaint(visibleRect());  // damage old area
QRect old = geom;
geom = newgeom;
addRepaintFull();

visibleRect() should include the shadow unless it's totally broken.

And Toplevel::addRepaintFull()

void Toplevel::addRepaintFull()
{
    repaints_region = visibleRect().translated(-pos());
    emit needsRepaint();
}

So this looks right to me, too.
Comment 6 Thomas Lübking 2013-09-11 16:37:18 UTC
There's probably more going on than just a configure notification, probably some extra shape or shadow re-setting.

It's however reproducible here, i'll print me some debug out.
Comment 7 Thomas Lübking 2013-09-11 19:13:27 UTC
Major issue:

diff --git a/kwin/composite.cpp b/kwin/composite.cpp
index 8489373..d70597c 100644
--- a/kwin/composite.cpp
+++ b/kwin/composite.cpp
@@ -1011,7 +1011,7 @@ void Toplevel::addDamageFull()
         return;
 
     damage_region = rect();
-    repaints_region = rect();
+    repaints_region |= rect();
 
     emit damaged(this, rect());
 }

There's a follow-up damage event (of the window) which replaces the visibleRect() repaint.

But this still cuts off the right side of the shadow (probably for a completey different reason, though)
Comment 8 Thomas Lübking 2013-09-11 21:29:19 UTC
Created attachment 82287 [details]
actual fix

the other issue is that the shadow region is not updated before calling addRepaintFull (so the shadow area is too small and doesn't extend the bounding rect on the right)
Comment 9 Martin Flöser 2013-09-12 14:13:57 UTC
ShipIt(tm)
Comment 10 Thomas Lübking 2013-09-23 23:12:11 UTC
Git commit db3d4eed1cd5e5da2b737b254895191af951bab8 by Thomas Lübking.
Committed on 11/09/2013 at 21:27.
Pushed by luebking into branch 'KDE/4.11'.

fix two damage artifact causes

1. when adding a full damange, that must not replace existing (larger) repaints
2. emit geometryChanged before invoking and to update shadowGeometry through addRepaintFull
FIXED-IN: 4.11.2

M  +1    -1    kwin/composite.cpp
M  +1    -0    kwin/events.cpp

http://commits.kde.org/kde-workspace/db3d4eed1cd5e5da2b737b254895191af951bab8