Bug 31183 – [Qt][Mac] flash plugin doesn't work when using QGraphicsWebView.

• Component: Plug-ins
• Status: RESOLVED FIXED

[Yongjun Zhang]

Flash plugin doesn't show any content in QGVLauncher, which uses QGraphcisWebView.  Audio still works; the page shows a blank placeholder in where the plugin suppose to be displayed.

[Yongjun Zhang]

The reason is CGContextRef obtained from QGraphicsView is null in function widget->macCGHandle(), if widget is of class type QGraphicsView.

Although it seems to be a Qt bug, Simon suggested to make a dedicated QPixmap and let the plugin paint on the pixmap, just like how windowless plugin is implemented.

[Yongjun Zhang]

Created attachment 42593 [details]
first attempt to fix this issue - use a QPixmap to paint plugin content.

[Holger Freyther]

You should initialize the contextRef from the constructor in the initializer list...

[Kenneth Rohde Christiansen]

Also,

+#if PLATFORM(QT)
+    CGContextRelease(m_contextRef);
+#endif

Here you dont check if m_contextRef is non-0, which you do elsewhere. If you need to do that, you should do it both places, or not do it at all.

Also, this patch seems to clutter the code a bit. Maybe try to refactor it into some methods with descriptive names.

[Girish Ramakrishnan]

Is the patch supposed to work? Crashes for me after applying on Qt 4.6 with cocoa build.

[Yongjun Zhang]

(In reply to comment #5)
> Is the patch supposed to work? Crashes for me after applying on Qt 4.6 with
> cocoa build.

It works for me.  could you paste the crash call stack?

[Girish Ramakrishnan]

(In reply to comment #6)
> (In reply to comment #5)
> > Is the patch supposed to work? Crashes for me after applying on Qt 4.6 with
> > cocoa build.
> 
> It works for me.  could you paste the crash call stack?

Ok, the crash happens only in 32-bit cocoa builds. Your patch works fine in carbon build. Are plugins in 32-bit cocoa supported? It looks like things like NP_NO_CARBON assume that cocoa is 64-bit only. I haven't looked into the problem deeply.

[Girish Ramakrishnan]

Wondeful patch!

Comments:
1. I don't agree with using the QPixmap even when we have a platformWidget(). With this patch, QWebView will triple buffer. We should use QPixmap only when we don't have a view.
2. The pixmap must be resized in updatePlugin(). This is the call back we get when the frame rect changes. Also, you must call initialize with a fill().
3. In transparent mode, the pixmap is not cleared with transparency. You will see that the ball leaves trailing marks in http://www.communitymx.com/content/source/E5141/wmodetrans.htm.
4. npWindow.type must be NPWindowTypeDrawable
5. Doesn't work there is no QWebPageClient. For example, taking screenshot of a QWebPage.

[Girish Ramakrishnan]

(In reply to comment #1)
> The reason is CGContextRef obtained from QGraphicsView is null in function
> widget->macCGHandle(), if widget is of class type QGraphicsView.
> 

This is not a Qt bug. QGraphicsView is a scroll area, you can only get the handle for the viewport() when painting.

[Girish Ramakrishnan]

Created attachment 43251 [details]
Implement plugin rendering

Here's my take. I was working on this simultaneously for a customer, I have merged my patch and  Yongjun Zhang's patch.

The idea is that when we don't have a QWebView as the pluginParent, we set the platformPluginWidget to 0. IOW, have a null platformPluginWidget() indicates to the code that we have to draw to a pixmap. I also prefer that plugins draw directly to the view's contextref when we have a native handle available.

A really ugly hack I had to do was to create a fake window to please Flash. Chrome does this too - http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/plugins/webplugin_delegate_impl_mac.mm?view=markup&pathrev=25433 (grep for fake).

What's missing: mouse and (keyboard? - i can test this only after i can give it focus :D) events don't work, printing doesn't work.

Can someone explain to me how mouse events are sent? Am I reading it right that we send the global pos(!). If so, having to make it work with QGraphicsView presents quite a challenge.

[Girish Ramakrishnan]

Created attachment 43252 [details]
Implement plugin rendering (2)

Only coding style fixes (the if statement had a brace though it was only one line)

[Kenneth Rohde Christiansen]

We seem to be getting a lot of Qt specific code in PluginViewMac. Is that really where they belong? or is it time to make a PluginViewMacQt ? Just wondering.

-        if (QWidget* widget = client->ownerWidget()) {
+        if (QWidget* widget = qobject_cast<QWidget*>(client->pluginParent()))

is ownerWidget not needed anymore? or only for linux?

[Girish Ramakrishnan]

(In reply to comment #12)
> We seem to be getting a lot of Qt specific code in PluginViewMac. Is that
> really where they belong? or is it time to make a PluginViewMacQt ? Just
> wondering.
> 

Right now it's doing ok. But yes, a few more Qt specific changes and we need to start thinking of the above.

> -        if (QWidget* widget = client->ownerWidget()) {
> +        if (QWidget* widget = qobject_cast<QWidget*>(client->pluginParent()))
> 
> is ownerWidget not needed anymore? or only for linux?

We still need it to position popups. But yes, ideally, the NPAPI should be modified so that we don't need to have such a thing as ownerWidget().

[Girish Ramakrishnan]

There's a problem with relying on platformPluginWidget(). For cases like,
m_page->mainFrame()->render(&pixmap) and the page is associated with a QWebView the plugin ends drawing to the view instead of the pixmap because we use the context of the platformPluginWidget(). Solution is to use yongjun's initial approach of redirecting painting of plugin to a pixmap in all cases.

Please reject this patch. I will post a new one.

[Girish Ramakrishnan]

Comment on attachment 43252 [details]
Implement plugin rendering (2)

Rejecting my own patch :)

[Girish Ramakrishnan]

Created attachment 43280 [details]
Implement plugin rendering (3)

[Girish Ramakrishnan]

Attached patch fixes:
1. Printing
2. Plugins use offscreen rendering in all cases.

Only events remains to be fixed.

[Girish Ramakrishnan]

Sigh, I should have checked before r?. There is a rather sever performance regression with QWebView. When playing flash, the cpu usage is 90%. Without using the pixmap, it is like 40-60%. I am not too happy about this. Should we just go back to my old patch?

To make my patch work with printing, we have to pass the correct CGContextRef based on the paintDevice (we have to get this from the qpainter)

[Girish Ramakrishnan]

Comment on attachment 43280 [details]
Implement plugin rendering (3)

Had a discussion with Tor Arne on irc:
1. Decided not to use off screen pixmap in all cases since performance regression not acceptable.
2. Make the contextref code x-platform. Only the QPixmap code will be ifdef'ed Qt.

Also, I have managed to get events working. But I will make that a separate patch.

To get printing and rendering to image to work, we have to probe painter->device(). I don't plan to implement this bit atm since it's not needed for my customer.

[Tor Arne Vestbø]

Comment on attachment 43280 [details]
Implement plugin rendering (3)

r- as per the earlier comment about doing this in a x-port way and abstracting the QPixmap stuff away in helper functions

> +#if defined(XP_MACOSX)
> +#include <QPixmap>
> +#endif

This needs to be wrapped in a PLATFORM(QT) btw

[Girish Ramakrishnan]

It was really bugging me as to _why_ we painting to pixmap is slow. I found the
root cause for the performance problem. 

+    IntRect r(rect->left, rect->top, rect->right + rect->left, rect->bottom +
rect->top);
+    invalidateRect(r);

Spot the bug :-) 

Fixing that makes QtLauncher and QGVLauncher have pretty much the same
performance.

[Simon Hausmann]

(In reply to comment #21)
> It was really bugging me as to _why_ we painting to pixmap is slow. I found the
> root cause for the performance problem. 
> 
> +    IntRect r(rect->left, rect->top, rect->right + rect->left, rect->bottom +
> rect->top);
> +    invalidateRect(r);
> 
> Spot the bug :-) 
> 
> Fixing that makes QtLauncher and QGVLauncher have pretty much the same
> performance.

Fantastic :)

[Girish Ramakrishnan]

Created attachment 43414 [details]
Implement Plugin Rendering (4)

Ok, I imagined events working correctly. Mouse clicks work but not mouse over. I am still figuring out how to fix it. In the meantime, can we get the rendering part in?

[Girish Ramakrishnan]

There is a missing ifdef around this. Any other comments?

+            m_pixmap = QPixmap(m_windowRect.size());
+            m_pixmap.fill(Qt::transparent);
+            m_contextRef = qt_mac_cg_context(&m_pixmap);

[Girish Ramakrishnan]

Created attachment 43502 [details]
Implement Plugin Rendering (5)

Latest and greatest patch with the ifdef changes in my previous comment.

Mouse move just does not get processed by Flash. Incredibly frustrating stuff... I would still like to get this patch in since rendering works great.

[Kenneth Rohde Christiansen]

(In reply to comment #25)
> Created an attachment (id=43502) [details]
> Implement Plugin Rendering (5)
> 
> Latest and greatest patch with the ifdef changes in my previous comment.
> 
> Mouse move just does not get processed by Flash. Incredibly frustrating
> stuff... I would still like to get this patch in since rendering works great.

Let me try to ping Tor Arne

[Simon Hausmann]

Comment on attachment 43502 [details]
Implement Plugin Rendering (5)

This is a great first step

[Girish Ramakrishnan]

Landed in r51234.

[Eric Seidel (no email)]

Closing based on comment above.

[Girish Ramakrishnan]

Opened 31794 for the events (I cannot change the status of this bug)

[Simon Hausmann]

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