1031576 - Add a new Image subclass for dynamically-created images and use it for -moz-element

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

Right now, -moz-element is a special case that reimplements some things that are already implemented elsewhere (such as clipping for border-image) and has a special drawing path that nothing else uses.

In this bug we'll add a new Image subclass that wraps a gfxDrawable. -moz-element code can then use the normal image drawing path. This will ensure that when bugs are encountered in the border-image or pixel snapping code, -moz-element gets the benefits, without any further work. (It's especially important that this not be a special path because -moz-element is used so rarely, so regressions may not be noticed immediately.)

In addition, this will support upcoming imagelib refactorings that require more imagelib involvement in the pixel snapping code. This was quite awkward for the -moz-element case since right now there is no Image/imgIContainer object in that situation.

Comment 1

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

Attachment: (Part 1) - Add an Image subclass for dynamically-created images

This patch adds a new Image subclass, DynamicImage, intended for use for emphemeral, dynamically-generated images. Such images are backed by a gfxDrawable.

The implementation is verbose just because there are so many methods on Image, but most of them are irrelevant for this use, so there's fairly little non-boilerplate code here.

Comment 2

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

Attachment: (Part 2) - Use dynamic images in the implementation of -moz-element

This patch replaces the current -moz-element drawing code with code that uses DynamicImage.

This should be at least as performant as the old code and sometimes better, since ClippedImage can often avoid copying.

To avoid issues with cache invalidation, I still don't cache the ClippedImage instances that get created.

Comment 3

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

Attachment: (Part 3) - Remove nsLayoutUtils::DrawPixelSnapped, since it's dead code now

After part 2, nsLayoutUtils::DrawPixelSnapped is dead code. This patch removes it.

Eliminating this code path should both make maintenance easier and make the soon-to-arrive imagelib refactorings much cleaner.

Comment 4

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

Try job here:

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

Comment 5

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

Hmm, looks like this patch busted something. I suspect I know exactly what the problem is; should be a quick fix. I'll take a look after dinner.

Comment 6

[Nick Cameron [:nrc]]

Comment on attachment 8447530 [details] [diff] [review]
(Part 2) - Use dynamic images in the implementation of -moz-element

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

lgtm

Comment 7

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

Attachment: (Part 2) - Use dynamic images in the implementation of -moz-element

Looks like we just needed to call DrawImage instead of DrawSingleImage in the Element case. This is odd because I just copied the DrawSingleImage call from the Image case.

IRC discussions between Nick and myself have led to the conclusion that the Image case may actually be dead code. If this turns out to be true, it'll be dealt with in a followup bug.

Comment 8

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

New try job here:

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

Comment 9

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

Comment on attachment 8447531 [details] [diff] [review]
(Part 3) - Remove nsLayoutUtils::DrawPixelSnapped, since it's dead code now

r=dbaron; sorry for the delay getting to this

Comment 10

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

Attachment: (Part 1) - Add an Image subclass for dynamically-created images

This patch got build failures on B2G because the 'using' directives needed a tweak. Not sure why this *doesn't* fail on most platforms!

Comment 11

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

Thanks for the review, David. This can land if B2G is green in this try job:

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

Comment 12

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

Attachment: (Part 1) - Add an Image subclass for dynamically-created images

Sigh. Fixing one set of problems revealed new problems. I'm pretty sure the difference in behavior here is that the B2G builds aren't running in unified mode.

Hopefully all the needed headers are present now. Another try push:

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

Comment 13

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

Attachment: (Part 1) - Add an Image subclass for dynamically-created images

Final rebase before landing.

Comment 14

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

Attachment: (Part 2) - Use dynamic images in the implementation of -moz-element

Final rebase before landing.

Comment 15

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

Attachment: (Part 3) - Remove nsLayoutUtils::DrawPixelSnapped, since it's dead code now

Final rebase before landing.

Comment 16

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

Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab8ac3c0de7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe38d8c189e5
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1d7b63dc997

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ab8ac3c0de7f
https://hg.mozilla.org/mozilla-central/rev/fe38d8c189e5
https://hg.mozilla.org/mozilla-central/rev/f1d7b63dc997