Summary: | Add a comment in order to clarify why BitmapImage::frameHasAlphaAtIndex returns true as default. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dongseong Hwang <dongseong.hwang> | ||||||||
Component: | Images | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, eric, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Dongseong Hwang
2012-07-03 04:14:29 PDT
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. |