Closed Bug 576143 Opened 14 years ago Closed 14 years ago

speed up gfxXlibNativeRenderer in the case where unsupported clip is the only reason for fallback

Categories

(Core :: Graphics, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b3
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: karlt, Assigned: karlt)

References

Details

(Keywords: perf)

Attachments

(2 files)

Currently if the native drawing does not support the clipping region from the context we do a costly fallback to draw twice, read back, and compare to extract alpha information.

A much more efficient approach would be to blit the background to a single matching temporary surface, draw once, and blit with the clip to the context.
Keywords: perf
Blocks: 537860
Blocks: 579676
Depends on: 580440
Blocks: 573933
I'd like to be able to find a visual to match the format of image surfaces.
This adds a static method to gfxXlibSurface similar to FindRenderFormat.
Attachment #458897 - Flags: review?(jmuizelaar)
This adds two new strategies that can be used to avoid alpha-extraction:
1. Copying the background from the target to the temp drawing surface.
2. Using a temp drawing surface with an alpha channel.
Attachment #458903 - Flags: review?(roc)
Requesting blocking2.0:
1. Bug 574583 significantly slowed down transparent flash performance in some
   situations and this recovers much of that.
2. Since retained layers, clips are more involved with drawing and so are
   causing fallback more often.
3. A number of reftests were modified in bug 573933 due to inaccuracies in the
   alpha-extraction path.  This is needed to restore the tests.
blocking2.0: --- → ?
Attachment #458897 - Flags: review?(jmuizelaar) → review+
blocking2.0: ? → betaN+
Comment on attachment 458903 [details] [diff] [review]
copy and draw over background to avoid alpha extraction when possible

>     if (!drawIsOpaque) {
>         tmpCtx = new gfxContext(tempXlibSurface);
>-        tmpCtx->SetOperator(gfxContext::OPERATOR_CLEAR);
>+        if (method == eCopyBackground) {
>+            tmpCtx->SetOperator(gfxContext::OPERATOR_SOURCE);
>+            tmpCtx->SetSource(target, -(offset + matrix.GetTranslation()));
>+            // The copy from the tempXlibSurface to the target context should
>+            // use operator SOURCE, but that would need a mask to bound the
>+            // operation.  Here we only copy opaque backgrounds so operator
>+            // OVER will behave like SOURCE masked by the surface.
>+            NS_ASSERTION(tempXlibSurface->GetContentType()
>+                         == gfxASurface::CONTENT_COLOR,
>+                         "Don't copy background with a transparent surface");
>+        } else {
>+            tmpCtx->SetOperator(gfxContext::OPERATOR_CLEAR);
>+        }
>         tmpCtx->Paint();
>     }
> 
>     if (!DrawOntoTempSurface(tempXlibSurface, -drawingRect.TopLeft())) {
>         return;
>     }
>   
>-    if (drawIsOpaque) {
>+    if (method != eAlphaExtraction) {
>         ctx->SetSource(tempXlibSurface, offset);
>         ctx->Paint();

I guess that should say something like "would need a mask or fill path to bound the operation".
Comment on attachment 458903 [details] [diff] [review]
copy and draw over background to avoid alpha extraction when possible

+        && visual->bits_per_rgb == 8
+        && visual->red_mask == 0xff0000
+        && visual->green_mask == 0xff00
+        && visual->blue_mask == 0xff
+        && gfxXlibSurface::DepthOfVisual(screen, visual) == 32;

+        || visual->c_class != TrueColor
+        || format->type != PictTypeDirect
+        || gfxXlibSurface::DepthOfVisual(screen, visual) != format->depth)

Style is to put operator at end of line. I don't like it, but that's what it is. Happens in various other places too.
Attachment #458903 - Flags: review?(roc) → review+
http://hg.mozilla.org/mozilla-central/rev/92559a5573bd
http://hg.mozilla.org/mozilla-central/rev/8fd528e132d7

Included the changes requested in comment 5.

Tested by http://hg.mozilla.org/mozilla-central/rev/02d92df7381d
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b3
Blocks: 583621
Depends on: 628190
You need to log in before you can comment on or make changes to this bug.