[Webkit-unassigned] [Bug 27561] ImageDecoder enhancements for WINCE port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27561





--- Comment #3 from George Staikos <staikos at kde.org>  2009-07-22 14:49:07 PDT ---
(In reply to comment #2)
> A few comments:
> 
> * Can you break this into distinct patches for the different pieces here?  This
> patch is very large and hard to review.
> 
> * You don't include ImageFrameSink.*, so no one can actually compile or use
> this.

   Those are asking the opposite things...  It was broken up to do the sink
part separately.

> * "#if PLATFORM (WINCE)" and similar all over, especially with "#else <existing
> code>", really sucks for readability.  Making this worse is the fact that
> frequently the two blocks of code are very similar or even share many common
> parts, and differ only slightly.  A combination of helper functions and
> subclasses seems like it could do this much more readably.
> 
> * goto?  Really?  It doesn't look necessary at all (a helper function plus a
> "continue;" would have served just as well).
> 
> Overall it seems like these capabilities should be added to the existing code
> in a way that blends more seamlessly and is usable by any port.  If I can
> manage to get back to working on Image system cleanup, maybe we can find a way
> of doing that stepwise.

  I think it needs more cleanup too.  I don't think we have any intention of
making our code potentially less fast for the sake of readability though, and
we don't want to risk breaking anything for others.  There is a balancing act
here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.