Hello.

On Fri, 2014-08-08 at 11:29, Jean-Philippe André wrote:
> Hi~
> 
> 
> On Thu, Aug 7, 2014 at 9:49 PM, Stefan Schmidt <ste...@datenfreihafen.org>
> wrote:
> 
> > Hello.
> >
> > Still on my way through the API/ABI reports form 1.10 to 1.11
> >
> > On Fri, 2014-06-13 at 01:18, Jean-Philippe ANDRXX wrote:
> > > jpeg pushed a commit to branch master.
> > >
> > >
> > http://git.enlightenment.org/core/efl.git/commit/?id=c21968bcd76e07ff300c8e102c0062f74fcb704d
> > >
> > > commit c21968bcd76e07ff300c8e102c0062f74fcb704d
> > > Author: Jean-Philippe Andre <jp.an...@samsung.com>
> > > Date:   Fri Jun 13 16:56:39 2014 +0900
> > >
> > >     Eet: Add support for ETC2 encoding and decoding
> > >
> > >     Since we now have full support for ETC2, add the colorspaces
> > >     to Eet.
> > >
> > >     @feature
> > > ---
> > >  src/Makefile_Eet.am     |   2 +
> > >  src/lib/eet/Eet.h       |   8 +-
> > >  src/lib/eet/eet_image.c | 310
> > +++++++++++++++++++++++++++++++++++-------------
> > >  3 files changed, 237 insertions(+), 83 deletions(-)
> > >
> > >  lib_eet_libeet_la_CPPFLAGS = -I$(top_builddir)/src/lib/efl \
> > > diff --git a/src/lib/eet/Eet.h b/src/lib/eet/Eet.h
> > > index 50645a0..95be2e5 100644
> > > --- a/src/lib/eet/Eet.h
> > > +++ b/src/lib/eet/Eet.h
> > > @@ -477,14 +477,18 @@ typedef enum _Eet_Image_Encoding
> > >  {
> > >     EET_IMAGE_LOSSLESS = 0,
> > >     EET_IMAGE_JPEG = 1,
> > > -   EET_IMAGE_ETC1 = 2
> > > +   EET_IMAGE_ETC1 = 2,
> > > +   EET_IMAGE_ETC2_RGB = 3,
> > > +   EET_IMAGE_ETC2_RGBA = 4
> > >  } Eet_Image_Encoding;
> > >
> > >  typedef enum _Eet_Colorspace
> > >  {
> > >     EET_COLORSPACE_ARGB8888 = 0,
> > >     /* The number between are reserved to preserve compatibility with
> > evas */
> > > -   EET_COLORSPACE_ETC1 = 8
> > > +   EET_COLORSPACE_ETC1 = 9,
> >
> > Reported by the ABI checker and it seems valid to me. We should keep
> > it at 8 here and leave 9 empty or move all newly added ones one up.
> >
> > All all code that might comapre to this will be wrong now and we have
> > an ABI break. Agreed?
> >
> 
> I've seen this in the ABI break report. I thought a bit about it.
> It wasn't really a bug in the first place (having value 8 instead of 9),
> since the idea was to have the same values for EET_ colorspaces and EVAS_
> colorspaces but "someone" messed up the first time.
> 
> All in all these values should basically be used only from inside EFL
> (evas+eet cooperation), although it's possible to use the encoding/decoding
> APIs from a client app (they're EAPIs after all). As usual we'll blame
> Cedric for introducing a "bug" in the first place (b1e576081175835cfef1626).

I can see the pain of changing this.

> Not sure what to do.
> I'd rather fix this value now to keep consistency between evas and eet,
> it's been only 3 months this is here, and the only sane way to use these
> values is through Edje anyways.

To be honest my guts tell me that its still an ABI break and one never
knows who is using such thing when using EFL. Sure its less likely because
it normally is used through Edje and because it is only in
for one release.

I'm not making a call here because I will only be called bureaucrat
again. Bring it up with raster and see what he wants to do about it.

If this stays as is I would at least favour a comment for it
explaining that it was 8 during the three month of the 1.10 release
being out and now changed to 9 to align with other defines.

> PS: Thanks for reviewing!

Good to see its appreciated, sometimes.

regards
Stefan Schmidt

------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to