Summary: | [Qt] Flash does not work on n900 | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Girish Ramakrishnan <girish> | ||||||||||||||||||||
Component: | Plug-ins | Assignee: | Girish Ramakrishnan <girish> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | ademar, ariya.hidayat, hausmann, kenneth, kling, tonikitoo, webkit.review.bot | ||||||||||||||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | Other | ||||||||||||||||||||||
OS: | Other | ||||||||||||||||||||||
Attachments: |
|
Description
Girish Ramakrishnan
2010-08-15 22:51:01 PDT
Created attachment 64482 [details]
wmode=opaque fix
This fixes the wmode=opaque case. Flash on n900 is version 9.x
XEmbed - It appears the Maemo5/Flash doesn't support embedded mode. I would like to be proven wrong. Transparent mode - Maemo5/Flash is broken and it thinks that the XPixmap that we pass it is a shared memory image! X Error: BadMatch (invalid parameter attributes) 8 Extension: 137 (MIT-SHM) Minor opcode: 4 (X_ShmGetImage) Resource id: 0x3a0001f 0x3a0001f is actually the X Pixmap handle that we pass to Flash. It renders ok with some artifacts. We are loading the wrong gdk library on the n900, I will add a patch for this. BTW, the reason fennec is unaffected is because it uses https://wiki.mozilla.org/Plugins:NokiaMaemoImageSurface (which I intend to implement for Qt/WebKit) Created attachment 64504 [details]
libgdk-x11-2.0.so.0
On maemo5, libgdk-x11-2.0.so is missing. Look for libgdk-x11-2.0.so.0 instead.
Comment on attachment 64504 [details] libgdk-x11-2.0.so.0 >WebCore/plugins/qt/PluginViewQt.cpp:665 > + #ifdef Q_WS_MAEMO_5 Any reason we can't use "libgdk-x11-2.0.so.0" for all platforms? (In reply to comment #5) > (From update of attachment 64504 [details]) > >WebCore/plugins/qt/PluginViewQt.cpp:665 > > + #ifdef Q_WS_MAEMO_5 > Any reason we can't use "libgdk-x11-2.0.so.0" for all platforms? Nope, I think that's a good idea. Flash in fact links to libgdk-x11-2.0.so.0 so it is probably more correct to look for that instead of libgdk-x11-2.0.so. wmode=opaque patch landed in 65524 Created attachment 64614 [details]
libgdk-x11-2.0.so.0
Look for libgdk-x11-2.0.so.0 on all platforms.
Comment on attachment 64614 [details]
libgdk-x11-2.0.so.0
r=me
libgdk-x11-2.0.so.0 landed in r65586. hence fixed? :) Initial version of local rendering is ready - https://wiki.mozilla.org/Plugins:NokiaMaemoImageSurface. The current implementation is simple - Create a QImage. Give QImage for the plugin to render. I would like to commit the fix as a series of patches. The first patch will contain a working implementation of the spec. Everything else that follows will just be optimization fixes. Currently, everything works but there are a few details that I need to double check with the Nokia folks before this is ready for commit. The code is at http://gitorious.org/~girish/webkit/girishs-webkit/commits/maemo5_local_rendering_44043. The patch is made specifically for Maemo5 but it's not ifdef'ed with Q_WS_MAEMO_5 intentionally. It just makes the whole code a mess. If you object to this, please let me know. My TODO list: 1. Get the image format correct. * Transparent (32-bit), Opaque (16-bit) - Check if Flash can render 32-bit in Opaque mode and 16-bit in transparent mode. 2. Transparency * Is it expecting premultiplied ARGB? * If I clear the image with Qt::transparent, the plugin seems to be painting it all black. 3. Test in raster and opengl * Works OK. 63% CPU with raster for X.org and Qt/WebKit combined. 4. Test if fallback is correct to pixmap mode. * This code path only works for opaque mode (see comment 2). Disable it or leave it broken? 5. Should we be using shared memory instead of QImage? * When using the X11 native backend, creating the image using MIT-SHM has the benefit that we can use XShmPutImage. 6. Should we do direct rendering to backing store? * With raster graphicssystem, maybe we can render straight into the backing store? This would give the _best_ performance. AFAIK, Qt will always create a 16-bit surface. So, need to check if transparent Flash can be drawn on 16-bit. 7. Force to windowless mode even for QWebView * See comment 2. XEmbed seems broken. Only windowless mode works. Maybe we can put the hack we use for QGraphicsWebView for QWebView also. (In reply to comment #12) > Initial version of local rendering is ready - https://wiki.mozilla.org/Plugins:NokiaMaemoImageSurface. > > The current implementation is simple - Create a QImage. Give QImage for the plugin to render. > > I would like to commit the fix as a series of patches. The first patch will contain a working implementation of the spec. Everything else that follows will just be optimization fixes. I'm fine with that :-) Please attach patches. > The patch is made specifically for Maemo5 but it's not ifdef'ed with Q_WS_MAEMO_5 intentionally. It just makes the whole code a mess. If you object to this, please let me know. Also fine with me :-) Created attachment 64710 [details]
localrendering
Created attachment 64711 [details]
localrendering
localrendering landed in r65612. Created attachment 64835 [details]
Inject wmode=opaque in QWebView in maemo5
Last patch for the n900.
Attachment 64835 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1466: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 64835 [details]
Inject wmode=opaque in QWebView in maemo5
1465 #if defined(MOZ_PLATFORM_MAEMO) && (MOZ_PLATFORM_MAEMO == 5)
1466 true ||
1467 #endif
1468 !client || !qobject_cast<QWidget*>(client->pluginParent())) {
Wouldnt this be better as
true) {
#else
!client || !qobject_cast<QWidget*>(client->pluginParent())) {
#endif
Created attachment 64844 [details]
Inject wmode=opaque in QWebView in maemo5
Fixed style errors and commit message
Inject wmode=opaque landed in r65668 Created attachment 64994 [details]
Direct local rendering
When using the raster graphicssystem (which is the default Qt graphicssystem on maemo5), allow flash to draw directly to the raster surface.
Reopening, since I have a couple of optimization patches. And it's easier to cherry-pick for PE1. Comment on attachment 64994 [details]
Direct local rendering
WebCore/plugins/qt/PluginViewQt.cpp:200
+ imagePainter.fillRect(exposedRect, Qt::white);
Please use m_image.fill(Qt::white) instead.
Otherwise LGTM, re=me.
Landed in r65775. (In reply to comment #24) > (From update of attachment 64994 [details]) > WebCore/plugins/qt/PluginViewQt.cpp:200 > + imagePainter.fillRect(exposedRect, Qt::white); > Please use m_image.fill(Qt::white) instead. > Ariya, I cannot use QImage::fill() to clear a portion of the QImage, right? Or are you suggesting something like: if (exposedRect == m_image.rect()) { m_image.fill(); } else { // create QPainter and fillRect } I went ahead and committed it anyway, that code was not introduced in this patch. Created attachment 65022 [details] Allow wmode=transparent in QWebView on Maemo5 r65775 adds wmode=transparent support. > if (exposedRect == m_image.rect()) {
> m_image.fill();
> } else {
> // create QPainter and fillRect
> }
Yes, unless you think exposedRect == m_image.rect() is not a common case.
Also, when filing with QPainter, when you know the destination image should be opaque, use source composition mode.
Comment on attachment 65022 [details]
Allow wmode=transparent in QWebView on Maemo5
LGTM.
(In reply to comment #28) > > if (exposedRect == m_image.rect()) { > > m_image.fill(); > > } else { > > // create QPainter and fillRect > > } > > Yes, unless you think exposedRect == m_image.rect() is not a common case. > Yes, it's not the common case. > Also, when filing with QPainter, when you know the destination image should be opaque, use source composition mode. AFAIK, Source has the same effect as SourceOver (default) when the source pixel is opaque (Qt::white in this case). Is there some implementation detail I should know about? Also, m_image is always 16-bit, so does setting composition mode Source matter here?
> AFAIK, Source has the same effect as SourceOver (default) when the source pixel is opaque
> (Qt::white in this case). Is there some implementation detail I should know about?
I reread the patch again and I realize now I was wrong. You're right, leave it as it is :)
Thanks ariya! Landed in r65796. Revision r65524 cherry-picked into qtwebkit-2.1 with commit 8ba974ee9f11c0dceef3cd1905ccfde3e00d4396 Revision r65586 cherry-picked into qtwebkit-2.1 with commit 62187e66f76f3fd914a124b0a83392a087e6b9e5 Revision r65612 cherry-picked into qtwebkit-2.1 with commit 2f4975cb871858c834a0b1d14f5ded437f0d7758 Revision r65668 cherry-picked into qtwebkit-2.1 with commit c18140525797adc8da944aa57615b2364a48a4aa Revision r65775 cherry-picked into qtwebkit-2.1 with commit a5841cad362505f9df286e2cd17af624ed2ec666 Revision r65796 cherry-picked into qtwebkit-2.1 with commit 2908706c089e66b5fcd1e0f004c5ade32efadb78 |