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

[bugzilla-daemon at webkit.org]

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





------- Comment #40 from pkasting at google.com  2009-06-15 10:17 PDT -------
(In reply to comment #36)
> Whatever the merits of the recent changes I think Holger's objections based on
> a fundamental design problem should be given due attention and resolved before
> proceeding.

I have spent over twenty hours in the past two days trying to respond to
Holger's concerns.  If I have not given them "due attention" it has certainly
not been for lack of trying.

While I tend to agree with Kevin Ollivier's recent comment that this is more an
issue of semantics that cannot truly be resolved with facts, I will do my best
to elaborate my responses to anything technical I can discern:

As far as I understand it the specific concern raised is that RGBA32Buffer
should use a Vector<unsigned> as its low-level storage type.  The reason for
this seems to be:
(1) That's what it was doing before
(2) This is sufficient for all ports

My answer to this technical concern was that (1) is not really a justification
for anything and (2) is not accurate.  Vector<unsigned> is not usable by the
Skia port today, which is the primary concern; it is in my opinion
inappropriate for the wx port long term and not the best design for the Cairo
port, though these are secondary issues.  (I was about to write up some patches
which would, in fact, convert these ports to using a different backing store,
so the question of "future" work here was not very abstract.)

There was an additional concern raised that other ports now needed to do extra
work to use the image decoders.  I don't believe this is a practical objection
as I am aware of no other ports making such an effort.  I don't feel a design
should be based on elevating the concerns of hypothetical other ports over the
real needs of existing ports.

There was an additional concern raised that using NativeImagePtr from
ImageDecoder.h is a "layering violation".  I have worked with the image
decoders for several years now and have never been made aware of any layering
requirement to avoid this type.

Both sides expressed interest in merging the functionality of RGBA32Buffer and
ImageBuffer and the proposed difference in action seemed solely to be, whether
or not to revert some changes in ImageDecoder.* before doing this work.  Given
that I see no gain from doing this, I don't understand how this is anything
other than a time cost and a hindrance to doing the work.

In summary, I believe the technical concerns raised were mainly that the work
done on bug 25709 decreased the "cross-platformness" of ImageDecoder and
related classes, to which my response was that these were certainly not
sufficiently cross-platform before to account for all platforms, but now are;
and that the changes made to accommodate ports like Skia were not harmful to
other ports (and thus should be backed out and rethought) but instead helpful
(now, for minor things like removing ImageSourceXXX.cpp files, and in the
future, for writing clearer or more efficient backend storage functions).  As
far as I know Holger continues to disagree with me on this and I do not know
how to further resolve this disagreement, which is why I proposed attempting to
reach a state where ImageBuffer and RGBA32Buffer are merged as quickly as
possible.

I hope this is helpful.


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