Summary: | [Qt] painting of windowed plugins faulty on certain scroll events | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||
Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, girish, hausmann, hyatt, kenneth, vestbo | ||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Robert Hogan
2011-01-19 13:00:03 PST
This is happening in: void RenderWidget::paint(PaintInfo& paintInfo, int tx, int ty) { if (!shouldPaint(paintInfo, tx, ty)) return; tx += x(); ty += y(); if (hasBoxDecorations() && (paintInfo.phase == PaintPhaseForeground || paintInfo.phase == PaintPhaseSelection)) paintBoxDecorations(paintInfo, tx, ty); if (paintInfo.phase == PaintPhaseMask) { paintMask(paintInfo, tx, ty); return; } if (!m_frameView || paintInfo.phase != PaintPhaseForeground || style()->visibility() != VISIBLE) return; Commenting out both: if (!shouldPaint(paintInfo, tx, ty)) return; and if (!m_frameView || paintInfo.phase != PaintPhaseForeground || style()->visibility() != VISIBLE) return; fixes it. Commenting out one or the other does not. It's very hard to debug paint events so I don't really stand a chance without a few pointers, but this 'fixes' it: --- a/Source/WebCore/rendering/RenderReplaced.cpp +++ b/Source/WebCore/rendering/RenderReplaced.cpp @@ -157,18 +157,17 @@ void RenderReplaced::paint(PaintInfo& paintInfo, int tx, int ty) bool RenderReplaced::shouldPaint(PaintInfo& paintInfo, int& tx, int& ty) { if (paintInfo.phase != PaintPhaseForeground && paintInfo.phase != PaintPhaseOutline && paintInfo.phase != PaintPhaseSelfOutline && paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPhaseMask) return false; if (!paintInfo.shouldPaintWithinRoot(this)) return false; // if we're invisible or haven't received a layout yet, then just bail. if (style()->visibility() != VISIBLE) return false; - int currentTX = tx + x(); int currentTY = ty + y(); // Early exit if the element touches the edges. @@ -180,10 +179,9 @@ bool RenderReplaced::shouldPaint(PaintInfo& paintInfo, int& tx, int& ty) top = min(selTop, top); bottom = max(selBottom, bottom); } int os = 2 * maximalOutlineSize(paintInfo.phase); - if (currentTX + leftVisualOverflow() >= paintInfo.rect.right() + os || currentTX + rightVisualOverflow() <= paintInfo.rect.x() - os) - return false; if (top >= paintInfo.rect.bottom() + os || bottom <= paintInfo.rect.y() - os) return false; diff --git a/Source/WebCore/rendering/RenderWidget.cpp b/Source/WebCore/rendering/RenderWidget.cpp index 13b572d..8f07d40 100644 --- a/Source/WebCore/rendering/RenderWidget.cpp +++ b/Source/WebCore/rendering/RenderWidget.cpp @@ -260,7 +260,7 @@ void RenderWidget::paint(PaintInfo& paintInfo, int tx, int ty) return; } - if (!m_frameView || paintInfo.phase != PaintPhaseForeground || style()->visibility() != VISIBLE) + if (!m_frameView /*|| paintInfo.phase != PaintPhaseForeground */|| style()->visibility() != VISIBLE) return; I'm not even sure what: - if (currentTX + leftVisualOverflow() >= paintInfo.rect.right() + os || currentTX + rightVisualOverflow() <= paintInfo.rect.x() - os) - return false; is doing. Just a guess but make sure the dirty rect you pass in is correct. There's nothing wrong with the cross-platform code there. (In reply to comment #2) > Just a guess but make sure the dirty rect you pass in is correct. There's nothing wrong with the cross-platform code there. Right enough, another 'fix' is: --- a/Source/WebKit/qt/Api/qwebframe.cpp +++ b/Source/WebKit/qt/Api/qwebframe.cpp @@ -374,6 +374,7 @@ void QWebFramePrivate::renderRelativeCoords(GraphicsContext* context, QWebFrame: QRect rect = intersectedRect; context->translate(x, y); rect.translate(-x, -y); + view->paintContents(context, rect); context->translate(-scrollX, -scrollY); rect.translate(scrollX, scrollY); context->clip(view->visibleContentRect()); It looks like RenderWidget::paint() needs to test the rect using its absolute co-ordinates in the frame but QWebFrame only supplies relative co-ordinates. I imagine this is pretty unique to plugins. And Qt is the only port calling frameview->paintContents() directly when rendering the page. So I think Qt needs to test the page for plugins/widgets and make sure they always get painted. There is something I don't understand about RenderWidget::paint(). If a Scroll event has moved the ScrollView down by exactly or just slightly more than the height of the RenderWidget shouldPaint() in RenderWidget::paint() will never pass. In the case of this bug the result is that the windowed plugin does not move when a single scroll event moves the viewport down by the same amount as (or slightly more than) the height of the plugin. So doesn't ScrollView need to take account of this somehow? Created attachment 81403 [details]
Patch
(In reply to comment #5) > Created an attachment (id=81403) [details] > Patch I've look at PluginViewSymbian and PluginViewWin - neither have the code I'm altering here. I don't have the set-up for testing either of them unfortunately. Comment on attachment 81403 [details]
Patch
r=me, thanks for digging into this!
Comment on attachment 81403 [details]
Patch
Sounds good to me. I agree with you conclusion investigating myself. It's really complex to reproduce, not sure we can do it through a tests. I ran the LayoutTests on my machine and didn't catch any regressions.
Comment on attachment 81403 [details] Patch Clearing flags on attachment: 81403 Committed r79397: <http://trac.webkit.org/changeset/79397> All reviewed patches have been landed. Closing bug. |