3.47 KB, patch
|Details | Diff | Splinter Review|
5.22 KB, patch
|Details | Diff | Splinter Review|
6.24 KB, patch
|Details | Diff | Splinter Review|
5.42 KB, patch
|Details | Diff | Splinter Review|
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.
adding myself to the cc list
adding keywords, nominating, as this is a blocker for another nsbeta3+ bug
Marking nsbeta3+, P1 since it blocks bug 32327 which is nsbeta3+, P1.
Created attachment 14753 [details] [diff] [review] proposed patch to viewmanager2 which allows views to specify whether they are double-buffered
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.
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.
applied the patch, turned off double buffering for both view and view root, doesn't correct problem
PDT agrees P1, especially since it blocks 32327
Created attachment 15291 [details] [diff] [review] Patch to add an AllowDoubleBuffering method to the viewmanager
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.
Created attachment 15318 [details] [diff] [review] Better patch - Allows ViewManager double-buffering to be turned off
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.
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.
Created attachment 15413 [details] [diff] [review] Patch file with only the code to add AllowDoubleBuffering method (clear backbuffer code removed).
firstname.lastname@example.org has reviewed the patch.
Fix checked in on the trunk. Waiting for clearance to checkin on the branch.
Am I to understand that this fix turns off double buffering for any pages on which there are <object> tags, or something similar?
The fix for this bug adds the capability to turn off double-buffering on viewmanger instance basis. email@example.com 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.
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?
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.
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.
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.
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.
> 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.
Per Kevin's last comment, marking nsbeta3-. The bug is already nominated for rtm.
oops really marking - this time
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.
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.
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.
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.
Marking nsbeta3- since we are past nsbeta3
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.
mid-air collision ? / bugzilla cleanup Reopening (current State: resolved and no resolution
Marking verified per last comments.