Bug 50495 - [Qt] wmode parameter for flash plugins always gets overridden to opaque on QGraphicsWebView
: [Qt] wmode parameter for flash plugins always gets overridden to opaque on QG...
Status: RESOLVED FIXED
: WebKit
Plug-ins
: 528+ (Nightly build)
: All All
: P3 Normal
Assigned To:
:
: Qt, QtTriaged
:
:
  Show dependency treegraph
 
Reported: 2010-12-03 15:20 PST by
Modified: 2011-03-01 20:04 PST (History)


Attachments
Check, that wmode is really "window" and do not override transparent wmode. (1.50 KB, patch)
2010-12-03 16:09 PST, Viatcheslav Ostapenko
no flags Review Patch | Details | Formatted Diff | Diff
Fix style error (1.50 KB, patch)
2010-12-03 16:20 PST, Viatcheslav Ostapenko
vestbo: review+
Review Patch | Details | Formatted Diff | Diff
Extra hunk removed and patch rebased to updated source tree. (1.30 KB, patch)
2011-02-10 10:58 PST, Viatcheslav Ostapenko
no flags Review Patch | Details | Formatted Diff | Diff
Add "reviewed by" (1.30 KB, patch)
2011-02-10 13:49 PST, Viatcheslav Ostapenko
no flags Review Patch | Details | Formatted Diff | Diff
Add bug id to changelog (1.35 KB, patch)
2011-02-10 15:22 PST, Viatcheslav Ostapenko
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-12-03 15:20:25 PST
"wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
------- Comment #1 From 2010-12-03 16:09:10 PST -------
Created an attachment (id=75572) [details]
Check, that wmode is really "window" and do not override transparent wmode.
------- Comment #2 From 2010-12-03 16:11:29 PST -------
Attachment 75572 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/qt/ChangeLog', u'WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp']" exit_code: 1
WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1558:  Missing space before ( in if(  [whitespace/parens] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #3 From 2010-12-03 16:20:57 PST -------
Created an attachment (id=75577) [details]
Fix style error
------- Comment #4 From 2010-12-06 01:33:44 PST -------
(In reply to comment #0)
> "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.

I believe we did this on purpose to always get windowless plugins on linux.
------- Comment #5 From 2010-12-07 07:41:20 PST -------
(In reply to comment #4)
> (In reply to comment #0)
> > "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
> 
> I believe we did this on purpose to always get windowless plugins on linux.

But transparent plugin is also windowless. Of course it can cause some performance overhead for blending, but there are a lot of places with blending in webkit, so I don't think it's real problem.
As I understand, there are 2 options when plugin can get into windowed mode:
1. when wmode is not specified, than plugin use it's own default (windowed for current implementations of flash plugins).
2. wmode=window when page wants plugin to play in the topmost layer above other elements.

My patch fixes only transparent wmode for flash plugin, but leaves it windowless even if page specifies wmode=window.
------- Comment #6 From 2010-12-12 02:25:44 PST -------
I don't understand plugins.  But I know there are people who do. :)
------- Comment #7 From 2010-12-24 00:31:26 PST -------
(In reply to comment #0)
> "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.

The change is intentional. We want to force flash to opaque mode in QGWV in all cases.

Note that we cannot make flash transparency work in QGraphicsWebView because Flash has a bug wherein it does not render to 32-bit pixmaps. Do you have a new version of flash that has fixed this problem or have the latest versions of flash fixed it? Do you see transparency working after your patch?

Flash transparency works only with QWebView wherein I added very hairy code to grab contents from the backing store to fake transparency.
------- Comment #8 From 2010-12-24 06:20:54 PST -------
(From update of attachment 75577 [details])
r- per Girish's comment
------- Comment #9 From 2010-12-24 11:20:53 PST -------
(In reply to comment #7)
> (In reply to comment #0)
> > "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
> The change is intentional. We want to force flash to opaque mode in QGWV in all cases.
> Note that we cannot make flash transparency work in QGraphicsWebView because Flash has a bug wherein it does not render to 32-bit pixmaps. Do you have a new version of flash that has fixed this problem or have the latest versions of flash fixed it? Do you see transparency working after your patch?
> Flash transparency works only with QWebView wherein I added very hairy code to grab contents from the backing store to fake transparency.

The flash code I have just blits from blits from bitmap, so if it has alpha on this bitmap backing store, than transparency would work. At least flash guys had working test cases with transparent flash. Unfortunatelly, I'm not in office now. I'll recheck after holidays.
------- Comment #10 From 2010-12-27 09:44:11 PST -------
(In reply to comment #9)
> (In reply to comment #7)
> > (In reply to comment #0)
> > > "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
> > The change is intentional. We want to force flash to opaque mode in QGWV in all cases.
> > Note that we cannot make flash transparency work in QGraphicsWebView because Flash has a bug wherein it does not render to 32-bit pixmaps. Do you have a new version of flash that has fixed this problem or have the latest versions of flash fixed it? Do you see transparency working after your patch?
> > Flash transparency works only with QWebView wherein I added very hairy code to grab contents from the backing store to fake transparency.
> 
> The flash code I have just blits from blits from bitmap, so if it has alpha on this bitmap backing store, than transparency would work. 

I agree that it will work if we had alpha on the pixmap that Flash would paint to. The problem is that so far all versions of Flash I have seen have not been able to render to transparent/32-bit pixmaps. Also, please do check if the Flash guys actually tested with Qt/WebKit and not a mozilla based browser. Thanks!
------- Comment #11 From 2010-12-27 21:55:49 PST -------
> I agree that it will work if we had alpha on the pixmap that Flash would paint 
> to. The problem is that so far all versions of Flash I have seen have not been > able to render to transparent/32-bit pixmaps. 

So, as I understand, they will produce some non-transparent backing store, which will be blit to qt painter normally and there is no need to override transparent wmode to opaque? This way if transparent rendering in flash plugin will be fixed, than there will be no need to change code again. 

> Also, please do check if the Flash guys actually tested with Qt/WebKit and not 
> a mozilla based browser. 

Actually, I've made this bug by their request (from flash plugin for s60 developers). They have some test cases with transparency and they fail. I'd like to figure it out myself when get back to office.
------- Comment #12 From 2011-01-26 12:00:07 PST -------
> I agree that it will work if we had alpha on the pixmap that Flash would paint 
> to. The problem is that so far all versions of Flash I have seen have not been 
> able to render to transparent/32-bit pixmaps. Also, please do check if the 
> Flash guys actually tested with Qt/WebKit and not a mozilla based browser. 
> Thanks!

Yes, new flash plugin works in transparent mode.
This test page works: http://125.16.213.243/upload/NGFlash10/AS3_FL9/Windowless/as3_opq/01_dynamic_ball.html

Also, if some flash plugin cannot handle transparent mode it can fallback to opaque mode internally. There is no need to restrict it in webkit.
------- Comment #13 From 2011-02-10 08:07:15 PST -------
(From update of attachment 75577 [details])
Current windowless flash plugin implementation supports transparent mode.
If some plugin doesn't support this, it could force to opaque internally.
------- Comment #14 From 2011-02-10 10:41:23 PST -------
(From update of attachment 75577 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=75577&action=review

r=me, but remove the extra hunk

> WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1532
> +

Don't add extra hunks
------- Comment #15 From 2011-02-10 10:58:45 PST -------
Created an attachment (id=82007) [details]
Extra hunk removed and patch rebased to updated source tree.
------- Comment #16 From 2011-02-10 13:49:42 PST -------
Created an attachment (id=82042) [details]
Add "reviewed by"
------- Comment #17 From 2011-02-10 13:51:01 PST -------
(From update of attachment 82042 [details])
Rejecting attachment 82042 [details] from commit-queue.

ostapenko.viatcheslav@nokia.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
------- Comment #18 From 2011-02-10 15:08:12 PST -------
(From update of attachment 82042 [details])
Missing bug id
------- Comment #19 From 2011-02-10 15:22:31 PST -------
Created an attachment (id=82060) [details]
Add bug id to changelog
------- Comment #20 From 2011-02-11 11:35:38 PST -------
The commit-queue encountered the following flaky tests while processing attachment 82060 [details]:

inspector/debugger-breakpoints-not-activated-on-reload.html bug 54300 (author: podivilov@chromium.org)
The commit-queue is continuing to process your patch.
------- Comment #21 From 2011-02-11 11:37:50 PST -------
(From update of attachment 82060 [details])
Clearing flags on attachment: 82060

Committed r78352: <http://trac.webkit.org/changeset/78352>
------- Comment #22 From 2011-02-11 11:37:57 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #23 From 2011-02-11 12:43:26 PST -------
http://trac.webkit.org/changeset/78352 might have broken GTK Linux 64-bit Debug
------- Comment #24 From 2011-02-15 07:26:20 PST -------
Adding to critical master.  This is required for flash.
------- Comment #25 From 2011-02-17 12:44:47 PST -------
Revision r78352 cherry-picked into qtwebkit-2.1.x with commit 9b0c70f <http://gitorious.org/webkit/qtwebkit/commit/9b0c70f>
------- Comment #26 From 2011-03-01 20:04:19 PST -------
*** Bug 39569 has been marked as a duplicate of this bug. ***