185445 2018-05-08 13:49:53 -0700 Set colorspace in the PDF plugin. 2018-05-08 18:35:03 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 pvollan pvollan bfulgham commit-queue simon.fraser thorton webkit-bug-importer oldest_to_newest 1421914 0 pvollan 2018-05-08 13:49:53 -0700 We should set the colorspace in the PDF plugin. 1421916 1 pvollan 2018-05-08 13:50:44 -0700 <rdar://problem/40030981> 1421919 2 339869 pvollan 2018-05-08 13:53:00 -0700 Created attachment 339869 Patch 1421968 3 339869 bfulgham 2018-05-08 15:18:18 -0700 Comment on attachment 339869 Patch r=me. 1421971 4 339869 thorton 2018-05-08 15:19:25 -0700 Comment on attachment 339869 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339869&action=review > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:637 > + if ([m_pdfLayerController.get() respondsToSelector:@selector(setDeviceColorSpace:)]) No need for the .get()s 1421972 5 339869 thorton 2018-05-08 15:20:19 -0700 Comment on attachment 339869 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339869&action=review > Source/WebKit/WebProcess/Plugins/PDF/PDFLayerControllerSPI.h:170 > +- (void) setDeviceColorSpace:(CGColorSpaceRef)colorSpace; No space after the ) >> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:637 >> + if ([m_pdfLayerController.get() respondsToSelector:@selector(setDeviceColorSpace:)]) > > No need for the .get()s No need for the .get()s > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:638 > + [m_pdfLayerController.get() setDeviceColorSpace:screenColorSpace()]; What does screenColorSpace do? It seems like it will usually be wrong, and we should be getting it from the page/view, not the screen (because there are multiple screens). 1421977 6 339869 bfulgham 2018-05-08 15:30:42 -0700 Comment on attachment 339869 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339869&action=review >> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:638 >> + [m_pdfLayerController.get() setDeviceColorSpace:screenColorSpace()]; > > What does screenColorSpace do? It seems like it will usually be wrong, and we should be getting it from the page/view, not the screen (because there are multiple screens). You could call screenColorSpace(this) maybe? The PDFPlugin's Widget will be tied to the correct screen, then 'screenColorSpace' can give you the right answer, tied to the display the PDF is being shown on. 1421979 7 339889 pvollan 2018-05-08 15:33:14 -0700 Created attachment 339889 Patch 1421982 8 pvollan 2018-05-08 15:36:31 -0700 (In reply to Brent Fulgham from comment #6) > Comment on attachment 339869 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339869&action=review > > >> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:638 > >> + [m_pdfLayerController.get() setDeviceColorSpace:screenColorSpace()]; > > > > What does screenColorSpace do? It seems like it will usually be wrong, and we should be getting it from the page/view, not the screen (because there are multiple screens). > > You could call screenColorSpace(this) maybe? The PDFPlugin's Widget will be > tied to the correct screen, then 'screenColorSpace' can give you the right > answer, tied to the display the PDF is being shown on. I used the PDF frame's view in the latest patch. Thanks for reviewing, all! 1421993 9 339889 simon.fraser 2018-05-08 16:03:45 -0700 Comment on attachment 339889 Patch How does this update if the screen colorspace changes? 1421995 10 pvollan 2018-05-08 16:11:05 -0700 (In reply to Simon Fraser (smfr) from comment #9) > Comment on attachment 339889 [details] > Patch > > How does this update if the screen colorspace changes? That is a good point ... The current patch does not handle this, I believe. Thanks for reviewing! 1422095 11 339889 commit-queue 2018-05-08 18:35:01 -0700 Comment on attachment 339889 Patch Clearing flags on attachment: 339889 Committed r231535: <https://trac.webkit.org/changeset/231535> 1422096 12 commit-queue 2018-05-08 18:35:03 -0700 All reviewed patches have been landed. Closing bug. 339869 2018-05-08 13:53:00 -0700 2018-05-08 15:33:13 -0700 Patch bug-185445-20180508135300.patch text/plain 2286 pvollan SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCShyZXZpc2lvbiAyMzE1MTApCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE4LTA1LTA4ICBQZXIgQXJuZSBW b2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBTZXQgY29sb3JzcGFjZSBpbiB0 aGUgUERGIHBsdWdpbi4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE4NTQ0NQorICAgICAgICA8cmRhcjovL3Byb2JsZW0vNDAwMzA5ODE+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1Bs dWdpbnMvUERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oOgorICAgICAgICAqIFdlYlByb2Nlc3Mv UGx1Z2lucy9QREYvUERGUGx1Z2luLm1tOgorICAgICAgICAoV2ViS2l0OjpQREZQbHVnaW46OlBE RlBsdWdpbik6CisKIDIwMTgtMDUtMDggIFBlciBBcm5lIFZvbGxhbiAgPHB2b2xsYW5AYXBwbGUu Y29tPgogCiAgICAgICAgIFRoZSBQREYgY29udGV4dCBtZW51IHNob3VsZCBub3QgYmUgY3JlYXRl ZCBpbiB0aGUgV2ViQ29udGVudCBwcm9jZXNzLgpJbmRleDogU291cmNlL1dlYktpdC9XZWJQcm9j ZXNzL1BsdWdpbnMvUERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVnaW5zL1BERi9QREZMYXllckNvbnRyb2xsZXJTUEku aAkocmV2aXNpb24gMjMxNTEwKQorKysgU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1BsdWdpbnMv UERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oCSh3b3JraW5nIGNvcHkpCkBAIC0xNjYsNiArMTY2 LDkgQEAgdHlwZWRlZiBOU19FTlVNKE5TSW50ZWdlciwgUERGTGF5ZXJDb250cgogLSAoaWQpYWNj ZXNzaWJpbGl0eUVsZW1lbnRGb3JBbm5vdGF0aW9uOihQREZBbm5vdGF0aW9uICopYW5ub3RhdGlv bjsKICNlbmRpZgogCisjaWYgX19NQUNfT1NfWF9WRVJTSU9OX01JTl9SRVFVSVJFRCA+PSAxMDE0 MDAKKy0gKHZvaWQpIHNldERldmljZUNvbG9yU3BhY2U6KENHQ29sb3JTcGFjZVJlZiljb2xvclNw YWNlOworI2VuZGlmCiBAZW5kCiAKICNpZiBfX01BQ19PU19YX1ZFUlNJT05fTUlOX1JFUVVJUkVE ID49IDEwMTMwMApJbmRleDogU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1BsdWdpbnMvUERGL1BE RlBsdWdpbi5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1Z2lu cy9QREYvUERGUGx1Z2luLm1tCShyZXZpc2lvbiAyMzE1MTApCisrKyBTb3VyY2UvV2ViS2l0L1dl YlByb2Nlc3MvUGx1Z2lucy9QREYvUERGUGx1Z2luLm1tCSh3b3JraW5nIGNvcHkpCkBAIC03NCw2 ICs3NCw3IEBACiAjaW1wb3J0IDxXZWJDb3JlL1BERkRvY3VtZW50SW1hZ2UuaD4KICNpbXBvcnQg PFdlYkNvcmUvUGFnZS5oPgogI2ltcG9ydCA8V2ViQ29yZS9QYXN0ZWJvYXJkLmg+CisjaW1wb3J0 IDxXZWJDb3JlL1BsYXRmb3JtU2NyZWVuLmg+CiAjaW1wb3J0IDxXZWJDb3JlL1BsdWdpbkRhdGEu aD4KICNpbXBvcnQgPFdlYkNvcmUvUGx1Z2luRG9jdW1lbnQuaD4KICNpbXBvcnQgPFdlYkNvcmUv UmVuZGVyQm94TW9kZWxPYmplY3QuaD4KQEAgLTYzMiw2ICs2MzMsMTAgQEAgaW5saW5lIFBERlBs dWdpbjo6UERGUGx1Z2luKFdlYkZyYW1lJiBmcgogCiAgICAgW21fY29udGFpbmVyTGF5ZXIgYWRk U3VibGF5ZXI6bV9jb250ZW50TGF5ZXIuZ2V0KCldOwogICAgIFttX2NvbnRhaW5lckxheWVyIGFk ZFN1YmxheWVyOm1fc2Nyb2xsQ29ybmVyTGF5ZXIuZ2V0KCldOworI2lmIF9fTUFDX09TX1hfVkVS U0lPTl9NSU5fUkVRVUlSRUQgPj0gMTAxNDAwCisgICAgaWYgKFttX3BkZkxheWVyQ29udHJvbGxl ci5nZXQoKSByZXNwb25kc1RvU2VsZWN0b3I6QHNlbGVjdG9yKHNldERldmljZUNvbG9yU3BhY2U6 KV0pCisgICAgICAgIFttX3BkZkxheWVyQ29udHJvbGxlci5nZXQoKSBzZXREZXZpY2VDb2xvclNw YWNlOnNjcmVlbkNvbG9yU3BhY2UoKV07CisjZW5kaWYKIH0KIAogUERGUGx1Z2luOjp+UERGUGx1 Z2luKCkK 339889 2018-05-08 15:33:14 -0700 2018-05-08 18:35:01 -0700 Patch bug-185445-20180508153313.patch text/plain 2340 pvollan SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCShyZXZpc2lvbiAyMzE1MTApCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE4LTA1LTA4ICBQZXIgQXJuZSBW b2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBTZXQgY29sb3JzcGFjZSBpbiB0 aGUgUERGIHBsdWdpbi4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE4NTQ0NQorICAgICAgICA8cmRhcjovL3Byb2JsZW0vNDAwMzA5ODE+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1Bs dWdpbnMvUERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oOgorICAgICAgICAqIFdlYlByb2Nlc3Mv UGx1Z2lucy9QREYvUERGUGx1Z2luLm1tOgorICAgICAgICAoV2ViS2l0OjpQREZQbHVnaW46OlBE RlBsdWdpbik6CisKIDIwMTgtMDUtMDggIFBlciBBcm5lIFZvbGxhbiAgPHB2b2xsYW5AYXBwbGUu Y29tPgogCiAgICAgICAgIFRoZSBQREYgY29udGV4dCBtZW51IHNob3VsZCBub3QgYmUgY3JlYXRl ZCBpbiB0aGUgV2ViQ29udGVudCBwcm9jZXNzLgpJbmRleDogU291cmNlL1dlYktpdC9XZWJQcm9j ZXNzL1BsdWdpbnMvUERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVnaW5zL1BERi9QREZMYXllckNvbnRyb2xsZXJTUEku aAkocmV2aXNpb24gMjMxNTEwKQorKysgU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1BsdWdpbnMv UERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oCSh3b3JraW5nIGNvcHkpCkBAIC0xNjYsNiArMTY2 LDkgQEAgdHlwZWRlZiBOU19FTlVNKE5TSW50ZWdlciwgUERGTGF5ZXJDb250cgogLSAoaWQpYWNj ZXNzaWJpbGl0eUVsZW1lbnRGb3JBbm5vdGF0aW9uOihQREZBbm5vdGF0aW9uICopYW5ub3RhdGlv bjsKICNlbmRpZgogCisjaWYgX19NQUNfT1NfWF9WRVJTSU9OX01JTl9SRVFVSVJFRCA+PSAxMDE0 MDAKKy0gKHZvaWQpc2V0RGV2aWNlQ29sb3JTcGFjZTooQ0dDb2xvclNwYWNlUmVmKWNvbG9yU3Bh Y2U7CisjZW5kaWYKIEBlbmQKIAogI2lmIF9fTUFDX09TX1hfVkVSU0lPTl9NSU5fUkVRVUlSRUQg Pj0gMTAxMzAwCkluZGV4OiBTb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1Z2lucy9QREYvUERG UGx1Z2luLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVnaW5z L1BERi9QREZQbHVnaW4ubW0JKHJldmlzaW9uIDIzMTUxMCkKKysrIFNvdXJjZS9XZWJLaXQvV2Vi UHJvY2Vzcy9QbHVnaW5zL1BERi9QREZQbHVnaW4ubW0JKHdvcmtpbmcgY29weSkKQEAgLTc0LDYg Kzc0LDcgQEAKICNpbXBvcnQgPFdlYkNvcmUvUERGRG9jdW1lbnRJbWFnZS5oPgogI2ltcG9ydCA8 V2ViQ29yZS9QYWdlLmg+CiAjaW1wb3J0IDxXZWJDb3JlL1Bhc3RlYm9hcmQuaD4KKyNpbXBvcnQg PFdlYkNvcmUvUGxhdGZvcm1TY3JlZW4uaD4KICNpbXBvcnQgPFdlYkNvcmUvUGx1Z2luRGF0YS5o PgogI2ltcG9ydCA8V2ViQ29yZS9QbHVnaW5Eb2N1bWVudC5oPgogI2ltcG9ydCA8V2ViQ29yZS9S ZW5kZXJCb3hNb2RlbE9iamVjdC5oPgpAQCAtNjMyLDYgKzYzMywxMiBAQCBpbmxpbmUgUERGUGx1 Z2luOjpQREZQbHVnaW4oV2ViRnJhbWUmIGZyCiAKICAgICBbbV9jb250YWluZXJMYXllciBhZGRT dWJsYXllcjptX2NvbnRlbnRMYXllci5nZXQoKV07CiAgICAgW21fY29udGFpbmVyTGF5ZXIgYWRk U3VibGF5ZXI6bV9zY3JvbGxDb3JuZXJMYXllci5nZXQoKV07CisjaWYgX19NQUNfT1NfWF9WRVJT SU9OX01JTl9SRVFVSVJFRCA+PSAxMDE0MDAKKyAgICBpZiAoW21fcGRmTGF5ZXJDb250cm9sbGVy IHJlc3BvbmRzVG9TZWxlY3RvcjpAc2VsZWN0b3Ioc2V0RGV2aWNlQ29sb3JTcGFjZTopXSkgewor ICAgICAgICBhdXRvIHZpZXcgPSB3ZWJGcmFtZSgpLT5jb3JlRnJhbWUoKS0+dmlldygpOworICAg ICAgICBbbV9wZGZMYXllckNvbnRyb2xsZXIgc2V0RGV2aWNlQ29sb3JTcGFjZTpzY3JlZW5Db2xv clNwYWNlKHZpZXcpXTsKKyAgICB9CisjZW5kaWYKIH0KIAogUERGUGx1Z2luOjp+UERGUGx1Z2lu KCkK