Bug 28167 – WINCE PORT: Implement ImageBuffer

• Component: Platform
• Status: RESOLVED FIXED

[Yong Li]

separated from bug 27511

[Yong Li]

Created attachment 34519 [details]
the patch

modified according to Eric's suggestion

[Eric Seidel (no email)]

Comment on attachment 34519 [details]
the patch

You mentioned in the other bug, that BufferedImage could be implemented by BitmapImage by implementing a BitmapImage(NativeImagePtr) constructor.  Why would we not want to do that?  I expect you could encounter later porting problems being the only port with a special BitmapImage-like-thing.

[Yong Li]

(In reply to comment #2)
> (From update of attachment 34519 [details])
> You mentioned in the other bug, that BufferedImage could be implemented by
> BitmapImage by implementing a BitmapImage(NativeImagePtr) constructor.  Why
> would we not want to do that?  I expect you could encounter later porting
> problems being the only port with a special BitmapImage-like-thing.

Because I think that is ugly. BitmapImage is too powerful. It is designed to receive encoded image data, decode the data with ImageSource, display the frames even they are just partially decoded, and animate with multiple frames and correct time intervals...

All of these are not necessary for ImageBuffer. ImageBuffer just holds a single-frame flat decoded buffer (or platform native image). That's why ImageBuffer::image() returns Image* but not BitmapImage*, because BitmapImage is much more than necessary.

So I think other ports should also do the similiar thing: using a light class like BufferedImage. It is very simple, and doesn't required too much coding.

[Yong Li]

Furthermore, I think the best way is let ImageBuffer be a subclass of Image. It's just used for rendering purpose

[Eric Seidel (no email)]

ImageBuffer and Image are used for different things.  ImageBuffer is a bag of bytes, designed to store image data, and used for things which need direct access to bit maps.  Image is an opaque image pointer which could be backed by an ImageBuffer, or any other sort of storage.  Image might also not even be a bitmap image (in the case of SVG or PDF), so it might not have a fixed size.

[Yong Li]

Created attachment 34765 [details]
updated

updated to implement premultiplied getting and putting image data

[Adam Barth]

Comment on attachment 34765 [details]
updated

Why is BufferedImage specific to WINCE?  That seems like a general concept that should be factored out and shared with the other ports if it's useful.  Also, it violates the "one class per file" rule.

[Yong Li]

(In reply to comment #7)
> (From update of attachment 34765 [details])
> Why is BufferedImage specific to WINCE?  That seems like a general concept that
> should be factored out and shared with the other ports if it's useful.  Also,
> it violates the "one class per file" rule.

Is this the reason for r-?

Where's the "one class per file" rule? If there is, I can find hundreds of violations.

Why is contributing to webkit so difficult?

[George Staikos]

The approach we've taken is that if something isn't 100% clearly a benefit to all platforms, we kept it as WINCE and then left it possible to move it to an ENABLE(feature) or unconditional code later.

[Adam Barth]

Comment on attachment 34765 [details]
updated

Based on the webkit-dev discussion and George's comment above, this patch is probably fine.  I'm not in love with this code

+ *dst++ = static_cast<unsigned char>((red * alpha + 254) / 255);

but I think it's ok for a first pass.

Thanks for your patience.

[Yong Li]

Thanks a lot. sorry for my languages

[Eric Seidel (no email)]

Comment on attachment 34765 [details]
updated

Rejecting patch 34765 from commit-queue.  This patch will require manual commit.

['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1

[Adam Barth]

Comment on attachment 34765 [details]
updated

Clearing flags on attachment: 34765

Committed r47968: <http://trac.webkit.org/changeset/47968>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.