[Webkit-unassigned] [Bug 184418] ImageFrame type used by non-Cocoa image decoder should not be the same as that used by ImageSource

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 9 11:12:42 PDT 2018


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

--- Comment #14 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
(In reply to Antti Koivisto from comment #12)
> > I tried very hard to merge both in one class named ImageFrame; see
> > <http://trac.webkit.org/changeset/206156>. My goal, which I did not have
> > time to do till now, was to have a single cached ImageFrame for all ports
> > and remove the second copy which the non-Cocoa image decoder still keep.
> > 
> > And now, you are making two implementations of ImageFrame again :).
> 
> I don't understand why. The types share almost nothing of any relevance.
> Even the actual image data is kept in m_backingStore in the decoder case and
> in m_nativeImage in ImageSource case. 
> 

When ImageSource asks the non CG decoder for image metadata or the NativeImage, the decoder checks first its cache. If the metadata or the NativeImage is not available, it calculates it only once and cache it in the ImageFrame. Then it sends it to the ImageSource which caches it again in the other ImageFrame. All the members which are surrounded by #if !USE(CG) and #endif are shared and used by the decoder and the ImageSource. This means all these data members are shared and have the same meaning:

    DecodingStatus m_decodingStatus { DecodingStatus::Invalid };
    IntSize m_size;

    NativeImagePtr m_nativeImage;
    SubsamplingLevel m_subsamplingLevel { SubsamplingLevel::Default };
    DecodingOptions m_decodingOptions;

    ImageOrientation m_orientation { DefaultImageOrientation };
    Seconds m_duration;
    bool m_hasAlpha { true };

> It makes it very difficult to make any changes to BitmapImage code as some
> platform specific decoder always breaks.

What are you trying to achieve? What difficulties does this merge prevent you from doing?

> I think the refactoring to merge the types was misguided.

If you explain your goal and how the double caching can be solved in the future this will help me understand the purpose of this patch. I am just afraid that I or somebody else comes in the future may say the same thing about it: "I think the refactoring to split the ImageFrame was misguided."

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180409/33303dc2/attachment-0002.html>


More information about the webkit-unassigned mailing list