923341 - Update imagelib surface caching code to avoid creating unnecessary Thebes contexts

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

Once we have gfxDrawTargetDrawable we can remove the unnecessary GetThebesSurfaceForDrawTarget() call in VectorImage::CreateDrawableAndShow.

Comment 1

[Seth Fowler [:seth] [:s2h]]

We should also update CachedSurface::Drawable. Any code that uses SurfaceCache should get a lookover once gfxDrawTargetDrawable is available.

Comment 2

[Seth Fowler [:seth] [:s2h]]

At the moment it looks like this will be solved by adding features to Moz2D (source clipping for patterns) to support the missing use cases we need, and then not calling DrawPixelSnapped at all on Moz2D platforms. This will get hammered out in more detail at the upcoming web rendering work week.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 1: Add a gfxSurfaceDrawable constructor for a DrawTarget

This adds a dreaded call to GetThebesSurfaceForDrawTarget, but only to use as a source. This should work everywhere, though may involve readback.

There's no other way to avoid this other than just never having a gfxContext that returns true for IsCairo() (which we're working on!).

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 2: Use SVG caches on all azure backends.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 3: Use azure for CreateSamplingRestrictedDrawable

Comment 6

[Matt Woodrow (:mattwoodrow)]

https://tbpl.mozilla.org/?tree=Try&rev=ebfe8d83183d

Comment 7

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 831284 [details] [diff] [review]
Part 2: Use SVG caches on all azure backends.

Review of attachment 831284 [details] [diff] [review]:
-----------------------------------------------------------------

So glad that ugly code is gone!

::: image/src/VectorImage.cpp
@@ +836,5 @@
>    // If we couldn't create the draw target, it was probably because it would end
>    // up way too big. Generally it also wouldn't fit in the cache, but the prefs
>    // could be set such that the cache isn't the limiting factor. If we couldn't
>    // create the surface, we are on a platform that does not support
>    // GetThebesSurfaceForDrawTarget. This will be fixed in bug 923341.

Should probably update this comment since |surface| is gone now. (Can just remove the last two sentences, I think.)

Is there another reason CreateOffscreenContentDrawTarget could fail?

@@ +842,2 @@
>      return Show(svgDrawable, aParams);
> +  

Nit: Stray whitespace.

Comment 8

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 831285 [details] [diff] [review]
Part 3: Use azure for CreateSamplingRestrictedDrawable

Review of attachment 831285 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxUtils.cpp
@@ +271,5 @@
> +    if (target) {
> +      drawable = new gfxSurfaceDrawable(target, size, gfxMatrix().Translate(-needed.TopLeft()));
> +    } else {
> +      drawable = new gfxSurfaceDrawable(temp, size, gfxMatrix().Translate(-needed.TopLeft()));
> +    }

Maybe just fold this into the previous if/else pair?

Comment 9

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 4: Update CachedSurface::Drawable

Missed this the first time.

Comment 10

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 831812 [details] [diff] [review]
Part 4: Update CachedSurface::Drawable

Review of attachment 831812 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment 11

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/faef54ec06cd
https://hg.mozilla.org/integration/mozilla-inbound/rev/79cfed9c9d1d
https://hg.mozilla.org/integration/mozilla-inbound/rev/810c38bc0493
https://hg.mozilla.org/integration/mozilla-inbound/rev/99108bac6f2d

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/faef54ec06cd
https://hg.mozilla.org/mozilla-central/rev/79cfed9c9d1d
https://hg.mozilla.org/mozilla-central/rev/810c38bc0493
https://hg.mozilla.org/mozilla-central/rev/99108bac6f2d