Created attachment 167326 [details] Patch

Comment on attachment 167326 [details] 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?

(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)

Comment on attachment 167326 [details] 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?

(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.

Comment on attachment 167326 [details] Patch Clearing flags on attachment: 167326 Committed r130519: <http://trac.webkit.org/changeset/130519>

All reviewed patches have been landed. Closing bug.

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?

*** Bug 94821 has been marked as a duplicate of this bug. ***

(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.

Filed bug 9722 for the overscaling.

Comment on attachment 167326 [details] 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.

(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.

(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).

(In reply to comment #11) > Filed bug 9722 for the overscaling. it's bug 99722