49743 - Mac plugins must not be double buffered

Status: VERIFIED FIXED

(Core Graveyard :: GFX, defect, P1)

Description

[Patrick C. Beard]

The drawing model for plugins on the Macintosh only supports on-screen drawing. 
Unfortunately, this is in conflict with the rendering model of the view manager, 
which either does everything double buffered, or nothing. We need a way to 
specify to the view manager, that a [frames, view, widget] need to be rendered 
directly to screen. This could be as simple as setting an additional flag on the 
view/widget corresponding to a plugin that draws everything onscreen.

Comment 1

[Peter Lubczynski]

adding myself to the cc list

Comment 2

[Mike Pinkerton (not reading bugmail)]

adding keywords, nominating, as this is a blocker for another nsbeta3+ bug

Comment 3

[Kevin McCluskey (gone)]

Marking nsbeta3+, P1 since it blocks bug 32327 which is nsbeta3+, P1.

Comment 4

[Kevin McCluskey (gone)]

Attachment: proposed patch to viewmanager2 which allows views to specify whether they are double-buffered

Comment 5

[Kevin McCluskey (gone)]

The patch that I have attached allows you to specify a new view flag, 
NS_VIEW_PUBLIC_FLAG_DONT_DOUBLEBUFFER.

If this flag is set on a view and the view has a widget which receives a PAINT 
message the paint message will be rendered directly to the window.

If this does not work for Mac plugins try setting the flag on the root view of 
the manager.

If this doesn't work, As a last resort, we could specify a global dont_double 
buffer flag that would get set when a plugin was encountered. 

Comment 6

[Kevin McCluskey (gone)]

To set the don't double buffer flag:

nsIView* pluginView;
...
pluginView->SetViewFlags(NS_VIEW_PUBLIC_FLAG_DONT_DOUBLEBUFFER)


If this doesn't work, try setting it on the root view by doing the following:

nsCOMPtr<nsIViewManager> vm;
pluginView->GetViewManager(*getter_AddRefs(vm));
nsIView* rootView;
vm->GetRootView(&rootView);
rootView->SetViewFlags(NS_VIEW_PUBLIC_FLAG_DONT_DOUBLEBUFFER);


I would assume that you need to do this code in nsObjectFrame.cpp in the

nsPluginInstanceOwner::CreateWidget(void) method.

Comment 7

[Paul Chen]

applied the patch, turned off double buffering for both view and view root,
doesn't correct problem

Comment 8

[Phil Peterson]

PDT agrees P1, especially since it blocks 32327

Comment 9

[Kevin McCluskey (gone)]

Attachment: Patch to add an AllowDoubleBuffering method to the viewmanager

Comment 10

[Kevin McCluskey (gone)]

The patch I just added adds a new method AllowDoubleBuffering to the
ViewManager. Just pass PR_FALSE to disable double-buffering for the viewmanager
associated with the the plug-ins view.

Comment 11

[Kevin McCluskey (gone)]

Attachment: Better patch - Allows ViewManager double-buffering to be turned off

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Patch looks good. a=roc

By the way, if I understand the bug correctly, then 
NS_VIEW_PUBLIC_FLAG_DONT_DOUBLEBUFFER should implementable in the following way: 
to solve the bug:
As we render display list elements, if we find one that has 
NS_VIEW_PUBLIC_FLAG_DONT_DOUBLEBUFFER, then we blit the current buffer contents 
to that view's on-screen area, paint the view onto the screen over that area, 
and then copy the resulting bits back from the screen to the buffer. (Skip the 
first blit if Mac plugins are always opaque.) Maybe that would a cooler 
long-term solution; I think it presents a nicer interface.

Comment 13

[Jason Eager]

This is a patch that has an approval, I don't know if it has a review or not.
It's been marked P1 by the PDT. Adding keywords to signify a patch and asking
for one more review so this can get checked in and stop mac suckage.

Comment 14

[Kevin McCluskey (gone)]

Attachment: Patch file with only the code to add AllowDoubleBuffering method (clear backbuffer code removed).

Comment 15

[Kevin McCluskey (gone)]

[e-mail] has reviewed the patch. 

Comment 16

[Kevin McCluskey (gone)]

Fix checked in on the trunk. Waiting for clearance to checkin on the branch.

Comment 17

[Simon Fraser [no longer active]]

Am I to understand that this fix turns off double buffering for any pages on 
which there are <object> tags, or something similar?

Comment 18

[Kevin McCluskey (gone)]

The fix for this bug adds the capability to turn off double-buffering on 
viewmanger instance basis. [e-mail] is working on the code in the 
embed and object frames which will turn of the double-buffering for the Mac. 
This will cause the content window which holds the plugin to draw directly to 
the screen.  If the plugin is inside an IFRAME only the IFRAME's content will be 
drawn directly to the screen. The rest of the page will be double-buffered.

Comment 19

[Simon Fraser [no longer active]]

OK. What I want to avoid is a hack that just disables double buffering for the 
entire page when an <object>, <applet> or <embed> tag is encountered; that would 
cause unacceptable flicking on a large number of pages.

Is peterl's work targeted for PR3 or RTM?

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Kevin, how do you feel about the idea of eventually disabling double-buffering 
on a per-view basis? It would be nice to be able to localise the need to disable 
double buffering, expecially because I would (eventually) like to try having 
subdocuments share the same view manager as their parent document, to allow 
z-indexed and transparent IFRAMEs, and to reduce bloat.

Comment 21

[Kevin McCluskey (gone)]

Were trying to get it in for PR3 but we still need ++ approval.

"OK. What I want to avoid is a hack that just disables double buffering for the 
entire page when an <object>, <applet> or <embed> tag is encountered;"

Well, depending on how the document is structured it may flicker. If the 
plugin/applet/embed is inside a frameset document and isolated to a frame or the 
plugin is inside a Iframe there shouldn't be any flicker, but if the content 
area is being managed by a single viewmanager then there will be extra flicker 
in the content area.  

Robert: I think your idea about turning off double buffering on a per view 
instance basic is interesting, but I'm not sure about copying from the onscreen 
window to the backbuffer. When we paint the plugin we simply send it a plugin 
paint notification, since we don't have a renderingcontext there isn't any way 
for us to blit from onscreen window to the backbuffer. 

We could copy the backbuffer to the onscreen window then render the non-doubled 
buffered view on top of it, this would elminate the flicker for the content area 
around the plugins, and this should be sufficient.  It has the restriction that 
the plugins must always be rendered on top of the other views. 

If we want the plugin's view to be support transparency and Z-index, perhaps we 
should enhance the Mac plugin API to allow us to render to an offscreen drawing 
surface.

Comment 22

[Simon Fraser [no longer active]]

I seem to recall that porkjockeys punted on Z-ordering with plugins a long time 
ago, so we need not consider that.

I think a good fix for this bug would be to have an API on nsIView that specifies 
that an individual view should not be double buffered. Then, for pages with any 
such views, page drawing would be 2-pass:
1. Draw normal views into the back buffer
2. Blit the back buffer to the screen
3. Draw non-buffered views to the screen

Would this be hard to implement?

I don't think it's feasible to change the mac plugin API to deal with double 
buffering, for 2 reasons:
i) plugins usually draw animation, and expect to draw directly, without going
   through an invalidate/update cycle. It is not possible to double-buffer in
   this situation without enhancements to the plugin API that allow the plugin
   to say that it just did some drawing that needs blitting to the screen.
ii) such changes would break backward compatibility big-time.

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm pretty sure the Mac gives you a way to grab the bits from the screen, but 
you're right that it's probably not currently accessible through GFX.

But if we could flag views as non-double-bufferable, then there are actually a 
variety of ways to use the information. It would be easy to scan the display 
list to see whether there are any DB-disabling views actually being rendered, 
and if so, disable DB only for that particular repaint operation. This would 
avoid flicker when we're painting content in the same page as a plugin that does 
not overlap with the plugin.

With just a little more work, we could do something along the lines of what 
Simon just suggested, or perhaps even better.

I don't understand this comment though:
> plugins usually draw animation, and expect to draw directly, without going
> through an invalidate/update cycle. It is not possible to double-buffer in
> this situation without enhancements to the plugin API that allow the plugin
> to say that it just did some drawing that needs blitting to the screen.
If a plugin is drawing at some time other than during a view manager 
orchestrated repaint in response to an NS_PAINT event, then it has no access to 
the view manager's double-buffering mechanisms and must necessarily be drawing 
directly to the screen.

Comment 24

[Kevin McCluskey (gone)]

> plugins usually draw animation, and expect to draw directly, without going
> through an invalidate/update cycle. It is not possible to double-buffer in
> this situation without enhancements to the plugin API that allow the plugin
> to say that it just did some drawing that needs blitting to the screen.

I think Simon is pointing out that the Mac plugin does not invalidate the window 
when it wants to paint. It just paints directly to the window, so the view 
manager doesn't always have a chance to manage the z-order or translucency. The 
only time the view manager has a chance to manage the painting is in reponse to 
an invalidate from the layout engine or an expose event from the window manager. 
If the plugin is doing an animation off of a timer then it will paint the window 
itself without any intervention from the view manager.

I think marking the View as non-double-buffered would be a good long term 
solution, but It's still handy to be able to turn off the double-buffering at 
the viewmanager level.

The simple marking of the view and copying forward from the backbuffer when the 
non-doubled buffered view is encountered in the display list is not a huge 
change, but It's big enough that it will not make it for PR3. 

Comment 25

[Michael La Guardia]

Per Kevin's last comment, marking nsbeta3-.  The bug is already nominated for rtm.

Comment 26

[Michael La Guardia]

oops really marking - this time

Comment 27

[ekrock's old account (dead)]

URGENT: PLEASE MARK THIS [nsbeta3++] AND CHECK IN AS IT IS BLOCKING BUG 32327, 
which is an approved [nsbeta3++]. Escalating Severity to blocker. Clearing 
[nsbeta3-] to trigger re-evaluation.

Comment 28

[Simon Fraser [no longer active]]

Proposed fixes to the view manager and nsObjectFrame.cpp look good, with teh 
following comments:

nsViewManager.cpp./.h is obsolete, so no changes are necessary there.
nsViewManager2.h: preferable to use PRPackedBool, rather than PRBool, for boolean 
members (8 vs. 32 bits).

nsObjectFrame.cpp: is this a leak:
+   nsCOMPtr<nsIWidget> widget = aWidget->GetParent(); ?
and
+     widget = widget->GetParent(); 
I think you may need nsCOMPtr<nsIWidget> widget = getter_AddRefs(aWidget->
GetParent()); 

For good usage, see
http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacEventHandler.cpp#446

With these patches , plugins look somewhat better. I had problems viewing 
QuickTime movies, but flash animations showed up, and didn't nuke subsequent 
windows.

I think we need to relnote the fact that plugins are still in progress on Mac.

Comment 29

[Peter Lubczynski]

Patch checked into the branch. Clearing status whiteboard.

Kevin is still thinking of a better way to handle the Mac plugin double 
buffer/painting/clipping issues so I'm not marking this FIXED quite yet.

Comment 30

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Just a note: nsViewManager is NOT obsolete. I've revived it. In fact, 
nsViewManager2 is obsolete and will hopefully be replaced by nsViewManager in 
due course.

Comment 31

[Kevin McCluskey (gone)]

Marking nsbeta3- since we are past nsbeta3

Comment 32

[Peter Lubczynski]

closing out this bug because the feature for turning off double buffering has 
been checked in

However, plugins are still having problems. Please continue discussions on Mac 
plugin paint problems in bug 54962.

Comment 33

[Matthias Versen [:Matti]]

mid-air collision ? / bugzilla cleanup
Reopening (current State: resolved and no resolution

Comment 34

[Matthias Versen [:Matti]]

fixed

Comment 35

[Chris Petersen]

Marking verified per last comments.