384845 - drawing support for X11 windowless plugins

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Karl Tomlinson (:karlt)]

Attachment: drawing support for X11 windowless plugins

A GraphicsExpose XEvent is sent through NPP_HandleEvent.
This contains the Xlib Drawable, its Display,
and the dirty rectangle (optional clip rectangle).  The Drawable
is an offscreen Pixmap.

The plugin should draw to the Drawable with the offset specified
in the x and y members of the NPWindow structure, with the
clipping rectangle specified in the clipRect member, and with the
Visual and Colormap specified in the ws_info member.  (A
NPP_SetWindow call notifies the plugin of any changes to these.)

"The clipRect member defines the clipping rectangle of the plug-in
in a coordinate system where the origin is the top-left corner of
the drawable" according to existing documentation.  The dirty
rectangle is specified similarly.

This patch was started by David Brittain <[e-mail]>, Leon Sha <[e-mail]>, Pete Collins <[e-mail]>, and Robert O'Callahan <[e-mail]>.

See bug #137189 for history and test code in unixprinting plugin.
(Just splitting things up to make it easier for me to understand.)

Comment 1

[Karl Tomlinson (:karlt)]

Comment on attachment 268749 [details] [diff] [review]
drawing support for X11 windowless plugins

jst if you could look at the plugin bits, and roc checks the layout bits, that would be great.

Comment 2

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

+#ifdef MOZ_X11 // XXX Don't others want this too!

Looks like the Mac and Windows code assume the rect is relative to the nearest nsIWidget, which is often but not always the case. So there's a bug here --- please file it.

+              // XXX Can we be certain that the location wrt to the window only
+              // changes when the location wrt to the drawable changes?

Probably the HDC changes every time so doupdatewindow is always true and this issue doesn't matter. Add some comments to that effect.

+void nsPluginInstanceOwner::Paint(const nsRect& aDirtyRect, PRUint32 ndc)

I think you can change this to a real HDC now.

+  // Sanitize the dirty rect so we do tell plugins that the area outside the

So we "don't", right?

+  // We could probably just render to gfxXlibSurface::XDrawable from
+  // gfxContext::CurrentSurface (provided the surface is an Xlib surface,
+  // not an image) which basically does cairo_xlib_surface_get_drawable?

That wouldn't work if there's a nontrivial transformation (e.g. scale) or other things going on, so we'd basically have to copy the checks done by gfxXlibNativeRenderer up here. Probably just remove this comment.

+    (gfxContext*)aRenderingContext.GetNativeGraphicData(nsIRenderingContext::NATIVE_THEBES_CONTEXT);

NS_STATIC_CAST

+  // Strip non-translations from the current matrix at least until we decide
+  // whether we and the plugin can handle them and their effects on events.
+  // Scale changes should be possible but rotations will require more thought.

Why are you doing this? gfxXlibNativeRenderer can handle this. Event transformation should be pretty easy too.

+    // gfxContext::CurrentSurface should be a gfxXlibSurface with the visual
+    // derived from gdk_rgb_get_colormap in moz_container_init.

It might not be. It could be a PDF surface or an image surface. Maybe just remove this comment and replace it with a comment about how we need a colormap but aren't sure which one to use...

Other than that nsObjectFrame looks good. I can review the rest if jst doesn't want to, but he'd better say either way.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: drawing support for X11 windowless plugins 2

* Addressed roc's suggestions.

* Added #ifdef MOZ_WIDGET_GTK2 where appropriate.

* Setting colormap from nearest window in DidReflow instead of from GdkRGB in NativeDraw as the former is more often correct.  Setting ws_info->display in the same place, instead of SetWindow which doesn't know the widget.

* Added some comments in nsObjectFrame::GetWindowOriginInPixels

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 269346 [details] [diff] [review]
drawing support for X11 windowless plugins 2

The above changes make it easy to split the patch in two distinct parts.  Neither is useful on their own though.

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: preparation for drawing support

This patch make the NPSetWindowCallbackStruct memory associated with window->ws_info available before SetWindow is called, and disables the plugin window GtkSocket or xtbin creation for windowless plugins.

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: preparation for drawing support diff -w

Most of the differences in the above patch are indentation changes,
so here it is without the whitespace differences.

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: drawing support 3

Just the painting and drawable/window info in ws_info.

roc can you review the code, please.
jst please just confirm that you are happy with the effects on the plugin API
[or you can look at the code too if you wish :)]

Comment 8

[Karl Tomlinson (:karlt)]

Attachment 269362 [details] [diff] "drawing support 3" now needs to be applied with -F4 following checkin from bug 347743.

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 269360 [details] [diff] [review]
preparation for drawing support

r+sr=jst

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 269362 [details] [diff] [review]
drawing support 3

Plugin API impact looks fine to me.

Comment 11

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

checked those patches in.

Comment 12

[Bill Gianopoulos [:WG9s]]

This is causing crashes with Java(TM) Plug-in 1.6.0_01-b06 reported in bug 386844.