Bug 78962 - [Qt][WK2] Draw tiles of previous contents-scale for non-opaque layers if they don't intersect with tiles in new content-scale
: [Qt][WK2] Draw tiles of previous contents-scale for non-opaque layers if they...
Status: RESOLVED FIXED
: WebKit
Layout and Rendering
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2012-02-18 05:13 PST by
Modified: 2012-02-21 15:33 PST (History)


Attachments
Patch (2.33 KB, patch)
2012-02-20 11:09 PST, Noam Rosenthal
kenneth: review+
Review Patch | Details | Formatted Diff | Diff
Patch (2.27 KB, patch)
2012-02-21 10:46 PST, Noam Rosenthal
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-02-18 05:13:41 PST
[Qt][WK2] Draw tiles of previous contents-scale for opaque layers if they don't intersect with previous tiles
------- Comment #1 From 2012-02-20 11:09:20 PST -------
Created an attachment (id=127834) [details]
Patch
------- Comment #2 From 2012-02-20 13:13:41 PST -------
(From update of attachment 127834 [details])
I don't get it fully. Maybe previous tile is a too overloaded word.
------- Comment #3 From 2012-02-20 15:10:58 PST -------
To reproduce, open the morphing-cubes demo in Qt's MiniBrowser, and change the content scale.
The cube sides always disappear and then reappear - that's because they're painted with semi-transparency and the tiles from the previous contents scale are discarded before the new ones are ready.
------- Comment #4 From 2012-02-21 02:55:37 PST -------
Yeah I understand the issue, but just not disregarding ones not intersecting sounds like it only solves this in some cases. What do you do when they infact do intersect and you still don't have the new content ready? clip it?
------- Comment #5 From 2012-02-21 08:45:17 PST -------
(From update of attachment 127834 [details])
Would be nice with a few better comments in the code, but r=me
------- Comment #6 From 2012-02-21 08:52:55 PST -------
(From update of attachment 127834 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=127834&action=review

> Source/WebKit2/UIProcess/qt/LayerBackingStore.cpp:120
> +        coveredRect.unite(tile.rect());

Why do you need to count the previous scale tiles in the covered rect?
------- Comment #7 From 2012-02-21 08:57:49 PST -------
(In reply to comment #6)
> (From update of attachment 127834 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127834&action=review
> 
> > Source/WebKit2/UIProcess/qt/LayerBackingStore.cpp:120
> > +        coveredRect.unite(tile.rect());
> 
> Why do you need to count the previous scale tiles in the covered rect?

hmm... I guess that line is unnecessary.
------- Comment #8 From 2012-02-21 09:00:01 PST -------
(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 127834 [details] [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=127834&action=review
> > 
> > > Source/WebKit2/UIProcess/qt/LayerBackingStore.cpp:120
> > > +        coveredRect.unite(tile.rect());
> > 
> > Why do you need to count the previous scale tiles in the covered rect?
> 
> hmm... I guess that line is unnecessary.

Though if things are fast enough there might be a situation where we have previous tiles from several scales... in that case we would still want to discard them if there's opacity.
------- Comment #9 From 2012-02-21 09:12:24 PST -------
(In reply to comment #8)
> Though if things are fast enough there might be a situation where we have previous tiles from several scales... in that case we would still want to discard them if there's opacity.

I think that would be weird, since we should only send createTile while rendering tiles and normally at this point the previous backing store should already have sent all the removeTile messages for its tiles, once its smart pointer was reset.

It would also mean that you would discard tiles randomly from one scale or the other depending on which one sets the coveredRect first, and mix them. So I think we should avoid it if it really can happen.
------- Comment #10 From 2012-02-21 09:35:13 PST -------
(In reply to comment #9)
> (In reply to comment #8)
> > Though if things are fast enough there might be a situation where we have previous tiles from several scales... in that case we would still want to discard them if there's opacity.
> 
> I think that would be weird, since we should only send createTile while rendering tiles and normally at this point the previous backing store should already have sent all the removeTile messages for its tiles, once its smart pointer was reset.
> 
> It would also mean that you would discard tiles randomly from one scale or the other depending on which one sets the coveredRect first, and mix them. So I think we should avoid it if it really can happen.

OK, I see what you mean. But that line is harmless, and I use it in the next patch to make sure we clip if we painted tiles that are not contained in the targetRect.
Otherwise I need to calculate two coverRects, which seems like an overkill.

Any objection to submitting the patch as is?
------- Comment #11 From 2012-02-21 09:50:46 PST -------
(In reply to comment #10)
> Any objection to submitting the patch as is?

Sorry no problem, I just wanted to make sure that it has more use than it's costing :)
------- Comment #12 From 2012-02-21 10:46:13 PST -------
Created an attachment (id=127996) [details]
Patch
------- Comment #13 From 2012-02-21 15:33:35 PST -------
(From update of attachment 127996 [details])
Clearing flags on attachment: 127996

Committed r108408: <http://trac.webkit.org/changeset/108408>
------- Comment #14 From 2012-02-21 15:33:40 PST -------
All reviewed patches have been landed.  Closing bug.