80199 2012-03-02 15:40:08 -0800 [chromium] Make compositeAndReadback and damage tracking play nicely together 2012-03-06 15:09:58 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 76668 1 shawnsingh shawnsingh backer cc-bugs jamesr nduca webkit.review.bot oldest_to_newest 569998 0 shawnsingh 2012-03-02 15:40:08 -0800 Issue reported in: http://code.google.com/p/chromium/issues/detail?id=116055 compositeAndReadback causes the composited rendering path to proceed, including damage tracking, but in single-threaded mode, compositeAndReadback does not actually affect the front buffer (which is the purpose of damage tracking). As a result, damage tracking gets out-of-sync with what really changed on the front buffer, and some repaints do not actually show up on the screen when partial swap is enabled. Jonathan, when you have a chance, can you please try to reproduce with --enable-threaded-compositing? (remember, HUD and rect visualization won't show up in multi threaded mode yet) The error should be un-reproducible, because as I understand, swapBuffers is actually performed on compositeAndReadback in multi-threaded mode. In my own linux osmesa attempts, I could not reproduce the problem in multi-threaded mode, and I could not reproduce the problem when I hacked a swapBuffers in single-threaded mode. There are 3 options I see to fix this problem. Nat can you please suggest which fix you think is best? I think option #3 is correct, and the other two are hacky. Thanks in advance. 1. do a swapBuffers on compositeAndReadback for single threaded mode. 2. bypass damage tracking in composite-and-readback single threaded mode. 3. (seems like the right thing to do...) change damage tracking to *accumulate* dirtiness until swapbuffers is actually performed. Eventually, when we can support per-surface scissoring, we can reset the dirty-accumulation after each surface is updated. Let me know then I will proceed ... thanks! 572125 1 130436 shawnsingh 2012-03-06 13:51:10 -0800 Created attachment 130436 Patch The fix is trivial, but testing it is not so obvious. Testing this fix requires observing front-buffer changes. As I understand, this could be done via chromium browser gpu pixel tests, but it won't happen in layout tests or webkit_unit_tests. Is it worth going through that trouble? ... also, I think context lost issues are already addressed by the existing code in compositeAndReadback (exits early if doComposite returns false). 572126 2 shawnsingh 2012-03-06 13:55:32 -0800 For clarity - Nat and I had discussed offline, and for now we're considering option #1. Thanks 572129 3 130436 nduca 2012-03-06 13:57:57 -0800 Comment on attachment 130436 Patch LGTM but copy the check for context lost at :85 up to :82 before calling swap. That has an assertion inside it that will pop if you call it with a lost context. 572146 4 130442 shawnsingh 2012-03-06 14:14:15 -0800 Created attachment 130442 contextLost() check placed before swap 572166 5 130442 jamesr 2012-03-06 14:43:16 -0800 Comment on attachment 130442 contextLost() check placed before swap Seems good, R=me 572172 6 130442 shawnsingh 2012-03-06 14:50:42 -0800 Comment on attachment 130442 contextLost() check placed before swap Cool, thanks for reviewing. 572192 7 130442 webkit.review.bot 2012-03-06 15:09:54 -0800 Comment on attachment 130442 contextLost() check placed before swap Clearing flags on attachment: 130442 Committed r109965: <http://trac.webkit.org/changeset/109965> 572193 8 webkit.review.bot 2012-03-06 15:09:58 -0800 All reviewed patches have been landed. Closing bug. 130436 2012-03-06 13:51:10 -0800 2012-03-06 14:14:12 -0800 Patch bug-80199-20120306135109.patch text/plain 1655 shawnsingh U3VidmVyc2lvbiBSZXZpc2lvbjogMTA5OTM0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjU3MDg4NTA1OThhZjc1 NTVkYjZmMjY5N2UxNWVjMmNhMTMxMDU4Yi4uNmVkOWJiYWUxZjQ0Y2Q0N2YzMGMyZGYwNWUxYzA3 ZmQwYmZiYTI5MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTAzLTA2ICBTaGF3 biBTaW5naCAgPHNoYXduc2luZ2hAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtjaHJvbWl1bV0g TWFrZSBjb21wb3NpdGVBbmRSZWFkYmFjayBhbmQgZGFtYWdlIHRyYWNraW5nIHBsYXkgbmljZWx5 IHRvZ2V0aGVyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD04MDE5OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFJlcXVpcmVzIHN5c3RlbS1sZXZlbCB0ZXN0aW5nIChwaXhlbHMgb24gZnJvbnQtYnVmZmVyIHNo b3VsZCBiZQorICAgICAgICBvYnNlcnZlZCBmb3IgY29ycmVjdCBiZWhhdmlvcikgdGhhdCBjYW5u b3QgYmUgZG9uZSBieSBsYXlvdXQgdGVzdHMKKyAgICAgICAgb3IgdW5pdCB0ZXN0cywgaGF2ZSB0 byByZWx5IG9uIG1hbnVhbCB0ZXN0aW5nLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mv Y2hyb21pdW0vY2MvQ0NTaW5nbGVUaHJlYWRQcm94eS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpD Q1NpbmdsZVRocmVhZFByb3h5Ojpjb21wb3NpdGVBbmRSZWFkYmFjayk6CisKIDIwMTItMDMtMDYg IERhdmlkIENvcnZveXNpZXIgIDxkYXZpZC5jb3J2b3lzaWVyQG9yYW5nZS5jb20+CiAKICAgICAg ICAgVW5yZWxlYXNlZCBnc3Rfb2JqZWN0X3JlZmVyZW5jZSB0byBhdWRpbyBzaW5rIGluIE1lZGlh UGxheWVyUHJpdmF0ZUdTdHJlYW1lcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3MvY2hyb21pdW0vY2MvQ0NTaW5nbGVUaHJlYWRQcm94eS5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ1NpbmdsZVRocmVhZFByb3h5 LmNwcAppbmRleCAwYWYwN2NiZGJmZDI5NTc4MzhmZWMxNDRiMjZhNmRhOWIzZGZjOGRmLi4wOGIz N2QyZTE0MTY2NzliYTRhMWQxZWI5ODY3MmNlM2E4NWRiZTMwIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ1NpbmdsZVRocmVhZFByb3h5 LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9D Q1NpbmdsZVRocmVhZFByb3h5LmNwcApAQCAtODAsNiArODAsNyBAQCBib29sIENDU2luZ2xlVGhy ZWFkUHJveHk6OmNvbXBvc2l0ZUFuZFJlYWRiYWNrKHZvaWQgKnBpeGVscywgY29uc3QgSW50UmVj dCYgcmVjdAogCiAgICAgbV9sYXllclRyZWVIb3N0SW1wbC0+cmVhZGJhY2socGl4ZWxzLCByZWN0 KTsKIAorICAgIG1fbGF5ZXJUcmVlSG9zdEltcGwtPnN3YXBCdWZmZXJzKCk7CiAgICAgZGlkU3dh cEZyYW1lKCk7CiAKICAgICBpZiAobV9sYXllclRyZWVIb3N0SW1wbC0+aXNDb250ZXh0TG9zdCgp KQo= 130442 2012-03-06 14:14:15 -0800 2012-03-06 15:09:54 -0800 contextLost() check placed before swap bug-80199-20120306141414.patch text/plain 1728 shawnsingh U3VidmVyc2lvbiBSZXZpc2lvbjogMTA5OTM0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjU3MDg4NTA1OThhZjc1 NTVkYjZmMjY5N2UxNWVjMmNhMTMxMDU4Yi4uNmVkOWJiYWUxZjQ0Y2Q0N2YzMGMyZGYwNWUxYzA3 ZmQwYmZiYTI5MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTAzLTA2ICBTaGF3 biBTaW5naCAgPHNoYXduc2luZ2hAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtjaHJvbWl1bV0g TWFrZSBjb21wb3NpdGVBbmRSZWFkYmFjayBhbmQgZGFtYWdlIHRyYWNraW5nIHBsYXkgbmljZWx5 IHRvZ2V0aGVyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD04MDE5OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFJlcXVpcmVzIHN5c3RlbS1sZXZlbCB0ZXN0aW5nIChwaXhlbHMgb24gZnJvbnQtYnVmZmVyIHNo b3VsZCBiZQorICAgICAgICBvYnNlcnZlZCBmb3IgY29ycmVjdCBiZWhhdmlvcikgdGhhdCBjYW5u b3QgYmUgZG9uZSBieSBsYXlvdXQgdGVzdHMKKyAgICAgICAgb3IgdW5pdCB0ZXN0cywgaGF2ZSB0 byByZWx5IG9uIG1hbnVhbCB0ZXN0aW5nLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mv Y2hyb21pdW0vY2MvQ0NTaW5nbGVUaHJlYWRQcm94eS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpD Q1NpbmdsZVRocmVhZFByb3h5Ojpjb21wb3NpdGVBbmRSZWFkYmFjayk6CisKIDIwMTItMDMtMDYg IERhdmlkIENvcnZveXNpZXIgIDxkYXZpZC5jb3J2b3lzaWVyQG9yYW5nZS5jb20+CiAKICAgICAg ICAgVW5yZWxlYXNlZCBnc3Rfb2JqZWN0X3JlZmVyZW5jZSB0byBhdWRpbyBzaW5rIGluIE1lZGlh UGxheWVyUHJpdmF0ZUdTdHJlYW1lcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3MvY2hyb21pdW0vY2MvQ0NTaW5nbGVUaHJlYWRQcm94eS5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ1NpbmdsZVRocmVhZFByb3h5 LmNwcAppbmRleCAwYWYwN2NiZGJmZDI5NTc4MzhmZWMxNDRiMjZhNmRhOWIzZGZjOGRmLi5jYzM5 OWI0OTJhNjhkODcxNWIwYjk1ZjEyMTc2Y2Q5MTkxODBjMGYzIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ1NpbmdsZVRocmVhZFByb3h5 LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9D Q1NpbmdsZVRocmVhZFByb3h5LmNwcApAQCAtODAsMTEgKzgwLDEyIEBAIGJvb2wgQ0NTaW5nbGVU aHJlYWRQcm94eTo6Y29tcG9zaXRlQW5kUmVhZGJhY2sodm9pZCAqcGl4ZWxzLCBjb25zdCBJbnRS ZWN0JiByZWN0CiAKICAgICBtX2xheWVyVHJlZUhvc3RJbXBsLT5yZWFkYmFjayhwaXhlbHMsIHJl Y3QpOwogCi0gICAgZGlkU3dhcEZyYW1lKCk7Ci0KICAgICBpZiAobV9sYXllclRyZWVIb3N0SW1w bC0+aXNDb250ZXh0TG9zdCgpKQogICAgICAgICByZXR1cm4gZmFsc2U7CiAKKyAgICBtX2xheWVy VHJlZUhvc3RJbXBsLT5zd2FwQnVmZmVycygpOworICAgIGRpZFN3YXBGcmFtZSgpOworCiAgICAg cmV0dXJuIHRydWU7CiB9CiAK