574583 - Extend windowless Flash Player drawing offset workaround to handle clips also

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Karl Tomlinson (:karlt)]

As discovered in bug 526797, Flash Player windowless plugins render the correct content in response to expose events, in either of (only) two (known) situations:

1) When the top-left of the rectangle in the expose event is the top-left of
   the plugin rectangle.

2) When the top-left of the rectangle in the expose event is the top-left of
   the Drawable.

When the Drawable top-left is above and to the left of the plugin rectangle
top-left, and the top-left of the expose rectangle is to the right and/or
below the top-left of the plugin, the plugin seems draw to the correct rectangle but the content drawn is as if the plugin rectangle top-left had moved to the top-left of the expose rectangle.

(I don't know what happens when the Drawable top-left is below or to the right
of the plugin rectangle, but not aligned with the expose event.)

In bug 526797, a workaround was added to always extend the expose above and to
the left until it hit the top-left of either the Drawable, plugin rectangle, or clip rectangle.

That used to cover most common situations because generally the drawable was
created for the expose event and so aligned with the expose event rectangle,
and so 1 or 2 could often be satisfied.  The workaround did not cover cases
where we needed to clip the drawing, because the plugin could not be asked to
paint outside the clip.

With retained layers, we no longer have an intermediate drawable for the expose
event as drawing goes directly to the layer.  We can't pollute the retained
contents in the layer but outside the expose region so drawing is clipped to
the expose region.  This clip is regularly inside the plugin rectangle when
the plugin invalidates a portion of its rectangle and when the plugin scrolls
into view.

There are testcases in bug 526797.

Comment 1

[Karl Tomlinson (:karlt)]

I think we need to tell the NativeRenderer that drawing from this plugin
cannot be clipped.  If it needs to clip, it will then take the (slow) fallback
path that creates intermediate surfaces for plugin rendering.  We'll then be
able to set the expose event origin to the origin of the intermediate surface.

This fallback path will be *very* slow for transparent Flash plugins because
Flash Player crashes when asked to render to a Drawable using a Visual with an alpha channel (Bug 445250 comment 4) and so we need to calculate alpha values for the compositing.  

The NativeRenderer currently uses the same *very* slow path even for opaque
renderering without clip support, but this can be improved.

Comment 2

[Karl Tomlinson (:karlt)]

Extending the invalidation rectangles that we receive from Flash Player out to
the full plugin rectangle may help in situations where the plugin rectangle is
fully visible as drawing will then cover the whole plugin and not need to be
clipped.

However, iff the plugin is only partially visible then this could only make things worse because there would still be a clip and so the slow fallback would just need to draw more.

Perhaps some heuristic based on the last info from nsDisplayPlugin::ComputeVisibility() could be used to guess whether the plugin is fully visible.

Comment 3

[Karl Tomlinson (:karlt)]

The "iff" was meant to be an ordinary "if".

Comment 4

[Karl Tomlinson (:karlt)]

(In reply to comment #1)
> This fallback path will be *very* slow for transparent Flash plugins because
> Flash Player crashes when asked to render to a Drawable using a Visual with an
> alpha channel (Bug 445250 comment 4) and so we need to calculate alpha values
> for the compositing.  
> 
> The NativeRenderer currently uses the same *very* slow path even for opaque
> renderering without clip support, but this can be improved.

These comments are mostly wrong sorry.  I got confused mixing up something else.

Currently the transparent fallback path is *very* slow, but we can make that faster in the situation where the reason for fallback is that a clip must be applied.

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: provide the appropriate hints to the NativeRenderer

Comment 6

[Karl Tomlinson (:karlt)]

The patch tells the renderer that this plugin can't be given a clip, so an
intermediate surface will be used when a clip is required.  (I'm working on
making that process faster, particularly for transparent plugins.)

It also tells the renderer that this plugin does not support other visuals
(Bug 445250 comment 4).

I didn't want to doom Flash Player to the slower path for the rest of
eternity, so the check is for Flash 10 (including .0 or .1 and any future minor releases).  There were no versions with windowless support on X11 before 10.0 (betas).

If we hear from Flash Player developers that this issue has been resolved then
we can also easily re-enable the fast path for a minor release.  Plugin developers can test by changing their plugin description string.

I removed DRAW_SUPPORTS_ALTERNATE_SCREEN because I'm not confident that
plugins would support that and Gecko doesn't use more than one screen anyway.

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: provide the appropriate hints to the NativeRenderer

(removed some extras that didn't belong in this patch)

Comment 8

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

Comment on attachment 455045 [details] [diff] [review]
provide the appropriate hints to the NativeRenderer

I'll review this. It loosk good.

Comment 9

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/7e66b0f2f675