[Webkit-unassigned] [Bug 26467] New: Cross-platform image system architecture cleanup metabug

[bugzilla-daemon at webkit.org]

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

           Summary: Cross-platform image system architecture cleanup metabug
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Images
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: pkasting at google.com
                CC: eric at webkit.org, hyatt at apple.com,
                    kevino at theolliviers.com, zecke at selfish.org,
                    treat at kde.org, bfulgham at webkit.org
 BugsThisDependsOn: 26460


This bug was filed at eseidel's request to have a clean slate to discuss some
of the issues originally raised on bug 26379.

(The following is quite long, but there has been enough discussion on this
issue that I want to try and be precise as to what's going on.)

The most prominent issue to be considered here is that RGBA32Buffer's name,
location, and implementation are all currently questionable.  RGBA32Buffer used
to be a Vector<unsigned> used by (to my knowledge) Cairo (i.e. Cairo/Win and
GTK+) and wx.  During Chromium development this was forked so the Skia backend
could store pixel data directly in an SkBitmap.  To unfork the Skia decoders on
bug 25709 I made RGBA32Buffer be an API around a platform-dependent storage
system, which at the moment is still a Vector<unsigned> for wx and Cairo but
could conceivably be e.g. a wxBitmap or cairo_surface_t or whatever.

Unfortunately, this means that "RGBA32" is a bit misleading, and ports that
want to switch to the cross-platform image decoders need to do extra work. 
Additionally, it may or may not be weird to have code in image-decoders/ that's
cross platform, when most such things are in graphics/.

Furthermore, the ImageBuffer class has been noted as another "platform-specific
low level pixel store", and as such, has at least some conceptual overlap with
both the original RGBA32Buffer and (perhaps even more) the current version.

As far as I can tell there is general agreement that these two classes should
combine into one (perhaps named "ImageFrame" or similar).  The actual storage
inside this class is less finalized.  Three proposals have been made:

(1) Store everything as a Vector<unsigned>, provide functions either in this
class or outside it (as friends or consumers or whatever) to adopt this pixel
data into a platform-specific storage class.  At least for Cairo and Skia this
can probably be done without copying (although the resulting object must not be
used past the life of the original pixel data, so care would need to be taken),
maybe not for wx.  This probably involves the least platform-specific code
overall.  This is closest to the old RGBA32Buffer implementation.

(2) Store everything in platform-specific storage, but provide any APIs
necessary to read/write data as needed by image decoders or other consumers. 
This has the fewest theoretical barriers to performance as ports can implement
operations directly, although it's not clear that there are large practical
differences; also, wx might not benefit much from this in cases where it lacks
sufficient APIs to operate on its own native surfaces (see bug 26379 comment
51).  This might be beneficial for special-purpose ports, e.g. a port on a
memory-constrained device that wished to always downsample to 16bpp, since that
port would never have to keep any frames in memory as 32bpp.  This is basically
what the current RGBA32Buffer implementation is, and perhaps ImageBuffer too
(but I haven't looked).

(3) Store things in platform-specific storage, but provide a default
implementation using Vector<unsigned>, for ports which don't want or need
anything else.  This is something of a hybrid of the above options; whether it
benefits anyone today probably depends on whether the Cairo or wx ports are
interested in using platform-specific storage soon, while whether it benefits
anyone in the future is hard to say.  It's possible this would provide an easy
"glide path" for future ports, although the set of APIs currently on
RGBA32Buffer is small and simple enough that I don't think there is much
practical difference.

I personally lean toward option (2), but option (3) would not be hard to write.

Of course, the exact details of the merged class, or even of merging with
ImageBuffer at all, are subject to change pending a deeper understanding of
ImageBuffer.

There is other work to be done here as well.  The ImageSource implementations
should probably merge, partly or completely, into one; the RGBA32Buffer APIs
should be changed to use more WebKit types; and in general the entire image
subsytem needs to be better-understood to the point where a complete design doc
can be made.  Ideally the CG port can be included here too, so the demarcation
of "CG versus other" can become clearer, and so the relevant classes can be
designed to accommodate not only CG but other ports too.  My (perhaps naive)
ideal is that someday there will be mostly just shared, cross-platform code for
boring bits like plumbing image frames between the decoders and the cache,
coupled to a single set of simple classes that do the actual low-level
per-platform storage.  Right now things are much more forked than this.


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