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

[bugzilla-daemon at webkit.org]

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





--- Comment #41 from Yong Li <yong.li at torchmobile.com>  2009-08-14 13:07:05 PDT ---
(In reply to comment #40)
> (In reply to comment #39)
> > (In reply to comment #38)
> > > (In reply to comment #37)
> > > > (In reply to comment #30)
> > > 
> I looked at most of the patches and missed any ImageFrameSinkWince class.  It
> looks in this patch like you have an ImageFrameSinkWince.cpp, but it's just
> filling out the functions of ImageFrameSink and doesn't do any subclassing.
> 
> Maybe I'm missing something.

Hm... Sorry, ImageFrameSinkWince has been removed by refactoring... too long
story. But createInstance and deleteInstance are already added, providing
ability to subclass ImageFrameSink, which should be fine.

> 
> > > I don't understand.  I know how GIF animation and frame disposal methods work. 
> > > The existing ports handle them correctly without using this mechanism.  Why
> > > doesn't the existing mechanism work for you, and how do you deal with not
> > > having the frame-clearing optimization for large GIFs?  I would think on a
> > > memory-constrained platform that'd be even more important than it is on
> > > desktops.
> > 
> > I've fixed many bugs on it, and our browser runs OK. Let me review this and get
> > back to you. I did that so long time ago :)
> 
> Try cases like http://www.aintitcool.com/files/HoD2.gif and
> http://img2.abload.de/img/almost_failedovs.gif and make sure that:
> * Images animate in the same time as Chrome 2/Safari 4 (not slower)
> * You don't blow out memory or CPU usage

Thanks for providing this test case. I'm confident our browser won't crash :)
Will try it
> 
> My object is to reduce the complexity and special-casing of shared code,
> especially the decoders themselves.  Let's say platform X supports native image
> storage in various formats (565, 888, 8888, etc.).  The pattern that makes
> sense to me is:
> 
> Image Decoder reads the pixel values out of the source image and calls setRGBA
> with them
> ImageFrameSink/RGBA32Buffer/whatever passes these values along to the backend's
> underlying NativeImage functions
> These functions decide how to downsample to an appropriate format
> 
> With your existing code, the downsampling would need to be done in the decoder,
> which would need to check whether the image frame claims to be 16-bit.  All
> that seems to do is cause us to plumb more state up through shared code and
> push the complexity on everyone.

I think you make a good point here.

> 
> > > > > Why does getFrame() return 0 when the status is frameEmpty?  An empty frame is
> > > > > still a valid one.
> > > > 
> > > > why? Who likes to waste CPU time on empty frame?
> > > 
> > > What CPU is being wasted?  We don't blit anything with empty frames.
> > 
> > So why cannot it return 0?
> 
> Practically, it can; there are only two reasons I wouldn't do it:
> * The API is lying
> * A backend can't read any data off a NULL pointer, such as the size of the
> frame, if it wants it
> 
> If it makes no difference, I don't see why we should lie to callers.  It's not
> a big deal, it just seems like a waste to explicitly check for an empty frame
> so we can lie.

The object itself has size and all information. getFrame() just return a
platform image, which can be null when it doesn't want to return a valid image.
for example, failed to allocated memory for it.

> 
> > > > > The proper use of setCanFreeBuffer() is not obvious to me.
> > > > 
> > > > This nofities the buffer that decoder has finished writting to the buffer, so
> > > > the buffer can be released when it's under memory pressure.
> > > 
> > > Isn't this the same signal as setting the status to FrameComplete?
> > 
> > FrameComplete is just for a single frame. But GIF decoder may still want to
> > read a finished frame when it processes the next one. 
> 
> So it's _not_ about the decoder being finished writing to the buffer.  It's
> some signal that the decoder is finished reading the buffer too.  In that case
> it seems like a bug that you don't finishBitmap() when we go to FrameComplete.

finisheBitmap() is called when ImageSource is taking the bitmap.

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