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

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Noam Rosenthal]

[Qt][WK2] Draw tiles of previous contents-scale for opaque layers if they don't intersect with previous tiles

[Noam Rosenthal]

Created attachment 127834 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 127834 [details]
Patch

I don't get it fully. Maybe previous tile is a too overloaded word.

[Noam Rosenthal]

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.

[Kenneth Rohde Christiansen]

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?

[Kenneth Rohde Christiansen]

Comment on attachment 127834 [details]
Patch

Would be nice with a few better comments in the code, but r=me

[Jocelyn Turcotte]

Comment on attachment 127834 [details]
Patch

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?

[Noam Rosenthal]

(In reply to comment #6)
> (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?

hmm... I guess that line is unnecessary.

[Noam Rosenthal]

(In reply to comment #7)
> (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.

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.

[Jocelyn Turcotte]

(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.

[Noam Rosenthal]

(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?

[Jocelyn Turcotte]

(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 :)

[Noam Rosenthal]

Created attachment 127996 [details]
Patch

[WebKit Review Bot]

Comment on attachment 127996 [details]
Patch

Clearing flags on attachment: 127996

Committed r108408: <http://trac.webkit.org/changeset/108408>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.