93795 2012-08-12 23:20:54 -0700 [chromium] renderSurface in incorrect space if owning layer has empty but non-zero bounds 2012-08-13 18:04:07 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 shawnsingh shawnsingh cc-bugs enne jamesr webkit.review.bot oldest_to_newest 693389 0 shawnsingh 2012-08-12 23:20:54 -0700 A particular translation transform was inside an if-statement that caused it to be accidentally skipped when it shouldn't be. The fix is simply to move it outside of the if-statement so it is not skipped. I have a feeling the other transform that remains inside the if-statement will also get incorrectly skipped, affecting correctness in high-DPI mode. However, that one is not as clear how to fix (if it actually is a problem in the first place?), because not-skipping it means that we might be scaling by a zero width or height, causing the entire subtree to become invisible. It might work to have separate scale transforms for width and height, and skip them individually when appropriate, but i'm not completely sure that would work as intended either. So yeah, if it's OK with reviewers I would like to land this simple straightforward part first, get it merged to m22 sooner, and then deal with the high-DPI part separately. 693398 1 157926 shawnsingh 2012-08-12 23:37:41 -0700 Created attachment 157926 Patch tested on osx with layout and unit tests. Also fixed one comment that used right-to-left name... We have discussed possibly switching the naming convention of transforms to that style, but for now those transforms were being written so that their name interprets what happens to the coordinate space from left-to-right. 693691 2 enne 2012-08-13 08:47:04 -0700 (In reply to comment #0) > A particular translation transform was inside an if-statement that caused it to be accidentally skipped when it shouldn't be. The fix is simply to move it outside of the if-statement so it is not skipped. > > I have a feeling the other transform that remains inside the if-statement will also get incorrectly skipped, affecting correctness in high-DPI mode. I am less sure about that, actually. I think the surface-creating code makes some assumptions that device scale = contentsScale(), and it's possible that this other transform might be moot. Along with a degenerate case like this bug with bounds() != contentBounds() case, I also worry about a layer with bounds() == contentBounds() creating a surface when deviceScale != 1. > So yeah, if it's OK with reviewers I would like to land this simple straightforward part first, get it merged to m22 sooner, and then deal with the high-DPI part separately. Is there a separate bug for investigating the high-DPI end of things? Is this something that you can also look into? 693692 3 157926 enne 2012-08-13 08:48:17 -0700 Comment on attachment 157926 Patch R=me. That's a really elegant test, to have all draw transforms come out to be identity. :) 694317 4 shawnsingh 2012-08-13 17:58:33 -0700 Thanks for the review =) I created https://bugs.webkit.org/show_bug.cgi?id=93919 for the highDPI questions we have. Of course please correct me if my comments there are not actually what you were worried about. =) 694325 5 shawnsingh 2012-08-13 18:04:07 -0700 Committed r125486: <http://trac.webkit.org/changeset/125486> 157926 2012-08-12 23:37:41 -0700 2012-08-13 08:48:17 -0700 Patch bug-93795-20120812233710.patch text/plain 6215 shawnsingh U3VidmVyc2lvbiBSZXZpc2lvbjogMTI1MzkwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDg2MjEyZThhMzA2Mjdm ZDc3ZWM0NDMzY2ZhMGJhNzE4NWQyOWU4Zi4uYTc1ODVmMTUwNTI2NDM4Yjg2NTVmZTgyYzFhOGNk NTM2N2E4MGNmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEyLTA4LTEyICBTaGF3 biBTaW5naCAgPHNoYXduc2luZ2hAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtjaHJvbWl1bV0g cmVuZGVyU3VyZmFjZSBpbiBpbmNvcnJlY3Qgc3BhY2UgaWYgb3duaW5nIGxheWVyIGhhcyBlbXB0 eSBidXQgbm9uLXplcm8gYm91bmRzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD05Mzc5NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIElmIGEgcmVuZGVyU3VyZmFjZSBpcyBjcmVhdGVkIGJ5IGEgbGF5ZXIgdGhh dCBoYWQgemVybworICAgICAgICBhcmVhIChlbXB0eSkgYnV0IG5vbi16ZXJvIGJvdW5kcyAoZWl0 aGVyIHdpZHRoIG9yIGhlaWdodCB3YXMKKyAgICAgICAgbm9uLXplcm8pLCB0aGVuIG9uZSB0cmFu c2xhdGlvbiB0cmFuc2Zvcm0gd2FzIGFjY2lkZW50YWxseSBiZWluZworICAgICAgICBza2lwcGVk LCBjYXVzaW5nIHRoZSByZW5kZXJTdXJmYWNlIGRyYXdUcmFuc2Zvcm0gdG8gYmUgaW5jb3JyZWN0 LgorICAgICAgICBUaGUgZml4IGlzIHNpbXBseSB0byBtb3ZlIHRoYXQgdHJhbnNmb3JtIG91dHNp ZGUgb2YgdGhlCisgICAgICAgIGlmLXN0YXRlbWVudCBzbyBpdCBpcyBub3Qgc2tpcHBlZC4KKwor ICAgICAgICBVbml0IHRlc3QgYWRkZWQ6CisgICAgICAgICAgQ0NMYXllclRyZWVIb3N0Q29tbW9u VGVzdC52ZXJpZnlUcmFuc2Zvcm1zRm9yRGVnZW5lcmF0ZUludGVybWVkaWF0ZUxheWVyKCkKKwor ICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDTGF5ZXJUcmVlSG9zdENv bW1vbi5jcHA6CisgICAgICAgIChXZWJDb3JlOjpjYWxjdWxhdGVEcmF3VHJhbnNmb3Jtc0ludGVy bmFsKToKKwogMjAxMi0wOC0xMiAgRGFuIEJlcm5zdGVpbiAgPG1pdHpAYXBwbGUuY29tPgogCiAg ICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xMTQ0MjQ4NT4gb2ZmLXNjcmVlbiBvciBpbmFjdGl2ZSBw YWdlcyBjYW4gY2hhbmdlIHRoZSBtb3VzZSBwb2ludGVyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi S2l0L2Nocm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9n CmluZGV4IDU3NWNjYTA2OTUxYjU1ZTYyNTBjNTg4YzBjM2Q0OGY3YjIwZTE5MTguLjZkM2UxMDFm MjFiYjBmZTM2MTQxYmJmMDg5NmQxMmQ4MzY2MmQzOWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvY2hyb21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdl TG9nCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTItMDgtMTIgIFNoYXduIFNpbmdoICA8c2hhd25zaW5n aEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgW2Nocm9taXVtXSByZW5kZXJTdXJmYWNlIGluIGlu Y29ycmVjdCBzcGFjZSBpZiBvd25pbmcgbGF5ZXIgaGFzIGVtcHR5IGJ1dCBub24temVybyBib3Vu ZHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkzNzk1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiB0ZXN0 cy9DQ0xheWVyVHJlZUhvc3RDb21tb25UZXN0LmNwcDoKKwogMjAxMi0wOC0xMiAgWXV0YSBLaXRh bXVyYSAgPHl1dGFrQGdvb2dsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZC4gRml4IENocm9t aXVtJ3MgQW5kcm9pZCBidWlsZC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDTGF5ZXJUcmVlSG9zdENvbW1vbi5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9jYy9DQ0xheWVyVHJlZUhvc3RDb21t b24uY3BwCmluZGV4IDlmNGEwMzVkNzY4MTlkOGM0ODNjNWRkNzQ3NDU1ZWJhYjlkZDE4N2MuLjgx ZTU2NWRhYWVlYTA2ZWU2ZGU2MjBmYzgzNzkzOWYxZDdiZGYwYmMgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDTGF5ZXJUcmVlSG9zdENv bW1vbi5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0v Y2MvQ0NMYXllclRyZWVIb3N0Q29tbW9uLmNwcApAQCAtNTAyLDEwICs1MDIsMTAgQEAgc3RhdGlj IHZvaWQgY2FsY3VsYXRlRHJhd1RyYW5zZm9ybXNJbnRlcm5hbChMYXllclR5cGUqIGxheWVyLCBM YXllclR5cGUqIHJvb3RMYXkKICAgICAvLyBUaGUgZHJhd1RyYW5zZm9ybSB0aGF0IGdldHMgY29t cHV0ZWQgYmVsb3cgaXMgZWZmZWN0aXZlbHkgdGhlIGxheWVyJ3MgZHJhd1RyYW5zZm9ybSwgdW5s ZXNzCiAgICAgLy8gdGhlIGxheWVyIGl0c2VsZiBjcmVhdGVzIGEgcmVuZGVyU3VyZmFjZS4gSW4g dGhhdCBjYXNlLCB0aGUgcmVuZGVyU3VyZmFjZSByZS1wYXJlbnRzIHRoZSB0cmFuc2Zvcm1zLgog ICAgIFdlYlRyYW5zZm9ybWF0aW9uTWF0cml4IGRyYXdUcmFuc2Zvcm0gPSBjb21iaW5lZFRyYW5z Zm9ybTsKKyAgICAvLyBNW2RyYXddID0gTVtwYXJlbnRdICogTFQgKiBUclthbmNob3IyY2VudGVy XSAqIFRyW2NlbnRlcjJvcmlnaW5dCisgICAgZHJhd1RyYW5zZm9ybS50cmFuc2xhdGUoLWxheWVy LT5ib3VuZHMoKS53aWR0aCgpIC8gMi4wLCAtbGF5ZXItPmJvdW5kcygpLmhlaWdodCgpIC8gMi4w KTsKICAgICBpZiAoIWxheWVyLT5jb250ZW50Qm91bmRzKCkuaXNFbXB0eSgpICYmICFsYXllci0+ Ym91bmRzKCkuaXNFbXB0eSgpKSB7Ci0gICAgICAgIC8vIE1bZHJhd10gPSBNW3BhcmVudF0gKiBM VCAqIFRyW2FuY2hvcjJjZW50ZXJdICogVHJbY2VudGVyMmFuY2hvcl0KLSAgICAgICAgZHJhd1Ry YW5zZm9ybS50cmFuc2xhdGUoLWxheWVyLT5ib3VuZHMoKS53aWR0aCgpIC8gMi4wLCAtbGF5ZXIt PmJvdW5kcygpLmhlaWdodCgpIC8gMi4wKTsKLSAgICAgICAgLy8gTVtkcmF3XSA9IE1bcGFyZW50 XSAqIExUICogVHJbYW5jaG9yMm9yaWdpbl0gKiBTW2NvbnRlbnQybGF5ZXJdCisgICAgICAgIC8v IE1bZHJhd10gPSBNW3BhcmVudF0gKiBMVCAqIFRyW2FuY2hvcjJvcmlnaW5dICogU1tsYXllcjJj b250ZW50XQogICAgICAgICBkcmF3VHJhbnNmb3JtLnNjYWxlTm9uVW5pZm9ybShsYXllci0+Ym91 bmRzKCkud2lkdGgoKSAvIHN0YXRpY19jYXN0PGRvdWJsZT4obGF5ZXItPmNvbnRlbnRCb3VuZHMo KS53aWR0aCgpKSwKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgbGF5ZXIt PmJvdW5kcygpLmhlaWdodCgpIC8gc3RhdGljX2Nhc3Q8ZG91YmxlPihsYXllci0+Y29udGVudEJv dW5kcygpLmhlaWdodCgpKSk7CiAgICAgfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9jaHJv bWl1bS90ZXN0cy9DQ0xheWVyVHJlZUhvc3RDb21tb25UZXN0LmNwcCBiL1NvdXJjZS9XZWJLaXQv Y2hyb21pdW0vdGVzdHMvQ0NMYXllclRyZWVIb3N0Q29tbW9uVGVzdC5jcHAKaW5kZXggOWZkMjI2 ODU4YmRhZGMwZmU1NTIxYzNhZDFjZTAxZDgwMjE1NGIzZC4uMWU2YTU5OGZjYjRhMTk3MDM4ZmRj MzdjZDQ2OGYzNzc2MjY0ZDZhOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9jaHJvbWl1bS90 ZXN0cy9DQ0xheWVyVHJlZUhvc3RDb21tb25UZXN0LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL3Rlc3RzL0NDTGF5ZXJUcmVlSG9zdENvbW1vblRlc3QuY3BwCkBAIC02MDksNiArNjA5 LDM2IEBAIFRFU1QoQ0NMYXllclRyZWVIb3N0Q29tbW9uVGVzdCwgdmVyaWZ5VHJhbnNmb3Jtc0Zv ckZsYXR0ZW5pbmdMYXllcikKICAgICBFWFBFQ1RfVFJBTlNGT1JNQVRJT05fTUFUUklYX0VRKGV4 cGVjdGVkR3JhbmRDaGlsZFNjcmVlblNwYWNlVHJhbnNmb3JtLCBncmFuZENoaWxkLT5zY3JlZW5T cGFjZVRyYW5zZm9ybSgpKTsKIH0KIAorVEVTVChDQ0xheWVyVHJlZUhvc3RDb21tb25UZXN0LCB2 ZXJpZnlUcmFuc2Zvcm1zRm9yRGVnZW5lcmF0ZUludGVybWVkaWF0ZUxheWVyKQoreworICAgIC8v IEEgbGF5ZXIgdGhhdCBpcyBlbXB0eSBpbiBvbmUgYXhpcywgYnV0IG5vdCB0aGUgb3RoZXIsIHdh cyBhY2NpZGVudGFsbHkgc2tpcHBpbmcgYSBuZWNlc3NhcnkgdHJhbnNsYXRpb24uCisgICAgLy8g V2l0aG91dCB0aGF0IHRyYW5zbGF0aW9uLCB0aGUgY29vcmRpbmF0ZSBzcGFjZSBvZiB0aGUgbGF5 ZXIncyBkcmF3VHJhbnNmb3JtIGlzIGluY29ycmVjdC4KKyAgICAvLworICAgIC8vIE5vcm1hbGx5 IHRoaXMgaXNuJ3QgYSBwcm9ibGVtLCBiZWNhdXNlIHRoZSBsYXllciB3b3VsZG4ndCBiZSBkcmF3 biBhbnl3YXksIGJ1dCBpZiB0aGF0IGxheWVyIGJlY29tZXMgYSByZW5kZXJTdXJmYWNlLCB0aGVu CisgICAgLy8gaXRzIGRyYXdUcmFuc2Zvcm0gaXMgaW1wbGljaXRseSBpbmhlcml0ZWQgYnkgdGhl IHJlc3Qgb2YgdGhlIHN1YnRyZWUsIHdoaWNoIHRoZW4gaXMgcG9zaXRpb25lZCBpbmNvcnJlY3Rs eSBhcyBhIHJlc3VsdC4KKworICAgIFJlZlB0cjxMYXllckNocm9taXVtPiByb290ID0gTGF5ZXJD aHJvbWl1bTo6Y3JlYXRlKCk7CisgICAgUmVmUHRyPExheWVyQ2hyb21pdW0+IGNoaWxkID0gTGF5 ZXJDaHJvbWl1bTo6Y3JlYXRlKCk7CisgICAgUmVmUHRyPExheWVyQ2hyb21pdW1XaXRoRm9yY2Vk RHJhd3NDb250ZW50PiBncmFuZENoaWxkID0gYWRvcHRSZWYobmV3IExheWVyQ2hyb21pdW1XaXRo Rm9yY2VkRHJhd3NDb250ZW50KCkpOworCisgICAgLy8gVGhlIGNoaWxkIGhlaWdodCBpcyB6ZXJv LCBidXQgaGFzIG5vbi16ZXJvIHdpZHRoIHRoYXQgc2hvdWxkIGJlIGFjY291bnRlZCBmb3Igd2hp bGUgY29tcHV0aW5nIGRyYXdUcmFuc2Zvcm1zLgorICAgIGNvbnN0IFdlYlRyYW5zZm9ybWF0aW9u TWF0cml4IGlkZW50aXR5TWF0cml4OworICAgIHNldExheWVyUHJvcGVydGllc0ZvclRlc3Rpbmco cm9vdC5nZXQoKSwgaWRlbnRpdHlNYXRyaXgsIGlkZW50aXR5TWF0cml4LCBGbG9hdFBvaW50Ojp6 ZXJvKCksIEZsb2F0UG9pbnQ6Onplcm8oKSwgSW50U2l6ZSgxMDAsIDEwMCksIGZhbHNlKTsKKyAg ICBzZXRMYXllclByb3BlcnRpZXNGb3JUZXN0aW5nKGNoaWxkLmdldCgpLCBpZGVudGl0eU1hdHJp eCwgaWRlbnRpdHlNYXRyaXgsIEZsb2F0UG9pbnQ6Onplcm8oKSwgRmxvYXRQb2ludDo6emVybygp LCBJbnRTaXplKDEwLCAwKSwgZmFsc2UpOworICAgIHNldExheWVyUHJvcGVydGllc0ZvclRlc3Rp bmcoZ3JhbmRDaGlsZC5nZXQoKSwgaWRlbnRpdHlNYXRyaXgsIGlkZW50aXR5TWF0cml4LCBGbG9h dFBvaW50Ojp6ZXJvKCksIEZsb2F0UG9pbnQ6Onplcm8oKSwgSW50U2l6ZSgxMCwgMTApLCBmYWxz ZSk7CisKKyAgICByb290LT5hZGRDaGlsZChjaGlsZCk7CisgICAgY2hpbGQtPmFkZENoaWxkKGdy YW5kQ2hpbGQpOworICAgIGNoaWxkLT5zZXRGb3JjZVJlbmRlclN1cmZhY2UodHJ1ZSk7CisKKyAg ICBleGVjdXRlQ2FsY3VsYXRlRHJhd1RyYW5zZm9ybXNBbmRWaXNpYmlsaXR5KHJvb3QuZ2V0KCkp OworCisgICAgQVNTRVJUX1RSVUUoY2hpbGQtPnJlbmRlclN1cmZhY2UoKSk7CisgICAgRVhQRUNU X1RSQU5TRk9STUFUSU9OX01BVFJJWF9FUShpZGVudGl0eU1hdHJpeCwgY2hpbGQtPnJlbmRlclN1 cmZhY2UoKS0+ZHJhd1RyYW5zZm9ybSgpKTsgLy8gVGhpcyBpcyB0aGUgcmVhbCB0ZXN0LCB0aGUg cmVzdCBhcmUgc2FuaXR5IGNoZWNrcy4KKyAgICBFWFBFQ1RfVFJBTlNGT1JNQVRJT05fTUFUUklY X0VRKGlkZW50aXR5TWF0cml4LCBjaGlsZC0+ZHJhd1RyYW5zZm9ybSgpKTsKKyAgICBFWFBFQ1Rf VFJBTlNGT1JNQVRJT05fTUFUUklYX0VRKGlkZW50aXR5TWF0cml4LCBncmFuZENoaWxkLT5kcmF3 VHJhbnNmb3JtKCkpOworfQorCiBURVNUKENDTGF5ZXJUcmVlSG9zdENvbW1vblRlc3QsIHZlcmlm eVJlbmRlclN1cmZhY2VMaXN0Rm9yUmVuZGVyU3VyZmFjZVdpdGhDbGlwcGVkTGF5ZXIpCiB7CiAg ICAgUmVmUHRyPExheWVyQ2hyb21pdW0+IHBhcmVudCA9IExheWVyQ2hyb21pdW06OmNyZWF0ZSgp Owo=