98518 2012-10-05 07:18:12 -0700 [Qt][WK2] Plugins are completely broken with a custom device pixel ratio 2012-10-29 07:15:01 -0700 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 98528 1 kbalazs kbalazs abecsi cmarcelo jesus jturcotte menard webkit.review.bot zoltan oldest_to_newest 735620 0 kbalazs 2012-10-05 07:18:12 -0700 Currently this is the case in MiniBrowser because it defines 1.5 as device pixel ratio. My goal currently is just to handle this case. We will probably have to return to this if we want to support plugins not just on the desktop. 735627 1 167326 kbalazs 2012-10-05 07:28:10 -0700 Created attachment 167326 Patch 735670 2 167326 kenneth 2012-10-05 08:56:26 -0700 Comment on attachment 167326 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167326&action=review > Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp:91 > - // See <https://bugs.webkit.org/show_bug.cgi?id=64663>. > - notImplemented(); > + QImage image = createQImage(); > + QPainter* painter = context.platformContext(); > + > + painter->save(); > + painter->scale(scaleFactor, scaleFactor); > + painter->drawImage(dstPoint, image, QRect(srcRect)); > + painter->restore(); why not do a fast path for scaleFactor == 1.0? 735698 3 kbalazs 2012-10-05 09:26:41 -0700 (In reply to comment #2) > (From update of attachment 167326 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167326&action=review > > > Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp:91 > > - // See <https://bugs.webkit.org/show_bug.cgi?id=64663>. > > - notImplemented(); > > + QImage image = createQImage(); > > + QPainter* painter = context.platformContext(); > > + > > + painter->save(); > > + painter->scale(scaleFactor, scaleFactor); > > + painter->drawImage(dstPoint, image, QRect(srcRect)); > > + painter->restore(); > > why not do a fast path for scaleFactor == 1.0? It's already there :) (above the changed lines) 735704 4 167326 kenneth 2012-10-05 09:29:54 -0700 Comment on attachment 167326 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167326&action=review >>> Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp:91 >>> + painter->restore(); >> >> why not do a fast path for scaleFactor == 1.0? > > It's already there :) (above the changed lines) Is it possible somehow to use the GraphicsContext3D to use the hardware for scaling? iguess you are basically scaling an image right? 735716 5 kbalazs 2012-10-05 09:40:02 -0700 (In reply to comment #4) > (From update of attachment 167326 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167326&action=review > > >>> Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp:91 > >>> + painter->restore(); > >> > >> why not do a fast path for scaleFactor == 1.0? > > > > It's already there :) (above the changed lines) > > Is it possible somehow to use the GraphicsContext3D to use the hardware for scaling? iguess you are basically scaling an image right? Yep, but we do software rendering here by design. The backing store is a shared memory buffer. Probably we could utilize ShareableSurface to make this accelerated. 735724 6 167326 kbalazs 2012-10-05 09:51:06 -0700 Comment on attachment 167326 Patch Clearing flags on attachment: 167326 Committed r130519: <http://trac.webkit.org/changeset/130519> 735725 7 kbalazs 2012-10-05 09:51:11 -0700 All reviewed patches have been landed. Closing bug. 738939 8 abecsi 2012-10-10 07:21:23 -0700 Since r130630 applies the device pixel ratio as a UI-side scale in the desktop webview plugins seem to be scaled twice, this issue seems to be fixes if reverting this change. Can you check, and confirm this? 741504 9 kbalazs 2012-10-13 12:07:31 -0700 *** Bug 94821 has been marked as a duplicate of this bug. *** 745258 10 kbalazs 2012-10-18 08:09:31 -0700 (In reply to comment #8) > Since r130630 applies the device pixel ratio as a UI-side scale in the desktop webview plugins seem to be scaled twice, this issue seems to be fixes if reverting this change. > Can you check, and confirm this? First of all, if I simply revert it than we will not paint at all again, so I think you mean we should ignore the scale factor. It seems to be more complicated. Now the plugin is overscaled. Actually now I think that my code is wrong, we should scale the image, not the context (or we can emulate scaling the image with coordinate transform on the context to be faster). However, if I change it like that it doesn't help. If I ignore the scaleFactor at all, it's better but still not perfect. I'm not sure about what is wrong in this case, but the buttons at the bottom of the video don't show up because the video covers that area as well and there are glitches in the video (maybe it's a missing clip or smg). If I change PluginProxy::contentScaleFactor to return constant 1, than it's shown correctly. We should find out what's the difference between us and mac. Actually by reading the code it seems to me that the scale is really applied twice in PluginProxy, so I will take a look at it on Mac. 745263 11 kbalazs 2012-10-18 08:14:54 -0700 Filed bug 9722 for the overscaling. 745276 12 167326 jturcotte 2012-10-18 08:28:14 -0700 Comment on attachment 167326 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167326&action=review > Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp:89 > + painter->scale(scaleFactor, scaleFactor); What if you just don't scale it here? I don't really understand all this code, but it seems like this shareablebitmap is the backing store of the plugin view and it is already scaled by PluginProxy, no? Also, does this mean that the plugin backing store isn't a layer by itself? Does it get re-blitted on every tile of the non-composited content for every update of the plugin? It's unlikely that we'll have a plugin big enough to need tiling, so why not pass the shareablebitmap directly to the ui process like we do for CoordinatedGraphicsLayer::setContentsToImage? Anyway this is just food for thought, if we can just fix the scaling issue I'd be pretty happy too. 745293 13 kbalazs 2012-10-18 08:50:56 -0700 (In reply to comment #12) > (From update of attachment 167326 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167326&action=review > > > Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp:89 > > + painter->scale(scaleFactor, scaleFactor); > > What if you just don't scale it here? This is the case when the panel at the bottom does not shown up. > I don't really understand all this code, but it seems like this shareablebitmap is the backing store of the plugin view and it is already scaled by PluginProxy, no? PluginProxy has an m_pluginBackingStore, this is the shared memory the plugin process paints onto, and m_backingStore which is shared between the UI and the web process. m_pluginBackingStore is getting blitted to m_backingStore in update. For me it also seems to be that the scaleFactor is applied multiple times: first in the plugin process, than when we blit, than when we blit m_backingStore to the graphicsContext in paint. I have no clue how could this work on mac. > Also, does this mean that the plugin backing store isn't a layer by itself? Does it get re-blitted on every tile of the non-composited content for every update of the plugin? I guess this is the case with the tiles covering the plugin but I'm not sure. > > It's unlikely that we'll have a plugin big enough to need tiling, so why not pass the shareablebitmap directly to the ui process like we do for CoordinatedGraphicsLayer::setContentsToImage? Sounds good but I need to investigate more. 746132 14 jturcotte 2012-10-19 01:45:29 -0700 (In reply to comment #13) > I have no clue how could this work on mac. It's possible that they're now always using CA layers in their case (just a guess). 753405 15 kbalazs 2012-10-29 07:15:01 -0700 (In reply to comment #11) > Filed bug 9722 for the overscaling. it's bug 99722 167326 2012-10-05 07:28:10 -0700 2012-10-18 08:28:14 -0700 Patch bug-98518-20121005162712.patch text/plain 1776 kbalazs U3VidmVyc2lvbiBSZXZpc2lvbjogMTMwNTAwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMjA4MzU1NTc2NzdlYzIz NTMxNjhlOWZiYjU5YzNiZWU1MzkyNzM4Mi4uMWU1MjExMjY5ZWUwM2U0MDA2Yzc4NDE1MzIxNzBl ZDcxN2JkYjE5MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEyLTEwLTA1ICBCYWxh enMgS2VsZW1lbiAgPGtiYWxhenNAd2Via2l0Lm9yZz4KKworICAgICAgICBbUXRdW1dLMl0gUGx1 Z2lucyBhcmUgY29tcGxldGVseSBicm9rZW4gd2l0aCBhIGN1c3RvbSBkZXZpY2UgcGl4ZWwgcmF0 aW8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk4NTE4 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW1wbGVt ZW50IHBhaW50aW5nIHdpdGggc2NhbGUgZmFjdG9yIGluIFNoYXJlYWJsZUJpdG1hcC4KKyAgICAg ICAgVGhlIGJhY2tpbmcgc3RvcmUgb2YgdGhlIHBsdWdpbiBhcmUgc3RpbGwgb3ZlcnNjYWxlZCBp biBNaW5pQnJvd3NlcgorICAgICAgICB3aXRoIHRoaXMgcGF0Y2ggYnV0IHRoaXMgaXMgb25seSB0 aGUgZWZmZWN0IG9mIHRoZSBmYWtlIGRldmljZSBzY2FsZQorICAgICAgICBmYWN0b3IgZGVmaW5l ZCBpbiBxbWwgKDEuNSkuIFdlIHNob3VsZCBwcm9iYWJseSByZW1vdmUgaXQgb24gZGVza3RvcC4K KworICAgICAgICAqIFNoYXJlZC9xdC9TaGFyZWFibGVCaXRtYXBRdC5jcHA6CisgICAgICAgIChX ZWJLaXQ6OlNoYXJlYWJsZUJpdG1hcDo6cGFpbnQpOgorCiAyMDEyLTEwLTA1ICBKb25nc2VvayBZ YW5nICA8anM0NS55YW5nQHNhbXN1bmcuY29tPgogCiAgICAgICAgIFtFRkxdW1dLMl0gUmVtb3Zl ICJ3ZWIiIHdvcmQgZnJvbSBld2tfd2ViX2Vycm9yIEFQSXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJLaXQyL1NoYXJlZC9xdC9TaGFyZWFibGVCaXRtYXBRdC5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9T aGFyZWQvcXQvU2hhcmVhYmxlQml0bWFwUXQuY3BwCmluZGV4IDg4ZTRkM2FlYzVmZWQ3YmI0ZTk5 ZDlkM2U0NTU1NjgwODJlYzE0MTAuLjZjYWRiMDg2ZDYzNTZmMTIwNjBkZmQ5ZDk0N2NmNGY2OTkw M2M5MmEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1NoYXJlZC9xdC9TaGFyZWFibGVCaXRt YXBRdC5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvU2hhcmVkL3F0L1NoYXJlYWJsZUJpdG1hcFF0 LmNwcApAQCAtODIsOCArODIsMTMgQEAgdm9pZCBTaGFyZWFibGVCaXRtYXA6OnBhaW50KEdyYXBo aWNzQ29udGV4dCYgY29udGV4dCwgZmxvYXQgc2NhbGVGYWN0b3IsIGNvbnN0IEkKICAgICAgICAg cmV0dXJuOwogICAgIH0KIAotICAgIC8vIFNlZSA8aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTY0NjYzPi4KLSAgICBub3RJbXBsZW1lbnRlZCgpOworICAgIFFJbWFnZSBp bWFnZSA9IGNyZWF0ZVFJbWFnZSgpOworICAgIFFQYWludGVyKiBwYWludGVyID0gY29udGV4dC5w bGF0Zm9ybUNvbnRleHQoKTsKKworICAgIHBhaW50ZXItPnNhdmUoKTsKKyAgICBwYWludGVyLT5z Y2FsZShzY2FsZUZhY3Rvciwgc2NhbGVGYWN0b3IpOworICAgIHBhaW50ZXItPmRyYXdJbWFnZShk c3RQb2ludCwgaW1hZ2UsIFFSZWN0KHNyY1JlY3QpKTsKKyAgICBwYWludGVyLT5yZXN0b3JlKCk7 CiB9CiAKIH0K