Add an assertion and a comment in order to clarify the states of BitmapImage.
Created attachment 150577 [details] patch
Comment on attachment 150577 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=150577&action=review > Source/WebCore/platform/graphics/BitmapImage.cpp:312 > + ASSERT(m_frames[index].m_isComplete); All case that call this method is guarded by if(isComplete). > Source/WebCore/platform/graphics/BitmapImage.cpp:326 > + // black. The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index)
Created attachment 150579 [details] patch v.2
Comment on attachment 150579 [details] patch v.2 View in context: https://bugs.webkit.org/attachment.cgi?id=150579&action=review > Source/WebCore/platform/graphics/BitmapImage.cpp:322 > + // When a frame has not finished decoding, always mark it as having alpha. The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index)
(In reply to comment #0) > Add an assertion and a comment in order to clarify the states of BitmapImage. I removed the assertion in patch v.2 because I misunderstood at the time to patch v.1.
(In reply to comment #4) > (From update of attachment 150579 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150579&action=review > > > Source/WebCore/platform/graphics/BitmapImage.cpp:322 > > + // When a frame has not finished decoding, always mark it as having alpha. > > The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index) The comment of ImageSource::frameHasAlphaAtIndex is long, so I pasted just summary of the comment to BitmapImage::frameHasAlphaAtIndex. bool ImageSource::frameHasAlphaAtIndex(ImageFrame* buffer) { // When a frame has not finished decoding, always mark it as having alpha. // Ports that check the result of this function to determine their // compositing op need this in order to not draw the undecoded portion as // black. // TODO: Perhaps we should ensure that each individual decoder returns true // in this case. return !frameIsCompleteAtIndex(buffer) || buffer->hasAlpha(); }
Comment on attachment 150579 [details] patch v.2 View in context: https://bugs.webkit.org/attachment.cgi?id=150579&action=review >>> Source/WebCore/platform/graphics/BitmapImage.cpp:322 >>> + // When a frame has not finished decoding, always mark it as having alpha. >> >> The comment was copied from ImageSource::frameHasAlphaAtIndex(size_t index) > > The comment of ImageSource::frameHasAlphaAtIndex is long, so I pasted just summary of the comment to BitmapImage::frameHasAlphaAtIndex. > > bool ImageSource::frameHasAlphaAtIndex(ImageFrame* buffer) > { > // When a frame has not finished decoding, always mark it as having alpha. > // Ports that check the result of this function to determine their > // compositing op need this in order to not draw the undecoded portion as > // black. > // TODO: Perhaps we should ensure that each individual decoder returns true > // in this case. > return !frameIsCompleteAtIndex(buffer) || buffer->hasAlpha(); > } You should add a line that said: // See ImageSource::framehasAlphaAtIndex for explanation of why incomplete images claim to have alpha.
Created attachment 150704 [details] patch v.3 Absolutely!
Comment on attachment 150704 [details] patch v.3 Clearing flags on attachment: 150704 Committed r121827: <http://trac.webkit.org/changeset/121827>
All reviewed patches have been landed. Closing bug.