[webkit-reviews] review requested: [Bug 26460] Cross-platform ICO decoder should return all icons in file as separate frames, sorted by decreasing quality : [Attachment 31526] part three

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 18 18:37:39 PDT 2009


Peter Kasting <pkasting at google.com> has asked  for review:
Bug 26460: Cross-platform ICO decoder should return all icons in file as
separate frames, sorted by decreasing quality
https://bugs.webkit.org/show_bug.cgi?id=26460

Attachment 31526: part three
https://bugs.webkit.org/attachment.cgi?id=31526&action=review

------- Additional Comments from Peter Kasting <pkasting at google.com>
This is the bulk of the work -- switch from inheritance to composition (so that
in the future ICOImageDecoder can decode several different BMPs within one
file) and move to lazy decoding like the decoders are supposed to do.

Even though the nitpicky details about how to decode (e.g. structs to look
through, offsets within them, how to decode individual pixels, etc.) have not
changed, this patch is still pretty hard to review.  Sorry, I don't know how to
make it simpler easily :/.  It _might_ be possible to just do composition first
and then lazy decoding as a second pass, but they're so intertwined that it'd
take a lot of work, I'd immediately throw all that work away, and I'm not sure
the separated patches would be more comprehensible.

Note that a lot of the diff here is still "mechanical" despite the last patch,
like switching to the new, simpler-to-use form of the readUintX() wrappers.


More information about the webkit-reviews mailing list