826093 - Remove ExtractFrame entirely and replace with a Clip method that returns an animated image

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

All callers of imgIContainer::ExtractFrame call it with FRAME_CURRENT, because they want the image to animate. Though we do probably want a way to disable animation for an image, ExtractFrame is not the best approach - it combines frame extraction with clipping, and most of the time we don't want to combine them. We can replace ExtractFrame with a simpler method that better matches our actual needs.

Comment 1

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

Actually, there is one caller of ExtractFrame that cares about freezing the animation of an image and _not_ clipping: imgRequestProxy::GetStaticRequest. So we need to split ExtractFrame into two concepts: freezing the animation of an image, and clipping a region out of an image. Bug 843895 covers replacing the freezing animation feature.

Comment 2

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

I think we're finally ready to go on this patch, which has been cooking for a while. The modern plan is to have an ImageWrapper, ClippedImage, take care of the clipping. ClippedImages still animate, and they do not actually result in any image data getting copied - instead the clipping is simulated at draw time. To get a ClippedImage, callers (inside or outside of imagelib) can use ImageOps::Clip().

Patches posted presently.

Comment 3

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

Attachment: (Part 1) - Add ClippedImage.

First part, in which we add the ImageWrapper that creates a clipped "view" of its underlying image.

Comment 4

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

Attachment: (Part 2) - Create a static utility class for image operations.

Here we create ImageOps. Now you can get a ClippedImage via ImageOps::Clip and a FrozenImage via ImageOps::Freeze. imgTools is avoided here because we don't need to expose these kinds of things to JavaScript directly and so the usability tradeoffs of involving an XPCOM service aren't worthwhile.

Comment 5

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

Attachment: (Part 3) - Use ClippedImage instead of ExtractFrame for -moz-image-rect.

Here we eliminate one of the two remaining uses of ExtractFrame.

Comment 6

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

Attachment: (Part 4) - Use ClippedImage instead of ExtractFrame for border-image.

Here we eliminate the other one. Note that ClippedImages animate, unlike the images returned from ExtractFrame, so we don't need to worry about whether the underlying image is animated or not anymore.

Comment 7

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

Attachment: (Part 5) - Remove imgIContainer::ExtractFrame.

With all of that out of the way, we can finally remove ExtractFrame and all of the nastiness that it involved.

Comment 8

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

Try job here: https://tbpl.mozilla.org/?tree=Try&rev=8bcd543e93e2

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 723726 [details] [diff] [review]
(Part 3) - Use ClippedImage instead of ExtractFrame for -moz-image-rect.

Why do we no longer need to worry about mFlags?  Is it taken into account later somehow?

r=me if it is.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 723730 [details] [diff] [review]
(Part 4) - Use ClippedImage instead of ExtractFrame for border-image.

Hmm.  So does this effectively defeat the caching we used to do?  Or is the caching just not really needed in the new setup?

r=me if the latter.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 723733 [details] [diff] [review]
(Part 5) - Remove imgIContainer::ExtractFrame.

Please rev the IID.  sr=me with that.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 723725 [details] [diff] [review]
(Part 2) - Create a static utility class for image operations.

sr=me

Comment 14

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

Thanks for the reviews Boris!

(In reply to Boris Zbarsky (:bz) from comment #11)
> Hmm.  So does this effectively defeat the caching we used to do?  Or is the
> caching just not really needed in the new setup?

I am doubtful that the caching makes sense anymore for typical use cases of border-image. It certainly does not make sense for the corner slices, which will typically be drawn just by adjusting the parameters to imgIContainer::Draw. (This is handled by ClippedImage, introduced in part 1.) A temporary surface _does_ have to be created when we draw the slices that tile, but I doubt that for typical uses this significantly impacts performance, because I expect the slices to be very small. We also win on memory usage here by not storing all of the slices persistently. And we no longer have to recopy everything every single time we redraw in the presence of animation - in that situation, this patch is a strict win.

If there's reason to believe it would be beneficial, we could easily add caching of slices at the imagelib layer. (i.e., inside ClippedImage) Basically, we'd just need to memoize the surface drawn by ClippedImage::DrawSingleTile for the last N combinations of arguments. (And take the current animation frame into account.) N=1 might be fine, but it depends how border-image usage looks in the wild.

Comment 15

[Boris Zbarsky [:bzbarsky]]

David, see comment 14?

Comment 16

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

(In reply to Seth Fowler [:seth] from comment #14)
> N=1 might be fine, but it
> depends how border-image usage looks in the wild.

Ack, sorry, momentarily forgot that ClippedImages are per-instance. N=1 is definitely fine.

Comment 17

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

(In reply to Boris Zbarsky (:bz) from comment #10)
> Why do we no longer need to worry about mFlags?  Is it taken into account
> later somehow?

It's taken into account because imgIContainer::Draw will also be passed FLAG_SYNC_DECODE later, and the decode will happen then. Prior to this patch, we needed to ensure it happened in PrepareImage in the moz-image-rect case because ExtractFrame would fail otherwise. ClippedImage doesn't have that issue.

(Note that we rely on this effect already in PrepareImage; you can see this happening in the "Special case" mentioned at the beginning of the method.)

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm not particularly sure why you'd think I'd have an opinion here, but I discussed this with seth and it sounds reasonable to me, at the very least from the perspective of not doing premature optimization.

Comment 19

[Joe Drew (not getting mail)]

Comment on attachment 723721 [details] [diff] [review]
(Part 1) - Add ClippedImage.

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

::: image/src/ClippedImage.cpp
@@ +168,5 @@
> +    new gfxImageSurface(gfxIntSize(mClip.width, mClip.height),
> +                        gfxASurface::ImageFormatARGB32);
> +  gfxContext ctx(surface);
> +  nsIntRect clipAtOrigin(0, 0, mClip.width, mClip.height);
> +  nsresult rv = Draw(&ctx, gfxPattern::FILTER_FAST, gfxMatrix(), clipAtOrigin,

Is there a reason you're using FILTER_FAST here?

@@ +299,5 @@
> +  if (sourceRect.width > mClip.width || sourceRect.height > mClip.height) {
> +    transform.Multiply(gfxMatrix().Translate(gfxPoint(-sourceRect.x, -sourceRect.y)));
> +    transform.Multiply(gfxMatrix().Scale(ClampFactor(sourceRect.width, mClip.width),
> +                                         ClampFactor(sourceRect.height, mClip.height)));
> +    transform.Multiply(gfxMatrix().Translate(gfxPoint(sourceRect.x, sourceRect.y)));

I presume you're not terribly interested in me going into great detail on your math here

::: image/src/ClippedImage.h
@@ +69,5 @@
> +                          uint32_t aWhichFrame,
> +                          uint32_t aFlags);
> +
> +  nsIntRect   mClip;              // The region to clip to.
> +  Maybe<bool> mShouldClip;        // Memoized ShouldClip() if present.

Is there a way we can make this be Maybe<nsIntRect> mClip?

Comment 20

[Joe Drew (not getting mail)]

Comment on attachment 723725 [details] [diff] [review]
(Part 2) - Create a static utility class for image operations.

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

::: image/src/ImageOps.h
@@ +29,5 @@
> +  static already_AddRefed<Image> Freeze(Image* aImage);
> +  static already_AddRefed<imgIContainer> Freeze(imgIContainer* aImage);
> +
> +  /**
> +   * Creates a clipped version of an existing image.

You should mention that it has no effect on animation, I suppose

Comment 21

[Joe Drew (not getting mail)]

Comment on attachment 723733 [details] [diff] [review]
(Part 5) - Remove imgIContainer::ExtractFrame.

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

::: image/public/imgIContainer.idl
@@ +57,4 @@
>   *
>   * Internally, imgIContainer also manages animation of images.
>   */
>  [scriptable, builtinclass, uuid(01c4f92f-f883-4837-a127-d8f30920e374)]

uuid needs to change

Comment 22

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

(In reply to Joe Drew (:JOEDREW! \o/) from comment #19)
> > +  nsresult rv = Draw(&ctx, gfxPattern::FILTER_FAST, gfxMatrix(), clipAtOrigin,
> 
> Is there a reason you're using FILTER_FAST here?

Yeah, because the intention was that no scaling would be happening. (I'm passing the identity matrix for the transform after all.) Is that invalid reasoning?

> @@ +299,5 @@
> > +  if (sourceRect.width > mClip.width || sourceRect.height > mClip.height) {
> > +    transform.Multiply(gfxMatrix().Translate(gfxPoint(-sourceRect.x, -sourceRect.y)));
> > +    transform.Multiply(gfxMatrix().Scale(ClampFactor(sourceRect.width, mClip.width),
> > +                                         ClampFactor(sourceRect.height, mClip.height)));
> > +    transform.Multiply(gfxMatrix().Translate(gfxPoint(sourceRect.x, sourceRect.y)));
> 
> I presume you're not terribly interested in me going into great detail on
> your math here

I definitely am, especially if there's a problem. My linear algebra is quite rusty. =)

TBH I am surprised that code is even needed, because I didn't expect to get transforms passed in that would result in grabbing a non-integer number of pixels. (That is, where aUserspaceToImagespace.Transform(aFill) results in a rectangle with non-integer width and height.) Without that code, I saw tiny lines at one end of border-image slices in some reftests.

Now that I look at this code again, it's pretty obvious that I could also do this with only one call to transform.Multiply(). I should fix that. Still getting used to this API.

> ::: image/src/ClippedImage.h
> @@ +69,5 @@
> > +                          uint32_t aWhichFrame,
> > +                          uint32_t aFlags);
> > +
> > +  nsIntRect   mClip;              // The region to clip to.
> > +  Maybe<bool> mShouldClip;        // Memoized ShouldClip() if present.
> 
> Is there a way we can make this be Maybe<nsIntRect> mClip?

The problem is we actually have three states here: "can't evaluate whether we should clip yet", "evaluated and the answer is no", "evaluated and the answer is yes". Maybe<bool> gives you three states quite nicely, but alas Maybe<nsIntRect> only gives you two.

Comment 23

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

Thanks for the reviews, by the way, Joe!

Comment 24

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

(In reply to Seth Fowler [:seth] from comment #22)
> Yeah, because the intention was that no scaling would be happening. (I'm
> passing the identity matrix for the transform after all.) Is that invalid
> reasoning?

Whoops; somehow I missed FILTER_NEAREST in the list. Nearest neighbor is actually  the right thing here, I think.

Comment 25

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

Attachment: (Part 1) - Add ClippedImage.

Switch to FILTER_NEAREST in ClippedImage::GetFrame. Get rid of some unneeded matrix multiplies.

Comment 26

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

Attachment: (Part 2) - Create a static utility class for image operations.

Address review comment.

Comment 27

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

Attachment: (Part 5) - Remove imgIContainer::ExtractFrame.

Address review comment.

Comment 28

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

There were a few oranges in the try job from cases where an assertion stating that we wouldn't tile in DrawSingleTile failed. It turns out this happens when gfxCallbackDrawable fails to create an offscreen surface and punts, sending the draw request directly to the callback as a last resort. There's not much that can be done in that case that won't result in some sort of visual artifact.

I turned the assertion into a warning and activated FLAG_CLAMP when it happens, so at least we get something on the screen. I can't reproduce this locally on OS X or Ubuntu, so hopefully it's difficult to encounter this IRL.

Comment 29

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

New try job here:

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

Comment 30

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

Try pretty much looks good although there are some weird errors on some of the Windows runs that seem to be harness or infrastructure issues. Just to be sure, I retriggered those tests a couple of times. If those retriggers come out green I think we're go for a push.

Comment 31

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

I find these Windows-only oranges a bit troubling. They're all timeout related. I'm willing to bet that it's just the copious amounts of logging code in the try version of this patch. Here's a Windows-only try push for the exact code that is posted to the bug, without the logging:

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

Comment 32

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

Argh. I just don't feel comfortable landing this without some more inquiry. On Windows there are a couple of tests where gfxCallbackDrawable (which we use explicitly in situations where we need to tile, since we can't tile a subimage directly) fails to allocate an offscreen surface and just bails and passes through the full, tiled Draw request to ClippedImage::DrawSingleTile. That method can't deal with such a request and we fail (by not drawing the tiled image properly).

Given that this happens so reliably in the mochitests (there are two specific tests that seem to trigger this behavior) I have to imagine that users might hit this too. It seems that there's no way around it, so I'm going to try to revise the patch to handle this situation when it occurs.

Comment 33

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

Attachment: (Part 1) - Add ClippedImage.

Rebase and minor tweaks.

Comment 34

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

Attachment: (Part 2) - Create a static utility class for image operations.

Rebase.

Comment 35

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

Attachment: (Part 3) - Use ClippedImage instead of ExtractFrame for -moz-image-rect.

Rebase.

Comment 36

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

Attachment: (Part 4) - Use ClippedImage instead of ExtractFrame for border-image.

Rebase.

Comment 37

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

Attachment: (Part 5) - Remove imgIContainer::ExtractFrame.

Rebase.

Comment 38

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

So I am no longer able to reproduce the issues on Windows. See this try job, where everything looks nice and green:

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

Comment 39

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

Pushed in:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e573640e390
https://hg.mozilla.org/integration/mozilla-inbound/rev/98258ea50c1e
https://hg.mozilla.org/integration/mozilla-inbound/rev/9129587531fe
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c65d8f508c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b97cab51e59

Comment 40

[Ryan VanderMeulen [:RyanVM]]

Backed out for Android reftest-2 failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/51ecec435845

https://tbpl.mozilla.org/php/getParsedLog.php?id=21123157&tree=Mozilla-Inbound

Comment 41

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

For convenience, here's the reftest analyzer link. Looks like layout/reftests/image-rect/background-with-other-properties.html, test five.

https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#log=REFTEST%2520TEST-UNEXPECTED-FAIL%2520%257C%2520http%253A//10.250.49.166%253A30210/tests/layout/reftests/image-rect/background-with-other-properties.html%2520%257C%2520image%2520comparison%2520%2528%253D%253D%2529%252C%2520max%2520difference%253A%2520113%252C%2520number%2520of%2520differing%2520pixels%253A%2520124%250AREFTEST%2520%2520%2520IMAGE%25201%2520%2528TEST%2529%253A%2520data%253Aimage/png%253Bbase64%252CiVBORw0KGgoAAAANSUhEUgAAAyAAAAPoCAYAAAAmy5qxAAAS50lEQVR4nO3ZsXHDMAAEQbSkUDWxF5bE1ujAuR0APNLyLgajVMxw8+MEAACIjLv/AAAA8H8IEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgsC5Bx8wEAAJ5PgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQ8XIHAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgsC5Axec4xeQEAgMcTIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABk1r3cZwNCgAAAwMcTIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkvNwBAICMAAEAADICBAAAyAgQAAAgI0AAAICMAAEAADICBAAAyAgQAAAgI0AAAICMAAEAADLLAmSMey8AAPB8AgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACATBcgs0eAAADAnydAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMise7r/WiAXXwAA4PEECAAAkBEgAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAAAZL3cAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgMyyABmT5xyTFwAAeDwBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBm3ct9NiAECAAAfDwBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEDGyx0AAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAILMsQMa+neN433P3bdVnAAAAF1oXIMfrHHed473qMwAAgAstDJC3AAEAAH5kAQEAADIWEAAAIGMBAQAAMhYQAAAgYwEBAAAyFhAAACBjAQEAADIWEAAAIGMBAQAAMhYQAAAgYwEBAAAyFhAAACBjAQEAADIWEAAAIGMBAQAAMhYQAAAgYwEBAAAyFhAAACBjAQEAADIWEAAAIGMBAQAAMhYQAAAgYwEBAAAyFhAAACBjAQEAADIWEAAAIGMBAQAAMhYQAAAgsy5A9u07Qo5X/7tvqz4DAAC40LIAAQAA+I0AAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgMwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHiqLwyJd4xcgMpMAAAAAElFTkSuQmCC%250AREFTEST%2520%2520%2520IMAGE%25202%2520%2528REFERENCE%2529%253A%2520data%253Aimage/png%253Bbase64%252CiVBORw0KGgoAAAANSUhEUgAAAyAAAAPoCAYAAAAmy5qxAAASq0lEQVR4nO3XMRZCIRAEwb3/pb9HMAAb1Kp95ITT8wAAAETm9AcAAID/IUAAAICMAAEAADICBAAAyAgQAAAgI0AAAICMAAEAADICBAAAyAgQAAAgsy1A5vABAAD3EyAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZAQIAACQESAAAEBGgAAAABkBAgAAZCx3AAAgI0AAAICMAAEAADICBAAAyAgQAAAgI0AAAICMAAEAADICBAAAyAgQAAAgI0AAAIDMtgCZxXtm8QEAANcTIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABk9i331YAQIAAA8PMECAAAkBEgAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAAAZyx0AAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAILMtQGbOPgAA4H4CBAAAyAgQAAAgI0AAAICMAAEAADICBAAAyAgQAAAgI0AAAIBMFyCrJ0AAAODrCRAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMvum+9sC+fADAACuJ0AAAICMAAEAADICBAAAyAgQAAAgI0AAAICMAAEAADICBAAAyAgQAAAgI0AAAICMAAEAADICBAAAyAgQAAAgI0AAAICMAAEAADICBAAAyFjuAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAAAZAQIAAGQECAAAkBEgAABARoAAAACZbQEyi/fM4gMAAK4nQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADICBAAACAjQAAAgIwAAQAAMgIEAADI7FvuqwEhQAAA4OcJEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyljsAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQGZbgMzhAwAA7idAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMgIEAAAICNAAACAjAABAAAyAgQAAMhY7gAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQEaAAAAAGQECAABkBAgAAJARIAAAQGYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAG71At1hfFQgiipFAAAAAElFTkSuQmCC%250AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAG71At1hfFQgiipFAAAAAElFTkSuQmCC%250A

Comment 42

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

Attachment: (Part 1) - Add ClippedImage.

Create our own temporary surface any time gfxUtils::DrawPixelSnapped would have if it was called on desktop Gecko. (On mobile DrawPixelSnapped won't create a temporary surface, but we need that.)

Comment 43

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

Attachment: (Part 5) - Remove imgIContainer::ExtractFrame.

Rebase.

Comment 44

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

I'm gonna take a gamble here because I think the fix is really straightforward. (We just needed to create a temporary surface in more cases because of differences between desktop and mobile.) I don't think this needs a try run. (I did test locally.) Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1799cffbba63
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9c38836a706
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fd51895fb0c
https://hg.mozilla.org/integration/mozilla-inbound/rev/a880fd3e6e2a
https://hg.mozilla.org/integration/mozilla-inbound/rev/957f2b35ce83

Comment 45

[Ryan VanderMeulen [:RyanVM]]

Still failing the same way. Backed out again.
https://hg.mozilla.org/integration/mozilla-inbound/rev/446b90989fdd

Comment 46

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

Argh. Sorry about that Ryan. Will investigate further.

Comment 47

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

Attachment: Part 6 - Fix Android rendering issues caused by sampling error.

I was able to resolve the Android reftest failures, although not in the way I had initially hoped. There are some cases where gfxUtils::DrawPixelSnapped ends up calling CreateSamplingRestrictedDrawable despite my best efforts, and in those cases the rendering differed between Android and desktop because that code path isn't enabled on Android. I ended up approaching the problem by making it possible to override that restriction by setting a bit on the gfxContext. We also check the size of the surface we're going to create in CreateSamplingRestrictedDrawable, and refuse to allocate anything bigger than 256x256. I did this so that we'd still keep the cases we did this for and the amount of memory consumed in check; now there's just the possibility of getting nice scaling for images of reasonable size. (And the Android reftest that was failing now passes!)

Try jobs here (first Linux/OS X/Windows, then Android):
https://tbpl.mozilla.org/?tree=Try&rev=de96f335b1e5
https://tbpl.mozilla.org/?tree=Try&rev=45ef9b3b7a80

Comment 48

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 732166 [details] [diff] [review]
Part 6 - Fix Android rendering issues caused by sampling error.

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

::: gfx/thebes/gfxContext.h
@@ +708,5 @@
>      void CopyAsDataURL();
>  #endif
>  
> +    bool ForceCorrectResampling() { return mForceCorrectResampling; }
> +    void SetForceCorrectResampling(bool aForce) { mForceCorrectResampling = aForce; }

Doing this feels like you're abusing gfxContext to store some internal image drawing state. I'd be much happier if this state was left to imagelib itself.

Comment 49

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

Attachment: (Part 1) - Add ClippedImage.

Rebased. When we need to generate a temporary surface, we always use the same code now, whether it occurs in Draw or GetFrame. (The code is implemented in GetFrameInternal.)

Comment 50

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

Attachment: (Part 2) - Create a static utility class for image operations.

Rebase.

Comment 51

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

Attachment: (Part 3) - Use ClippedImage instead of ExtractFrame for -moz-image-rect.

Per discussion of this issue between Jeff and myself and further discussion at the Rendering meeting, marked layout/reftests/image-rect/background-with-other-properties.html as fuzzy on Android and B2G so we don't need any workarounds.

Comment 52

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

Attachment: (Part 4) - Use ClippedImage instead of ExtractFrame for border-image.

Rebase.

Comment 53

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

Attachment: (Part 5) - Remove imgIContainer::ExtractFrame.

Rebase.

Comment 54

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

Looks like we're good to go on Android. The reftests are all green on this try job:

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

Comment 55

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/521215a7b32e
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb83f2bd2eb1
https://hg.mozilla.org/integration/mozilla-inbound/rev/011194378653
https://hg.mozilla.org/integration/mozilla-inbound/rev/79cfda08fdbb
https://hg.mozilla.org/integration/mozilla-inbound/rev/051cf1c1449c

Comment 56

[Phil Ringnalda (:philor)]

Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/4c45dbd81a32 - apparently just over 2 hours between your try push and your inbound push was enough time for someone to undermine you, image-rect/background-with-other-properties.html | image comparison (==), max difference: 113, number of differing pixels: 124

Comment 57

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

Ah man. That's because I forgot to push the change that added a fuzzy-if to that test. =( This bug just won't die!

Comment 58

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

I really hope this is the last time! Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f04bd50a5c13
https://hg.mozilla.org/integration/mozilla-inbound/rev/548590c42367
https://hg.mozilla.org/integration/mozilla-inbound/rev/96764c6123e2
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec0f65680925
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef4448dadcc3

Comment 59

[Boris Zbarsky [:bzbarsky]]

Could this be causing the large TResize talos regressions?

Regression: Mozilla-Inbound-Non-PGO - TResize - WINNT 6.2 x64 - 28.6% increase
Regression: Mozilla-Inbound-Non-PGO - TResize - Win7 - 57.8% increase

as well as smaller regressions for Paint:

Regression: Mozilla-Inbound-Non-PGO - Paint - Win7 - 10.6% increase

and Ts, Paint:

Regression: Mozilla-Inbound-Non-PGO - Ts, Paint - Win7 - 3.63% increase
Regression: Mozilla-Inbound-Non-PGO - Ts Paint, MAX Dirty Profile - Win7 - 3.52% increase
Regression: Mozilla-Inbound-Non-PGO - Ts Paint, MED Dirty Profile - Win7 - 3.97% increase

Comment 60

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f04bd50a5c13
https://hg.mozilla.org/mozilla-central/rev/548590c42367
https://hg.mozilla.org/mozilla-central/rev/96764c6123e2
https://hg.mozilla.org/mozilla-central/rev/ec0f65680925
https://hg.mozilla.org/mozilla-central/rev/ef4448dadcc3

Comment 61

[John Daggett (:jtd)]

These changes do appear to have caused the Tp regression Boris notes in comment 59.  

http://graphs.mozilla.org/graph.html#tests=[[255,131,12]]&sel=1365190444103.1843,1365211468436.9038&displayrange=7&datatype=running

Testing this now to confirm.

Comment 62

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

They did.

I am about to go into a meeting, but once I'm out I'll file a followup bug on making ClippedImage faster. Basically we need to memoize whatever temporary surfaces we end up creating. Because I think there's some potential for error there I wanted to do that as a separate bug.

Comment 63

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

OK, the regressions will be fixed in bug 859377. bz, jtd, I'll CC you.