[Webkit-unassigned] [Bug 26379] Reconsider image decoding architecture/APIs

[bugzilla-daemon at webkit.org]

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





------- Comment #51 from kevino at theolliviers.com  2009-06-15 16:41 PDT -------
(In reply to comment #43)
> (In reply to comment #41)
> For wx the idea is similar except to use a wxBitmap instead of a
> cairo_surface_t.  The benefits are more concrete for wx as it is definitely not
> only copying the data when reading it out, but also flipping BGRA -> RGBA.

BTW, while I probably won't have time to delve too deeply into this over the
next couple days, I did want to mention a possible source of confusion if you
plan on looking at the wx port. 

As I mentioned in our discussion on IRC, the wx port has two different graphics
rendering APIs. There is wxDC, a bitmap-based API which uses GDI, GDK and
CoreGraphics APIs to draw, and wxGraphicsContext, a vector-based API which uses
GDI+, Cairo and CoreGraphics to draw. Use of wxGraphicsContext is designated in
wx code using #if USE(WXGC) blocks. Of course, whenever possible we want to use
wxGraphicsContext since WebCore::GraphicsContext is based around a vector-based
drawing API. Unfortunately, even if we use wxGraphicsContext, wxBitmap stores
the bitmap data using the native type expected by wxDC, rather than
wxGraphicsContext. This is why we do a conversion (and data copy, really) from
wxBitmap to wxGraphicsBitmap in RGBA32Buffer::asNewNativeImage, because when
USE(WXGC) is true (eventually always), we need the image data to be in a format
usable by wxGraphicsContext. To make matters worse, the APIs for types like
wxGraphicsBitmap are very minimal and essentially only allow conversion from
the standard wx data type (e.g. wxBitmap to wxGraphicsBitmap), so there's
currently no way to do things like get direct pixel access for reading or
writing with these types. 

So, unfortunately, even if we use wxBitmap as the image buffer type, we will
still have a copy operation to go from wxBitmap to wxGraphicsBitmap. My concern
performance-wise is whether or not we can continue to efficiently cache the
wxGraphicsBitmap type if the underlying wxBitmap buffer can change at any time.
e.g. we'd need a way to know when the buffer has been 'dirtied' and needs a new
wxGraphicsBitmap to be created from it, so that we're only creating new
wxGraphicsBitmap types when necessary and not any time a wxGraphicsBitmap is
being asked for. At the same time, we probably don't want to recreate it after
every write operation, so we'd probably need some mechanism to return a cache
if not dirty or recreate the image if so.

I realize this sucks, but it's what I've got to work with right now. :/ I can
look at extending the wxGraphicsBitmap API, of course, but it will take some
time to both implement and then have the change propagate into a release wx
version, so we'll need a workaround in place in the short term.

RE: Adam

> Techical arguments do not always require empirical data of the sort you are
> requesting.  Patches are often turned down based on technical reasons other
> than a lack of unit tests or benchmarks.

Yes, but we're discussing a patch that has already gone through the review
process and been approved, meaning that the person or persons who reviewed the
patch did not consider whatever flaws they saw as serious enough to outweigh
the benefit of getting the patch into the tree. (An assessment I'd agree with
in this case.) While no one is perfect, given that, I think as a practical
matter there should be a higher burden of proof to show that the flaws are
serious enough to warrant a revert. 

Anyway, I think things are going in right direction now and people are getting
on the same page, so it's probably a moot point now. :)


-- 
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.