132911 2014-05-14 09:48:26 -0700 sourceFrameOnScreenForShareItem: can be off by a pixel 2014-05-14 12:34:02 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Mac All RESOLVED FIXED InRadar P2 Normal --- 1 beidson beidson simon.fraser thorton zalan oldest_to_newest 1009273 0 beidson 2014-05-14 09:48:26 -0700 sourceFrameOnScreenForShareItem: can be off by a pixel This is due largely to sub pixel layout and conversions between FloatRects and IntRects. In radar at <rdar://problem/16878020> 1009283 1 231456 beidson 2014-05-14 10:22:26 -0700 Created attachment 231456 Patch v1 This splits off the client rect calculation into a utility method (which will also be used in bug 132912), and that method is guaranteed to keep everything in terms of floats. Only a single rounding conversion from Float->Int rect takes place. This fixes the known cases of off-by-one-pixel errors. 1009292 2 231456 simon.fraser 2014-05-14 10:38:07 -0700 Comment on attachment 231456 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=231456&action=review > Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm:404 > + return NSRect(); Isn't there an NSEmptyRect or similar? I'm not sure this is constructing an empty rect. > Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm:409 > + return NSRect(); Ditto. > Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm:414 > + // FIXME: Ideally we'd like to convert the content rect to screen coordinates > + // without the lossy float -> int conversion. > + // Creating a rounded int rect works well in practice, but might still lead to > + // off-by-one-pixel problems in edge cases. You should file a bugzilla/radar to track this and reference it here. 1009299 3 beidson 2014-05-14 11:03:30 -0700 http://trac.webkit.org/changeset/168847 1009315 4 231456 darin 2014-05-14 12:34:02 -0700 Comment on attachment 231456 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=231456&action=review >> Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm:404 >> + return NSRect(); > > Isn't there an NSEmptyRect or similar? I'm not sure this is constructing an empty rect. NSZeroRect 231456 2014-05-14 10:22:26 -0700 2014-05-14 12:34:02 -0700 Patch v1 patch text/plain 6453 beidson ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv bWFjL0NoYW5nZUxvZwppbmRleCA5NGFiYmMyLi4wNzRlNDNiIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L21hYy9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMjUgQEAKKzIwMTQtMDUtMTQgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUu Y29tPgorCisgICAgICAgIHNvdXJjZUZyYW1lT25TY3JlZW5Gb3JTaGFyZUl0ZW06IGNhbiBiZSBv ZmYgYnkgYSBwaXhlbAorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTY4NzgwMjA+IGFuZCBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMyOTExCisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW4gYSBsYW5kIG9mIExheW91dFVu aXRzIGFuZCBzdWItcGl4ZWwgcmVuZGVyaW5nLCBjb252ZXJ0aW5nIGJldHdlZW4gSW50UmVjdHMg YW5kCisgICAgICAgIEZsb2F0UmVjdHMgY2FuIGJlIGVycm9yIHByb25lLiBTbyB3ZSBub3cgdHJ5 IHRvIG1pbmltaXplIHN1Y2ggY29udmVyc2lvbnMuCisKKyAgICAgICAgKiBNaXNjL1dlYlNoYXJp bmdTZXJ2aWNlUGlja2VyQ29udHJvbGxlci5tbToKKyAgICAgICAgKC1bV2ViU2hhcmluZ1NlcnZp Y2VQaWNrZXJDb250cm9sbGVyIHNoYXJpbmdTZXJ2aWNlOnNvdXJjZUZyYW1lT25TY3JlZW5Gb3JT aGFyZUl0ZW06XSk6CisKKyAgICAgICAgKiBXZWJDb3JlU3VwcG9ydC9XZWJDb250ZXh0TWVudUNs aWVudC5oOgorICAgICAgICAqIFdlYkNvcmVTdXBwb3J0L1dlYkNvbnRleHRNZW51Q2xpZW50Lm1t OgorICAgICAgICAoV2ViQ29udGV4dE1lbnVDbGllbnQ6OmNsaWVudEZsb2F0UmVjdEZvck5vZGUp OiBTcGxpdCB0aGlzIGludG8gYSBzZXBhcmF0ZSB1dGlsaXR5IG1ldGhvZAorICAgICAgICAgIGFz IGl0IHdpbGwgYmUgc2hhcmVkIHdpdGggb3RoZXIgY29kZSB2ZXJ5IHNvb24uIEluc3RlYWQgb2Yg dXNpbmcgYWJzb2x1dGVCb3VuZGluZ0JveFJlY3QoKQorICAgICAgICAgIHdoaWNoIGRvZXMgbG9z c3kgZmxvYXQtPmludCBjb252ZXJzaW9ucywgdHJ5IHRvIGtlZXAgdGhpbmdzIGluIHRlcm1zIG9m IEZsb2F0UmVjdHMgYXMKKyAgICAgICAgICBtdWNoIGFzIHBvc3NpYmxlLgorICAgICAgICAoV2Vi Q29udGV4dE1lbnVDbGllbnQ6OnNjcmVlblJlY3RGb3JIaXRUZXN0Tm9kZSk6IENhbGwgY2xpZW50 RmxvYXRSZWN0Rm9yTm9kZSB0byBnZXQgdGhlCisgICAgICAgICAgRmxvYXRSZWN0LCB0aGVuIHBl cmZvcm0gdGhlIGxvc3N5IHJvdW5kaW5nIGNvbnZlcnNpb24gdG8gYW4gSW50UmVjdCBvbmx5IG9u Y2UuCisKIDIwMTQtMDUtMTQgIEFudHRpIEtvaXZpc3RvICA8YW50dGlAYXBwbGUuY29tPgogCiAg ICAgICAgIEdJRiBhbmltYXRpb25zIGRvbid0IHJlc3RhcnQgYWZ0ZXIgc2Nyb2xsaW5nIHdpdGgg aU9TIFdlYktpdDEKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvbWFjL01pc2MvV2ViU2hhcmlu Z1NlcnZpY2VQaWNrZXJDb250cm9sbGVyLm1tIGIvU291cmNlL1dlYktpdC9tYWMvTWlzYy9XZWJT aGFyaW5nU2VydmljZVBpY2tlckNvbnRyb2xsZXIubW0KaW5kZXggMTI2ZmFiNi4uMDIwNTI5NiAx MDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvTWlzYy9XZWJTaGFyaW5nU2VydmljZVBpY2tl ckNvbnRyb2xsZXIubW0KKysrIGIvU291cmNlL1dlYktpdC9tYWMvTWlzYy9XZWJTaGFyaW5nU2Vy dmljZVBpY2tlckNvbnRyb2xsZXIubW0KQEAgLTIwOSw3ICsyMDksNyBAQCAtIChOU1JlY3Qpc2hh cmluZ1NlcnZpY2U6KE5TU2hhcmluZ1NlcnZpY2UgKilzaGFyaW5nU2VydmljZSBzb3VyY2VGcmFt ZU9uU2NyZWVuRgogICAgIGlmICghX21lbnVDbGllbnQpCiAgICAgICAgIHJldHVybiBOU1JlY3Qo KTsKIAotICAgIHJldHVybiAoTlNSZWN0KV9tZW51Q2xpZW50LT5zY3JlZW5SZWN0Rm9ySGl0VGVz dE5vZGUoKTsKKyAgICByZXR1cm4gX21lbnVDbGllbnQtPnNjcmVlblJlY3RGb3JIaXRUZXN0Tm9k ZSgpOwogfQogCiAtIChOU1dpbmRvdyAqKXNoYXJpbmdTZXJ2aWNlOihOU1NoYXJpbmdTZXJ2aWNl ICopc2hhcmluZ1NlcnZpY2Ugc291cmNlV2luZG93Rm9yU2hhcmVJdGVtczooTlNBcnJheSAqKWl0 ZW1zIHNoYXJpbmdDb250ZW50U2NvcGU6KE5TU2hhcmluZ0NvbnRlbnRTY29wZSAqKXNoYXJpbmdD b250ZW50U2NvcGUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvbWFjL1dlYkNvcmVTdXBwb3J0 L1dlYkNvbnRleHRNZW51Q2xpZW50LmggYi9Tb3VyY2UvV2ViS2l0L21hYy9XZWJDb3JlU3VwcG9y dC9XZWJDb250ZXh0TWVudUNsaWVudC5oCmluZGV4IGJiNDQ1MzEuLmQxOWUwMDMgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJLaXQvbWFjL1dlYkNvcmVTdXBwb3J0L1dlYkNvbnRleHRNZW51Q2xpZW50 LmgKKysrIGIvU291cmNlL1dlYktpdC9tYWMvV2ViQ29yZVN1cHBvcnQvV2ViQ29udGV4dE1lbnVD bGllbnQuaApAQCAtMzIsNiArMzIsMTAgQEAKIEBjbGFzcyBXZWJTaGFyaW5nU2VydmljZVBpY2tl ckNvbnRyb2xsZXI7CiBAY2xhc3MgV2ViVmlldzsKIAorbmFtZXNwYWNlIFdlYkNvcmUgeworY2xh c3MgTm9kZTsKK30KKwogY2xhc3MgV2ViQ29udGV4dE1lbnVDbGllbnQgOiBwdWJsaWMgV2ViQ29y ZTo6Q29udGV4dE1lbnVDbGllbnQgewogcHVibGljOgogICAgIFdlYkNvbnRleHRNZW51Q2xpZW50 KFdlYlZpZXcgKndlYlZpZXcpOwpAQCAtNTEsNyArNTUsNyBAQCBwdWJsaWM6CiAgICAgdmlydHVh bCB2b2lkIHNlYXJjaFdpdGhTcG90bGlnaHQoKSBvdmVycmlkZTsKICAgICB2aXJ0dWFsIHZvaWQg c2hvd0NvbnRleHRNZW51KCkgb3ZlcnJpZGU7CiAKLSAgICBXZWJDb3JlOjpJbnRSZWN0IHNjcmVl blJlY3RGb3JIaXRUZXN0Tm9kZSgpIGNvbnN0OworICAgIE5TUmVjdCBzY3JlZW5SZWN0Rm9ySGl0 VGVzdE5vZGUoKSBjb25zdDsKIAogI2lmIEVOQUJMRShTRVJWSUNFX0NPTlRST0xTKQogICAgIHZv aWQgY2xlYXJTaGFyaW5nU2VydmljZVBpY2tlckNvbnRyb2xsZXIoKTsKQEAgLTYyLDYgKzY2LDgg QEAgcHVibGljOgogcHJpdmF0ZToKICAgICBOU01lbnUgKmNvbnRleHRNZW51Rm9yRXZlbnQoTlNF dmVudCAqLCBOU1ZpZXcgKik7CiAKKyAgICBib29sIGNsaWVudEZsb2F0UmVjdEZvck5vZGUoV2Vi Q29yZTo6Tm9kZSosIFdlYkNvcmU6OkZsb2F0UmVjdCYpIGNvbnN0OworCiAgICAgV2ViVmlldyAq bV93ZWJWaWV3OwogI2lmIEVOQUJMRShTRVJWSUNFX0NPTlRST0xTKQogICAgIFJldGFpblB0cjxX ZWJTaGFyaW5nU2VydmljZVBpY2tlckNvbnRyb2xsZXI+IG1fc2hhcmluZ1NlcnZpY2VQaWNrZXJD b250cm9sbGVyOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9tYWMvV2ViQ29yZVN1cHBvcnQv V2ViQ29udGV4dE1lbnVDbGllbnQubW0gYi9Tb3VyY2UvV2ViS2l0L21hYy9XZWJDb3JlU3VwcG9y dC9XZWJDb250ZXh0TWVudUNsaWVudC5tbQppbmRleCA1OWUxODg5Li4yNDkxZmZlIDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViS2l0L21hYy9XZWJDb3JlU3VwcG9ydC9XZWJDb250ZXh0TWVudUNsaWVu dC5tbQorKysgYi9Tb3VyY2UvV2ViS2l0L21hYy9XZWJDb3JlU3VwcG9ydC9XZWJDb250ZXh0TWVu dUNsaWVudC5tbQpAQCAtNDcsMTQgKzQ3LDE1IEBACiAjaW1wb3J0IDxXZWJDb3JlL0NvbnRleHRN ZW51Lmg+CiAjaW1wb3J0IDxXZWJDb3JlL0NvbnRleHRNZW51Q29udHJvbGxlci5oPgogI2ltcG9y dCA8V2ViQ29yZS9Eb2N1bWVudC5oPgotI2ltcG9ydCA8V2ViQ29yZS9VUkwuaD4KKyNpbXBvcnQg PFdlYkNvcmUvRnJhbWUuaD4KKyNpbXBvcnQgPFdlYkNvcmUvRnJhbWVWaWV3Lmg+CiAjaW1wb3J0 IDxXZWJDb3JlL0xvY2FsaXplZFN0cmluZ3MuaD4KICNpbXBvcnQgPFdlYkNvcmUvUGFnZS5oPgor I2ltcG9ydCA8V2ViQ29yZS9SZW5kZXJCb3guaD4KICNpbXBvcnQgPFdlYkNvcmUvUmVuZGVyT2Jq ZWN0Lmg+CiAjaW1wb3J0IDxXZWJDb3JlL1NoYXJlZEJ1ZmZlci5oPgotI2ltcG9ydCA8V2ViQ29y ZS9GcmFtZS5oPgotI2ltcG9ydCA8V2ViQ29yZS9GcmFtZVZpZXcuaD4KICNpbXBvcnQgPFdlYkNv cmUvUnVudGltZUFwcGxpY2F0aW9uQ2hlY2tzLmg+CisjaW1wb3J0IDxXZWJDb3JlL1VSTC5oPgog I2ltcG9ydCA8V2ViS2l0TGVnYWN5L0RPTVByaXZhdGUuaD4KIAogdXNpbmcgbmFtZXNwYWNlIFdl YkNvcmU7CkBAIC0zNjQsMzIgKzM2NSw1NSBAQCB2b2lkIFdlYkNvbnRleHRNZW51Q2xpZW50Ojpz dG9wU3BlYWtpbmcoKQogICAgIFtOU0FwcCBzdG9wU3BlYWtpbmc6bmlsXTsKIH0KIAotSW50UmVj dCBXZWJDb250ZXh0TWVudUNsaWVudDo6c2NyZWVuUmVjdEZvckhpdFRlc3ROb2RlKCkgY29uc3QK K2Jvb2wgV2ViQ29udGV4dE1lbnVDbGllbnQ6OmNsaWVudEZsb2F0UmVjdEZvck5vZGUoTm9kZSog bm9kZSwgRmxvYXRSZWN0JiByZWN0KSBjb25zdAogewotICAgIFBhZ2UqIHBhZ2UgPSBbbV93ZWJW aWV3IHBhZ2VdOwotICAgIGlmICghcGFnZSkKLSAgICAgICAgcmV0dXJuIEludFJlY3QoKTsKLQot ICAgIE5vZGUqIG5vZGUgPSBwYWdlLT5jb250ZXh0TWVudUNvbnRyb2xsZXIoKS5jb250ZXh0KCku aGl0VGVzdFJlc3VsdCgpLmlubmVyTm9kZSgpOwotICAgIGlmICghbm9kZSkKLSAgICAgICAgcmV0 dXJuIEludFJlY3QoKTsKKyAgICBBU1NFUlQobm9kZSk7CiAKICAgICBSZW5kZXJPYmplY3QqIHJl bmRlcmVyID0gbm9kZS0+cmVuZGVyZXIoKTsKICAgICBpZiAoIXJlbmRlcmVyKSB7CiAgICAgICAg IC8vIFRoaXMgbWV0aG9kIHNob3VsZG4ndCBiZSBjYWxsZWQgaW4gY2FzZXMgd2hlcmUgdGhlIGNv bnRyb2xsZWQgbm9kZSBoYXNuJ3QgcmVuZGVyZWQuCiAgICAgICAgIEFTU0VSVF9OT1RfUkVBQ0hF RCgpOwotICAgICAgICByZXR1cm4gSW50UmVjdCgpOworICAgICAgICByZXR1cm4gZmFsc2U7CiAg ICAgfQogCi0gICAgSW50UmVjdCByZWN0ID0gcmVuZGVyZXItPmFic29sdXRlQm91bmRpbmdCb3hS ZWN0KCk7CisgICAgaWYgKCFyZW5kZXJlci0+aXNCb3goKSkKKyAgICAgICAgcmV0dXJuIGZhbHNl OworICAgIFJlbmRlckJveCogcmVuZGVyQm94ID0gdG9SZW5kZXJCb3gocmVuZGVyZXIpOworCisg ICAgTGF5b3V0UmVjdCBsYXlvdXRSZWN0ID0gcmVuZGVyQm94LT5jbGllbnRCb3hSZWN0KCk7Cisg ICAgRmxvYXRRdWFkIGZsb2F0UXVhZCA9IHJlbmRlckJveC0+bG9jYWxUb0Fic29sdXRlUXVhZChG bG9hdFF1YWQobGF5b3V0UmVjdCkpOworICAgIHJlY3QgPSBmbG9hdFF1YWQuYm91bmRpbmdCb3go KTsKKworICAgIHJldHVybiB0cnVlOworfQorCitOU1JlY3QgV2ViQ29udGV4dE1lbnVDbGllbnQ6 OnNjcmVlblJlY3RGb3JIaXRUZXN0Tm9kZSgpIGNvbnN0Cit7CisgICAgUGFnZSogcGFnZSA9IFtt X3dlYlZpZXcgcGFnZV07CisgICAgaWYgKCFwYWdlKQorICAgICAgICByZXR1cm4gTlNSZWN0KCk7 CisKKyAgICBOb2RlKiBub2RlID0gcGFnZS0+Y29udGV4dE1lbnVDb250cm9sbGVyKCkuY29udGV4 dCgpLmhpdFRlc3RSZXN1bHQoKS5pbm5lck5vZGUoKTsKKyAgICBpZiAoIW5vZGUpCisgICAgICAg IHJldHVybiBOU1JlY3QoKTsKKwogICAgIEZyYW1lVmlldyogZnJhbWVWaWV3ID0gbm9kZS0+ZG9j dW1lbnQoKS52aWV3KCk7CiAgICAgaWYgKCFmcmFtZVZpZXcpIHsKICAgICAgICAgLy8gVGhpcyBt ZXRob2Qgc2hvdWxkbid0IGJlIGNhbGxlZCBpbiBjYXNlcyB3aGVyZSB0aGUgY29udHJvbGxlZCBu b2RlIGlzbid0IGluIGEgcmVuZGVyZWQgdmlldy4KICAgICAgICAgQVNTRVJUX05PVF9SRUFDSEVE KCk7Ci0gICAgICAgIHJldHVybiBJbnRSZWN0KCk7CisgICAgICAgIHJldHVybiBOU1JlY3QoKTsK ICAgICB9CiAKLSAgICByZXR1cm4gZnJhbWVWaWV3LT5jb250ZW50c1RvU2NyZWVuKHJlY3QpOwor ICAgIEZsb2F0UmVjdCByZWN0OworICAgIGlmICghY2xpZW50RmxvYXRSZWN0Rm9yTm9kZShub2Rl LCByZWN0KSkKKyAgICAgICAgcmV0dXJuIE5TUmVjdCgpOworCisgICAgLy8gRklYTUU6IElkZWFs bHkgd2UnZCBsaWtlIHRvIGNvbnZlcnQgdGhlIGNvbnRlbnQgcmVjdCB0byBzY3JlZW4gY29vcmRp bmF0ZXMKKyAgICAvLyB3aXRob3V0IHRoZSBsb3NzeSBmbG9hdCAtPiBpbnQgY29udmVyc2lvbi4K KyAgICAvLyBDcmVhdGluZyBhIHJvdW5kZWQgaW50IHJlY3Qgd29ya3Mgd2VsbCBpbiBwcmFjdGlj ZSwgYnV0IG1pZ2h0IHN0aWxsIGxlYWQgdG8KKyAgICAvLyBvZmYtYnktb25lLXBpeGVsIHByb2Js ZW1zIGluIGVkZ2UgY2FzZXMuCisgICAgSW50UmVjdCBpbnRSZWN0KHJlY3QpOworICAgIHJldHVy biBmcmFtZVZpZXctPmNvbnRlbnRzVG9TY3JlZW4oaW50UmVjdCk7CiB9CiAKIE5TTWVudSAqV2Vi Q29udGV4dE1lbnVDbGllbnQ6OmNvbnRleHRNZW51Rm9yRXZlbnQoTlNFdmVudCAqZXZlbnQsIE5T VmlldyAqdmlldykK