Last Comment Bug 685082 - Transparent plugins not cleared on Maemo
: Transparent plugins not cleared on Maemo
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Maemo
: -- normal (vote)
: mozilla9
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
http://www.communitymx.com/content/so...
Depends on:
Blocks: 626602
  Show dependency treegraph
 
Reported: 2011-09-07 00:46 PDT by Oleg Romashin (:romaxa)
Modified: 2011-09-14 06:49 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Transparent rendering Plugins part (1.80 KB, patch)
2011-09-07 03:05 PDT, Oleg Romashin (:romaxa)
karlt: review-
Details | Diff | Review
Shadowable Image layer buffer update part (1.04 KB, patch)
2011-09-07 03:06 PDT, Oleg Romashin (:romaxa)
cjones.bugs: review+
Details | Diff | Review
Transparent rendering Plugins part (1.78 KB, patch)
2011-09-07 21:23 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Transparent rendering Plugins part (1.85 KB, patch)
2011-09-07 23:18 PDT, Oleg Romashin (:romaxa)
karlt: review-
Details | Diff | Review
Transparent rendering Plugins part (1.78 KB, patch)
2011-09-08 00:05 PDT, Oleg Romashin (:romaxa)
karlt: review+
Details | Diff | Review

Description Oleg Romashin (:romaxa) 2011-09-07 00:46:58 PDT
For Maemo transparent plugins we don't have anymore plugin surface clearing code called:
we call PaintRectToSurface to surface from here:
http://hg.mozilla.org/releases/mozilla-2.1/diff/a96c75dbc151/dom/plugins/PluginInstanceChild.cpp#l1.412

And this check never works when mIsTransparent = TRUE:
http://hg.mozilla.org/releases/mozilla-2.1/diff/a96c75dbc151/dom/plugins/PluginInstanceChild.cpp#l1.314

https://bugzilla.mozilla.org/show_bug.cgi?id=626602#c98

Problem is reproducible on Maemo5 with enabled plugins...
Comment 1 Oleg Romashin (:romaxa) 2011-09-07 02:11:08 PDT
Also facing another problem, that even if that clearing with transparent color enabled fill does not seems to happen...
SetColor(alpha 0), SetOperator(gfxContext::OPERATOR_SOURCE), Rectangle(), Fill(); - does not clear surface, same with operator clear + paint()
if I replace that code with Qpainter code, then surface cleared correctly... also we do have surface->Flush() - which was missing before and causing some problems
don't understand what is going on yet...
Comment 2 Oleg Romashin (:romaxa) 2011-09-07 02:35:26 PDT
Ok, did call MarkDirty() clearing surface start working correctly
Comment 3 Oleg Romashin (:romaxa) 2011-09-07 02:57:10 PDT
Also found that BasicShadowableImageLayer::Paint does Image layer update using OPERATOR_OVER all the time which is incorrect
Comment 4 Oleg Romashin (:romaxa) 2011-09-07 03:05:26 PDT
Created attachment 558761 [details] [diff] [review]
Transparent rendering Plugins part
Comment 5 Oleg Romashin (:romaxa) 2011-09-07 03:06:25 PDT
Created attachment 558762 [details] [diff] [review]
Shadowable Image layer buffer update part
Comment 6 Karl Tomlinson (ni?:karlt) 2011-09-07 19:45:00 PDT
Comment on attachment 558761 [details] [diff] [review]
Transparent rendering Plugins part

>-    if (aColor.a > 0.0) {
>+    if (aColor.a > 0.0 || (mIsTransparent && !CanPaintOnBackground())) {

Looks like this was my fault (Bug 626602 comment 98).
Please drop the aColor.a > 0.0 test, as the rest is sufficient.

>     if (mMaemoImageRendering &&
>         aSurface->GetType() == gfxASurface::SurfaceTypeImage) {
>+        aSurface->MarkDirty(gfxRect(aRect.x, aRect.y, aRect.width, aRect.height));
>         aSurface->Flush();

The MarkDirty() should be done after the plugin has drawn.
Comment 7 Oleg Romashin (:romaxa) 2011-09-07 20:30:36 PDT
> >     if (mMaemoImageRendering &&
> >         aSurface->GetType() == gfxASurface::SurfaceTypeImage) {
> >+        aSurface->MarkDirty(gfxRect(aRect.x, aRect.y, aRect.width, aRect.height));
> >         aSurface->Flush();
> 
> The MarkDirty() should be done after the plugin has drawn.
Err... MarkDirty and flush need to be here in order to flush operations which has been done before (such like surface clear), plugin will render data into that surface, because plugin is not using aSurface cairoContext....

After plugin paint flush in not needed because plugin not using that context.
Flush after plugin paint done on UI process (RecvShow)
Comment 8 Oleg Romashin (:romaxa) 2011-09-07 21:23:47 PDT
Created attachment 559041 [details] [diff] [review]
Transparent rendering Plugins part

Remove aColor check, rest is the same
Comment 9 Oleg Romashin (:romaxa) 2011-09-07 23:18:59 PDT
Created attachment 559048 [details] [diff] [review]
Transparent rendering Plugins part

Mark dirty before readback
Comment 10 Karl Tomlinson (ni?:karlt) 2011-09-07 23:52:35 PDT
Comment on attachment 559048 [details] [diff] [review]
Transparent rendering Plugins part

I think you'd have to mark the back surface dirty here to achieve the goal because the surfaces have been swapped here.

But on further thought, I'd prefer the MarkDirty in PaintRectToPlatformSurface() after the mPluginIface->event() call.

It doesn't make much difference with mMaemoImageRendering, but there are other paths using multiple surfaces.  For them it is hard to track which surface needs to be MarkDirty()ed but this is often enough not the right surface to mark dirty and I'm not certain about what impact an unnecessary MarkDirty might have on performance there.
Comment 11 Oleg Romashin (:romaxa) 2011-09-08 00:05:08 PDT
Created attachment 559061 [details] [diff] [review]
Transparent rendering Plugins part

oh ok, now I understood it.
Comment 12 Ed Morley [:emorley] 2011-09-13 17:23:26 PDT
In my queue of bits and pieces for try then inbound.

Note You need to log in before you can comment on or make changes to this bug.