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

Sun Jun 14 21:14:28 PDT 2009

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

------- Comment #25 from zecke at selfish.org  2009-06-14 21:14 PDT -------
(In reply to comment #23)
> (In reply to comment #20)
> > 3.  As for the EFL and Windows CE ports, these are basically Qt ports, so this
> > sounds akin to arguing that "pkasting broke the Qt port, and he also broke the
> > Qt port and the other Qt port to boot."

I don't even know where to start to get the record straight...

1.) EFL is using EFL... and not Qt...
2.) Torchmobile is a Windows CE port
3.) Me using image-decoders on Qt, yes is Qt related...

> Plus, I asked repeatedly both on bugs and IRC for input from Qt port
> maintainers to ensure that my changes didn't break Qt, and it was my impression
> that Qt _isn't_ broken.  None of the Torchmobile or EFL people have asked for
> help in any way.  The only bustage to Qt I'm aware of was the the temporary
> bustage about 48 hours ago that I _had already said I was planning to fix_ and
> that _was_ in fact worked around with a roughly 3-line patch (not a "115 line
> copy").

I don't event want to go in the process. Landing patches with a reviewer
raising his flag is not nice and the responses I got on irc are not making me
feel comfortable about Chromium at all. But leave that out for now.

> 
> These comments boil down to "you kept other ports working by copying a few
> functions' implementations temporarily.  THIS IS AN ENORMOUS PENALTY!"  I think
> it's clear that this isn't an "enormous penalty" or in fact much of any
> penalty, since it applies to exactly _two_ ports (not CG, the most widely used
> port as part of Safari; not Skia, the second most widely used port as part of
> Chromium; and not QT, the port I broke so apparently badly), and in fact _I_
> was the one that made the changes for those other ports, not those ports'
> maintainers (although kollivier on IRC last night said the changes "sounded
> easy" before I went ahead and wrote the wx patch, so even that wasn't a big
> deal).  I could alternately have provided an ImageDecoder.cpp that implemented
> these two ports' functions for things they have in common at the moment, but
> this seemed like just causing the ports additional hassle since as Brent said
> in comment 21 things are about to change even more.

In a way I fundamentally disagree with.

> In summary, Holger, you are arguing that I have caused great pain to wx and
> Cairo, but the only thing I've caused is my _own_ patches to implement some
> functions that are shortly going to differ between the two ports, the Cairo
> maintainer

which is alp and me as we inherited it and not brent!

> This isn't productive dialog at all.  Be reasonable and stop trying to use
> other ports as weapons against me when their maintainers disagree with you or
> are silent.

Sigh.

To summarize:

I disagree with the changes to RGBA32Buffer. I think Skia should move towards
making ImageBuffer suitable for use and nuke RGBA32Buffer all together or make
truly cross platform. It is no acceptable temporarily solution to create a copy
of clones (cairo, wx, Qt... whatever port)... it is moving in the wrong
direction and my patch is reinforcing it.

And second. There is no reason to change the default behavior (no reason to
#ifdef for CAIRO,WX), it should work for everyone (with eventually some
performance impacts):

   1.) sometimes the decoded image is just copied somewhere else
   2.) some graphics libraries (e.g. cairo) allow you to adopt the memory and
there will be exactly one conversion to image triggered the
BitmapImage::cacheFrame.

and again my patch is restoring sanity.

So from my point of view your changes shouldn't have landed as they are a step
back and not one ahead. The way ahead is to move the code over to ImageBuffer
which I'm very supportive of and until then I want the sanity of the old
solution back.

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