78962 2012-02-18 05:13:41 -0800 [Qt][WK2] Draw tiles of previous contents-scale for non-opaque layers if they don't intersect with tiles in new content-scale 2012-02-21 15:33:40 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 noam noam hausmann jturcotte kenneth webkit.review.bot oldest_to_newest 559770 0 noam 2012-02-18 05:13:41 -0800 [Qt][WK2] Draw tiles of previous contents-scale for opaque layers if they don't intersect with previous tiles 560294 1 127834 noam 2012-02-20 11:09:20 -0800 Created attachment 127834 Patch 560330 2 127834 kenneth 2012-02-20 13:13:41 -0800 Comment on attachment 127834 Patch I don't get it fully. Maybe previous tile is a too overloaded word. 560397 3 noam 2012-02-20 15:10:58 -0800 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. 560772 4 kenneth 2012-02-21 02:55:37 -0800 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? 560914 5 127834 kenneth 2012-02-21 08:45:17 -0800 Comment on attachment 127834 Patch Would be nice with a few better comments in the code, but r=me 560925 6 127834 jturcotte 2012-02-21 08:52:55 -0800 Comment on attachment 127834 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? 560931 7 noam 2012-02-21 08:57:49 -0800 (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. 560935 8 noam 2012-02-21 09:00:01 -0800 (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. 560940 9 jturcotte 2012-02-21 09:12:24 -0800 (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. 560960 10 noam 2012-02-21 09:35:13 -0800 (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? 560971 11 jturcotte 2012-02-21 09:50:46 -0800 (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 :) 561017 12 127996 noam 2012-02-21 10:46:13 -0800 Created attachment 127996 Patch 561287 13 127996 webkit.review.bot 2012-02-21 15:33:35 -0800 Comment on attachment 127996 Patch Clearing flags on attachment: 127996 Committed r108408: <http://trac.webkit.org/changeset/108408> 561288 14 webkit.review.bot 2012-02-21 15:33:40 -0800 All reviewed patches have been landed. Closing bug. 127834 2012-02-20 11:09:20 -0800 2012-02-21 10:46:13 -0800 Patch bug-78962-20120220110918.patch text/plain 2384 noam U3VidmVyc2lvbiBSZXZpc2lvbjogMTA4MjA0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMjM5MTE2ZjkyODIwMGY5 OGQ5ZDdiZThkZjI2NDRiMjdhN2M5OWFjOC4uMWVhOGM3ZjYwZWYwYTg4YWVlNmMwZjBiYzQ5Nzky OTQxNmNjYWY5NCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEyLTAyLTIwICBObydh bSBSb3NlbnRoYWwgIDxub2FtLnJvc2VudGhhbEBub2tpYS5jb20+CisKKyAgICAgICAgW1F0XVtX SzJdIERyYXcgdGlsZXMgb2YgcHJldmlvdXMgY29udGVudHMtc2NhbGUgZm9yIG9wYXF1ZSBsYXll cnMgaWYgdGhleSBkb24ndCBpbnRlcnNlY3Qgd2l0aCBwcmV2aW91cyB0aWxlcworICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Nzg5NjIKKworICAgICAgICBP bmx5IGF2b2lkIHBhaW50aW5nIG9sZC1zY2FsZSB0aWxlcyBpbiBzZW1pLXRyYW5zcGFyZW50IHNp dHV0YXRpb25zIGlmIHRoZSBvbGQKKyAgICAgICAgdGlsZXMgaW50ZXJzZWN0IHdpdGggZXhpc3Rp bmcgdGlsZXMuCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgKiBVSVByb2Nlc3MvcXQvTGF5ZXJCYWNraW5nU3RvcmUuY3BwOgorICAgICAgICAoV2ViS2l0 OjpMYXllckJhY2tpbmdTdG9yZTo6cGFpbnRUb1RleHR1cmVNYXBwZXIpOgorCiAyMDEyLTAyLTE4 ICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBsZS5jb20+CiAKICAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzEwODkxODAxPiBCYWNraW5nU3RvcmU6OnNjcm9sbCgpIHVubmVjZXNzYXJpbHkgY29waWVz IHBpeGVscyBhcm91bmQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9xdC9M YXllckJhY2tpbmdTdG9yZS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvcXQvTGF5ZXJC YWNraW5nU3RvcmUuY3BwCmluZGV4IGNkMDFhYzY2ODc2ODFiNjQxNmE5ZDA0OWY3YTM1ZmZhYjU1 NTNhYTMuLmUxMzI0M2ExOTY1OTU3MDkwMGQ5N2M0YWYzZTZiOTMyNWUwMGEyYzMgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9xdC9MYXllckJhY2tpbmdTdG9yZS5jcHAKKysr IGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3F0L0xheWVyQmFja2luZ1N0b3JlLmNwcApAQCAt MTAwLDYgKzEwMCw3IEBAIHZvaWQgTGF5ZXJCYWNraW5nU3RvcmU6OnBhaW50VG9UZXh0dXJlTWFw cGVyKFRleHR1cmVNYXBwZXIqIHRleHR1cmVNYXBwZXIsIGNvbnN0CiAKICAgICAvLyBXZSBoYXZl IHRvIGRvIHRoaXMgZXZlcnkgdGltZSB3ZSBwYWludCwgaW4gY2FzZSB0aGUgb3BhY2l0eSBoYXMg Y2hhbmdlZC4KICAgICBIYXNoTWFwPGludCwgTGF5ZXJCYWNraW5nU3RvcmVUaWxlPjo6aXRlcmF0 b3IgZW5kID0gbV90aWxlcy5lbmQoKTsKKyAgICBGbG9hdFJlY3QgY292ZXJlZFJlY3Q7CiAgICAg Zm9yIChIYXNoTWFwPGludCwgTGF5ZXJCYWNraW5nU3RvcmVUaWxlPjo6aXRlcmF0b3IgaXQgPSBt X3RpbGVzLmJlZ2luKCk7IGl0ICE9IGVuZDsgKytpdCkgewogICAgICAgICBMYXllckJhY2tpbmdT dG9yZVRpbGUmIHRpbGUgPSBpdC0+c2Vjb25kOwogICAgICAgICBpZiAoIXRpbGUudGV4dHVyZSgp KQpAQCAtMTA3LDEyICsxMDgsMTYgQEAgdm9pZCBMYXllckJhY2tpbmdTdG9yZTo6cGFpbnRUb1Rl eHR1cmVNYXBwZXIoVGV4dHVyZU1hcHBlciogdGV4dHVyZU1hcHBlciwgY29uc3QKIAogICAgICAg ICBpZiAodGlsZS5zY2FsZSgpID09IG1fc2NhbGUpIHsKICAgICAgICAgICAgIHRpbGVzVG9QYWlu dC5hcHBlbmQoJnRpbGUpOworICAgICAgICAgICAgY292ZXJlZFJlY3QudW5pdGUodGlsZS5yZWN0 KCkpOwogICAgICAgICAgICAgY29udGludWU7CiAgICAgICAgIH0KIAogICAgICAgICAvLyBPbmx5 IHNob3cgdGhlIHByZXZpb3VzIHRpbGUgaWYgdGhlIG9wYWNpdHkgaXMgaGlnaCwgb3RoZXJ3aXNl IGVmZmVjdCBsb29rcyBsaWtlIGEgYnVnLgotICAgICAgICBpZiAob3BhY2l0eSA+IDAuOTUpCi0g ICAgICAgICAgICB0aWxlc1RvUGFpbnQucHJlcGVuZCgmdGlsZSk7CisgICAgICAgIGlmIChvcGFj aXR5IDwgMC45NSAmJiBjb3ZlcmVkUmVjdC5pbnRlcnNlY3RzKHRpbGUucmVjdCgpKSkKKyAgICAg ICAgICAgIGNvbnRpbnVlOworCisgICAgICAgIHRpbGVzVG9QYWludC5wcmVwZW5kKCZ0aWxlKTsK KyAgICAgICAgY292ZXJlZFJlY3QudW5pdGUodGlsZS5yZWN0KCkpOwogICAgIH0KIAogICAgIGZv ciAoc2l6ZV90IGkgPSAwOyBpIDwgdGlsZXNUb1BhaW50LnNpemUoKTsgKytpKQo= 127996 2012-02-21 10:46:13 -0800 2012-02-21 15:33:35 -0800 Patch patch.diff text/plain 2326 noam ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAxYmEyZTllLi4zYzQ3MjFkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTItMDItMjAgIE5vJ2FtIFJvc2VudGhhbCAgPG5vYW0ucm9zZW50aGFsQG5va2lhLmNv bT4KKworICAgICAgICBbUXRdW1dLMl0gRHJhdyB0aWxlcyBvZiBwcmV2aW91cyBjb250ZW50cy1z Y2FsZSBmb3Igb3BhcXVlIGxheWVycyBpZiB0aGV5IGRvbid0IGludGVyc2VjdCB3aXRoIHByZXZp b3VzIHRpbGVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD03ODk2MgorCisgICAgICAgIE9ubHkgYXZvaWQgcGFpbnRpbmcgb2xkLXNjYWxlIHRpbGVzIGlu IHNlbWktdHJhbnNwYXJlbnQgc2l0dXRhdGlvbnMgaWYgdGhlIG9sZAorICAgICAgICB0aWxlcyBp bnRlcnNlY3Qgd2l0aCBleGlzdGluZyB0aWxlcy4KKworICAgICAgICBSZXZpZXdlZCBieSBLZW5u ZXRoIFJvaGRlIENocmlzdGlhbnNlbi4KKworICAgICAgICAqIFVJUHJvY2Vzcy9xdC9MYXllckJh Y2tpbmdTdG9yZS5jcHA6CisgICAgICAgIChXZWJLaXQ6OkxheWVyQmFja2luZ1N0b3JlOjpwYWlu dFRvVGV4dHVyZU1hcHBlcik6CisKIDIwMTItMDItMjAgIE1hcnRpbiBSb2JpbnNvbiAgPG1yb2Jp bnNvbkBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtVTklYXSBQbHVnaW4gaW5mb3JtYXRpb24gZmll bGRzIGFyZSBub3QgaW50ZXJwcmV0ZWQgYXMgVVRGLTgKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQyL1VJUHJvY2Vzcy9xdC9MYXllckJhY2tpbmdTdG9yZS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9V SVByb2Nlc3MvcXQvTGF5ZXJCYWNraW5nU3RvcmUuY3BwCmluZGV4IGNkMDFhYzYuLjhmNTgzMDMg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9xdC9MYXllckJhY2tpbmdTdG9y ZS5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3F0L0xheWVyQmFja2luZ1N0b3Jl LmNwcApAQCAtMTAwLDYgKzEwMCw3IEBAIHZvaWQgTGF5ZXJCYWNraW5nU3RvcmU6OnBhaW50VG9U ZXh0dXJlTWFwcGVyKFRleHR1cmVNYXBwZXIqIHRleHR1cmVNYXBwZXIsIGNvbnN0CiAKICAgICAv LyBXZSBoYXZlIHRvIGRvIHRoaXMgZXZlcnkgdGltZSB3ZSBwYWludCwgaW4gY2FzZSB0aGUgb3Bh Y2l0eSBoYXMgY2hhbmdlZC4KICAgICBIYXNoTWFwPGludCwgTGF5ZXJCYWNraW5nU3RvcmVUaWxl Pjo6aXRlcmF0b3IgZW5kID0gbV90aWxlcy5lbmQoKTsKKyAgICBGbG9hdFJlY3QgY292ZXJlZFJl Y3Q7CiAgICAgZm9yIChIYXNoTWFwPGludCwgTGF5ZXJCYWNraW5nU3RvcmVUaWxlPjo6aXRlcmF0 b3IgaXQgPSBtX3RpbGVzLmJlZ2luKCk7IGl0ICE9IGVuZDsgKytpdCkgewogICAgICAgICBMYXll ckJhY2tpbmdTdG9yZVRpbGUmIHRpbGUgPSBpdC0+c2Vjb25kOwogICAgICAgICBpZiAoIXRpbGUu dGV4dHVyZSgpKQpAQCAtMTA3LDEyICsxMDgsMTcgQEAgdm9pZCBMYXllckJhY2tpbmdTdG9yZTo6 cGFpbnRUb1RleHR1cmVNYXBwZXIoVGV4dHVyZU1hcHBlciogdGV4dHVyZU1hcHBlciwgY29uc3QK IAogICAgICAgICBpZiAodGlsZS5zY2FsZSgpID09IG1fc2NhbGUpIHsKICAgICAgICAgICAgIHRp bGVzVG9QYWludC5hcHBlbmQoJnRpbGUpOworICAgICAgICAgICAgY292ZXJlZFJlY3QudW5pdGUo dGlsZS5yZWN0KCkpOwogICAgICAgICAgICAgY29udGludWU7CiAgICAgICAgIH0KIAogICAgICAg ICAvLyBPbmx5IHNob3cgdGhlIHByZXZpb3VzIHRpbGUgaWYgdGhlIG9wYWNpdHkgaXMgaGlnaCwg b3RoZXJ3aXNlIGVmZmVjdCBsb29rcyBsaWtlIGEgYnVnLgotICAgICAgICBpZiAob3BhY2l0eSA+ IDAuOTUpCi0gICAgICAgICAgICB0aWxlc1RvUGFpbnQucHJlcGVuZCgmdGlsZSk7CisgICAgICAg IC8vIFdlIHNob3cgdGhlIHByZXZpb3VzLXNjYWxlIHRpbGUgYW55d2F5IGlmIGl0IGRvZXNuJ3Qg aW50ZXJzZWN0IHdpdGggYW55IGN1cnJlbnQtc2NhbGUgdGlsZS4KKyAgICAgICAgaWYgKG9wYWNp dHkgPCAwLjk1ICYmIGNvdmVyZWRSZWN0LmludGVyc2VjdHModGlsZS5yZWN0KCkpKQorICAgICAg ICAgICAgY29udGludWU7CisKKyAgICAgICAgdGlsZXNUb1BhaW50LnByZXBlbmQoJnRpbGUpOwor ICAgICAgICBjb3ZlcmVkUmVjdC51bml0ZSh0aWxlLnJlY3QoKSk7CiAgICAgfQogCiAgICAgZm9y IChzaXplX3QgaSA9IDA7IGkgPCB0aWxlc1RvUGFpbnQuc2l6ZSgpOyArK2kpCg==