1291054 - Clean up some Decoder methods related to sizes, frame rects, and surface allocation

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

We're in the process of reworking how decoders interact with the surface cache (see bug 1291045 for the details for single-frame images). Before writing the more complicated patches, it'd be a good idea to do a little cleanup of the Decoder methods related to image size, frame rects, and surface allocation, since we'll be adding new code that interacts with those methods.

Comment 1

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

Attachment: (Part 1) - Clean up Decoder's size getters a little bit.

This is just some quick cleanup to improve comments and switch over to Moz2D
types.

Comment 2

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

Attachment: (Part 2) - Add a Decoder::OutputSize() getter and use it in the decoders.

This patch adds an OutputSize() method that returns the size of the surface that
the decoder is going to create. This lets us remove the formula for computing
this from a bunch of places.

Comment 3

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

Attachment: (Part 3) - Rename Decoder::GetSize() to Decoder::Size() to be consistent with the style guide.

Infallible getters aren't supposed to have |Get| in the name, per our style
guide. Let's rename GetSize() to Size() to make it match up. This also makes it
consistent with OutputSize() and TargetSize().

Comment 4

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

Attachment: (Part 4) - Add Decoder convenience methods for the common case of frame rects that cover the whole image.

Most decoders only use a frame rect that covers the entire image. Let's add some
convenience methods to calculate that in one place. This not only makes the code
a little shorter, but more importantly makes it more obvious what the code is
doing at callsites.

Comment 5

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

Attachment: (Part 5) - Remove Decoder::AllocateBasicFrame(), which is dead code.

Patch title says it all. Nothing calls AllocateBasicFrame(), which is a remnant
of the pre-downscale-during-decode days.

Comment 6

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8776739 [details] [diff] [review]
(Part 4) - Add Decoder convenience methods for the common case of frame rects that cover the whole image.

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

::: image/decoders/nsPNGDecoder.cpp
@@ -147,5 @@
>    // Check if the image has a transparent color in its palette.
>    if (aFormat == SurfaceFormat::B8G8R8A8) {
>      return TransparencyType::eAlpha;
>    }
> -  if (!IntRect(IntPoint(), Size()).IsEqualEdges(aFrameRect)) {

We can remove the |aFrameRect| arg as well.

Comment 7

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

Thanks for the quick reviews!

(In reply to Edwin Flores [:eflores] [:edwin] from comment #6)
> We can remove the |aFrameRect| arg as well.

That's a good point! I'll get that fixed.

Comment 8

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

(In reply to Edwin Flores [:eflores] [:edwin] from comment #6)
> We can remove the |aFrameRect| arg as well.

Oh wait, I misread it the same way you did. It's still used, alas.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e94e5cf47ef
(Part 1) - Clean up Decoder's size getters a little bit. r=edwin
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c5ceb44358d
(Part 2) - Add a Decoder::OutputSize() getter and use it in the decoders. r=edwin
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2f8694f12c8
(Part 3) - Rename Decoder::GetSize() to Decoder::Size() to be consistent with the style guide. r=edwin
https://hg.mozilla.org/integration/mozilla-inbound/rev/21260281d501
(Part 4) - Add Decoder convenience methods for the common case of frame rects that cover the whole image. r=edwin
https://hg.mozilla.org/integration/mozilla-inbound/rev/b617b61db290
(Part 5) - Remove Decoder::AllocateBasicFrame(), which is dead code. r=edwin

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5e94e5cf47ef
https://hg.mozilla.org/mozilla-central/rev/5c5ceb44358d
https://hg.mozilla.org/mozilla-central/rev/a2f8694f12c8
https://hg.mozilla.org/mozilla-central/rev/21260281d501
https://hg.mozilla.org/mozilla-central/rev/b617b61db290