Closed Bug 1215763 Opened 4 years ago Closed 4 years ago

get rid of nsAuto*Ptr usage in image/

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(3 files)

No description provided.
These turned up when grepping around for nsAutoPtr; it seemed easier to
remove them as a first step.
Attachment #8675213 - Flags: review?(seth)
These conversions should be straightforward, but we have to add a .get
for nsExpirationTracker::Iterator.
Attachment #8675214 - Flags: review?(seth)
The wrinkle here is that while we can pass a UniquePtr to
ConvertHostARGBRow, we can't pass UniquePtr to
EncodeImageDataRow{24,32}, as the latter get called with raw pointers
out of our control.  Passing &row[0] seemed nicer than calling .get().
Attachment #8675215 - Flags: review?(seth)
Attachment #8675213 - Flags: review?(seth) → review+
Attachment #8675214 - Flags: review?(seth) → review+
Comment on attachment 8675215 [details] [diff] [review]
part 3 - s/nsAutoArrayPtr/UniquePtr/ in nsBMPEncoder

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

Looks good; glad to be rid of nsAutoPtr!

::: image/encoders/bmp/nsBMPEncoder.cpp
@@ +200,5 @@
>      // BMP requires RGBA with post-multiplied alpha, so we need to convert
>      for (int32_t y = mBMPInfoHeader.height - 1; y >= 0 ; y --) {
>        ConvertHostARGBRow(&aData[y * aStride], row, mBMPInfoHeader.width);
>        if(mBMPInfoHeader.bpp == 24) {
> +        EncodeImageDataRow24(&row[0]);

I don't feel strongly, but we are using |.get()| elsewhere in ImageLib, so for consistency it might be better to just use it here too. It also feels a bit cleaner to me.
Attachment #8675215 - Flags: review?(seth) → review+
Thanks for the reviews!

(In reply to Seth Fowler [:seth] [:s2h] from comment #4)
> > +        EncodeImageDataRow24(&row[0]);
> 
> I don't feel strongly, but we are using |.get()| elsewhere in ImageLib, so
> for consistency it might be better to just use it here too. It also feels a
> bit cleaner to me.

That's fine, in similar cleanups for gfx/, jrmuizel and vladv both preferred .get(), so at least we're consistent across the tree!
You need to log in before you can comment on or make changes to this bug.