1070426 - Defer imgFrame::SetHasNoAlpha() until we unlock the imgFrame

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

Just like in bug 1069652, I'm trying to address issues that arise from imgFrame objects being shared by multiple RawAccessRefs. It seems wise to defer imgFrame::SetHasAlpha() until we unlock, just as we did with imgFrame::Optimize(), since it also replaces mImageSurface.

Comment 1

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

Attachment: Defer imgFrame::SetHasNoAlpha() until we unlock

Here's the patch.

Comment 2

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8492490 [details] [diff] [review]
Defer imgFrame::SetHasNoAlpha() until we unlock

Seth asked me to steal this on irc.

Comment 3

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

Thanks Timothy! Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b22cf1d48af

Comment 4

[Michael Wu [:mwu]]

FWIW, I didn't get to this because SetHasNoAlpha() is a fairly sketchy call and I was thinking about the right way to go about this. In most cases, I think the decoder should know whether alpha is necessary before outputting decoded data, so this sort of information should be passed on and handled while initializing an imgFrame. Dealing with it after decoding is somewhat backwards. I'm planning to work on decoders later though, so I'll try to kill SetHasNoAlpha later.

Comment 5

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

(In reply to Michael Wu [:mwu] from comment #4)
> In most cases, I
> think the decoder should know whether alpha is necessary before outputting
> decoded data, so this sort of information should be passed on and handled
> while initializing an imgFrame.

Yes, that would definitely be preferable; the decoder should provide that information to RasterImage::EnsureFrame.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2b22cf1d48af