Bug 8921 - Use WebCore to render full-frame images
Summary: Use WebCore to render full-frame images
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-15 10:36 PDT by Anders Carlsson
Modified: 2006-05-16 02:26 PDT (History)
0 users

See Also:


Attachments
Implement ImageDocument (69.01 KB, patch)
2006-05-15 11:54 PDT, Anders Carlsson
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2006-05-15 10:36:47 PDT
Currently this is handled by WebKit
Comment 1 Anders Carlsson 2006-05-15 11:54:40 PDT
Created attachment 8325 [details]
Implement ImageDocument
Comment 2 Darin Adler 2006-05-15 12:08:42 PDT
Comment on attachment 8325 [details]
Implement ImageDocument

Maciej says he's going to review this and I don't want to steal his thunder.

But I will point out that all or most of WebImageData.h/m, WebImageDecodeItem.h/m, WebImageDecoder.h/m, WebImageRenderer.h/m, and WebImageRendererFactory.h/m can be deleted too.
Comment 3 Maciej Stachowiak 2006-05-15 18:15:42 PDT
I see no actual bugs, but here's a few optional style suggestions:

You could conside factoringing the first-time initialization in this function into a separate function:

+bool ImageTokenizer::writeRawData(const char *data, int len)

This could use a FIXME for things that won't work on non-Apple platforms:

+void ImageTokenizer::finish()

It might be nice to encapsulate the choice of whether to use the decoder and feed raw data a bit better than an "if" in the write function - perhaps a separate function for this, or let the tokenizer manage the decoder, or something.

+    if (Tokenizer* t = d->m_doc->tokenizer()) {

Also, as Darin mentioned, it would be nice to remove all the image support code from WebCoreSupport, now that you are removing the last use of it.

Even with all these minor comments, r=me!