The default bug view has changed. See this FAQ.

drawing support for X11 windowless plugins

RESOLVED FIXED

Status

()

Core
Plug-ins
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

10 years ago
Created attachment 268749 [details] [diff] [review]
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 <mozilla@paperetto.com>, Leon Sha <leon.sha@sun.com>, Pete Collins <pete@mozdevgroup.com>, and Robert O'Callahan <roc@ocallahan.org>.

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

Comment 1

10 years ago
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.
Attachment #268749 - Flags: review?(roc)
Attachment #268749 - Flags: review?(jst)
(Assignee)

Updated

10 years ago
Blocks: 384975
+#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.
(Assignee)

Updated

10 years ago
Blocks: 384988
(Assignee)

Comment 3

10 years ago
Created attachment 269346 [details] [diff] [review]
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
Attachment #268749 - Attachment is obsolete: true
Attachment #269346 - Flags: superreview?(roc)
Attachment #269346 - Flags: review?(roc)
Attachment #269346 - Flags: review?(jst)
Attachment #268749 - Flags: review?(roc)
Attachment #268749 - Flags: review?(jst)
(Assignee)

Updated

10 years ago
Attachment #269346 - Attachment is patch: true
Attachment #269346 - Attachment mime type: text/x-patch → text/plain
(Assignee)

Comment 4

10 years ago
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.
Attachment #269346 - Flags: superreview?(roc)
Attachment #269346 - Flags: review?(roc)
Attachment #269346 - Flags: review?(jst)
(Assignee)

Comment 5

10 years ago
Created attachment 269360 [details] [diff] [review]
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.
Attachment #269360 - Flags: superreview?(jst)
Attachment #269360 - Flags: review?(jst)
(Assignee)

Comment 6

10 years ago
Created attachment 269361 [details] [diff] [review]
 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.
(Assignee)

Comment 7

10 years ago
Created attachment 269362 [details] [diff] [review]
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 :)]
Attachment #269346 - Attachment is obsolete: true
Attachment #269362 - Flags: superreview?(roc)
Attachment #269362 - Flags: review?(roc)
Attachment #269362 - Flags: review?(jst)
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
Attachment #269362 - Flags: superreview?(roc)
Attachment #269362 - Flags: superreview+
Attachment #269362 - Flags: review?(roc)
Attachment #269362 - Flags: review+
(Assignee)

Comment 8

10 years ago
Attachment 269362 [details] [diff] "drawing support 3" now needs to be applied with -F4 following checkin from bug 347743.
Comment on attachment 269360 [details] [diff] [review]
preparation for drawing support

r+sr=jst
Attachment #269360 - Flags: superreview?(jst)
Attachment #269360 - Flags: superreview+
Attachment #269360 - Flags: review?(jst)
Attachment #269360 - Flags: review+
Comment on attachment 269362 [details] [diff] [review]
drawing support 3

Plugin API impact looks fine to me.
Attachment #269362 - Flags: review?(jst) → review+
(Assignee)

Updated

10 years ago
Blocks: 386537
checked those patches in.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Depends on: 386844
This is causing crashes with Java(TM) Plug-in 1.6.0_01-b06 reported in bug 386844.
Flags: in-testsuite?
(Assignee)

Updated

9 years ago
Depends on: 430450
(Assignee)

Updated

9 years ago
Depends on: 435764
You need to log in before you can comment on or make changes to this bug.