391295 - APNG blending mode APNG_BLEND_OP_SOURCE not implemented

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P4)

Description

[Justin Dolske [:Dolske]]

While working on my APNG editor, I found that I wasn't able to get this blend mode to work. I found a note on Andrew Smith's APNG page (http://littlesvr.ca/apng/) which indicated this hadn't been implemented, and just confirmed in email that it's still the case.

This flag is needed to help optimize APNG encoding. I have a script for the editor which will compare a frame with the previous frame, and crop to the bounding box of the differences. By using APNG_DISPOSE_OP_NONE with APNG_BLEND_OP_SOURCE, this can significantly reduce the size of some animations. (In one test, from 436K to 104K).

Encoding and decoding with this blend mode works, but yields the wrong result... It looks the same as APNG_BLEND_OP_OVER. I'd guess that the code responsible for compositing each frame for display isn't checking for this flag.

Comment 1

[Stuart Parmenter]

I'd like to get this in and it should be pretty straight easy..  Andrew or Alfred any chance you guys could take a look at this?

Comment 2

[Andrew Smith]

I'll work on it.

Comment 3

[Andrew Smith]

Attachment: fix

The bulk of the fix is in imgContainer::DrawFrameTo().

Tested on Linux only, shouldn't be a problem on other platforms either. Here are a couple of samples to test:

http://littlesvr.ca/apng/images/keepandnoblend.png (APNG_BLEND_OP_OVER)
http://littlesvr.ca/apng/images/keepandblend.png (APNG_BLEND_OP_SOURCE)

Comment 4

[Andrew Smith]

Oh I forgot to mention it would be nice if there was a way to reset the cairo operator to the default, so use of defaultOperator in the patch woulnd't be needed. I couldn't find such a function.

Comment 5

[Andrew Smith]

Attachment: fix

oops, that was the wrong patch.

Comment 6

[Stuart Parmenter]

Comment on attachment 282351 [details] [diff] [review]
fix

i do'nt think the change to frameDisposalMethod in gfxIImageFrame is needed is it?  Since you're adding a new blendMethod attribute you need to generate a new UUID for the interface.

the rest looks fine

Comment 7

[Andrew Smith]

Attachment: fix

Same fix with both concerns from comment #6 addressed.

Comment 8

[Stuart Parmenter]

it would be nice to get this in for beta as it will be the first big release with APNG support.  low risk, good upside, but not critical.

Comment 9

[Damon Sicore (:damons)]

Comment on attachment 285167 [details] [diff] [review]
fix

Clear to land for M9.  Please do so ASAP.

Comment 10

[Reed Loden [:reed]]

Attachment: unbitrotten patch

unbitrotten patch for check-in

Comment 11

[Reed Loden [:reed]]

Checking in gfx/idl/gfxIImageFrame.idl;
/cvsroot/mozilla/gfx/idl/gfxIImageFrame.idl,v  <--  gfxIImageFrame.idl
new revision: 1.18; previous revision: 1.17
done
Checking in gfx/src/shared/gfxImageFrame.cpp;
/cvsroot/mozilla/gfx/src/shared/gfxImageFrame.cpp,v  <--  gfxImageFrame.cpp
new revision: 1.42; previous revision: 1.41
done
Checking in gfx/src/shared/gfxImageFrame.h;
/cvsroot/mozilla/gfx/src/shared/gfxImageFrame.h,v  <--  gfxImageFrame.h
new revision: 1.13; previous revision: 1.12
done
Checking in modules/libpr0n/decoders/png/nsPNGDecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp,v  <--  nsPNGDecoder.cpp
new revision: 1.73; previous revision: 1.72
done
Checking in modules/libpr0n/public/imgIContainer.idl;
/cvsroot/mozilla/modules/libpr0n/public/imgIContainer.idl,v  <--  imgIContainer.idl
new revision: 1.12; previous revision: 1.11
done
Checking in modules/libpr0n/src/imgContainer.cpp;
/cvsroot/mozilla/modules/libpr0n/src/imgContainer.cpp,v  <--  imgContainer.cpp
new revision: 1.58; previous revision: 1.57
done