[poppler] Releasing RC1 one week later

[Carlos Garcia Campos]

El vie, 07-03-2008 a las 20:03 +1100, Brad Hards escribió:
> On Monday 03 March 2008 09:25:10 pm Carlos Garcia Campos wrote:
> > I've started to look at the optional content stuff and I've realized
> > there are some things  implemented in the qt4 frontend that should be in
> > the core, like the order array and radio buttons stuff. I've
> > changed/fixed other minor issues. Bradh, could you have a look at these
> > patches, please?
> >
> > http://people.freedesktop.org/~carlosgc/poppler-optcontent.diff
> I haven't tried / tested any of them, but I'll still offer my opinion, based 
> on initial review.

we can just keep both interfaces, getOCGs() returns the
optionalContentGroups and getOrderArray() the array object, and
getOrderedOCGs or something like that would return the list of optional
content already ordered. 

> >From 82f3d80caed7a5d42eab4328d44fc9e7afa0c4e4 Mon Sep 17 00:00:00 2001
> From: Carlos Garcia Campos <carlosgc at gnome.org>
> Date: Sun, 2 Mar 2008 12:07:46 +0100
> Subject: [PATCH] Fix some memory leaks and error handling in optional content
> This one is OK.  I thought I needed xref, but if you can make it work OK 
> without it, I think it is a good change.
> 
> 
> 
> >From 79ee040726e33a190cfad7293ee97bfed3cc7b74 Mon Sep 17 00:00:00 2001
> From: Carlos Garcia Campos <carlosgc at gnome.org>
> Date: Sun, 2 Mar 2008 13:07:50 +0100
> Subject: [PATCH] Code cleanups in optional content
> This one is OK, except for this part:
> -  GooString* name() const;
> +  GooString* getName() const { return m_name; }
>  
> -  Ref ref() const;
> -  void setRef(const Ref ref);
> +  Ref getRef() const { return m_ref; }
> +  void setRef(const Ref ref) { m_ref = ref; }
>  
> -  State state() { return m_state; };
> -  void setState(State state) { m_state = state; };
> +  State getState() { return m_state; }
> +  void setState(State state) { m_state = state; }
> The foo() -> getFoo() rename is fine (not standard for Qt, but looks more 
> standard for poppler),

yes, that's consistent with poppler core code. 

>  but please don't put the implementation in the header.
> 

ok, no problem, but just out of curiosity, why?

> 
> 
> >From 758391d3342489c329a1734a18a2736f0743c583 Mon Sep 17 00:00:00 2001
> From: Carlos Garcia Campos <carlosgc at gnome.org>
> Date: Sun, 2 Mar 2008 19:23:25 +0100
> Subject: [PATCH] Return optional content items in getOCGs already ordered 
> based on the order array
> Don't do this - see my previous discussion about building the tree structure 
> twice.
> 
> 
> 
> >From 16eab98505150042f6bd8041f3bdef99ff880ac7 Mon Sep 17 00:00:00 2001
> From: Carlos Garcia Campos <carlosgc at gnome.org>
> Date: Sun, 2 Mar 2008 19:43:15 +0100
> Subject: [PATCH] Use an array instead of a list for optional content groups
> Sorry, I don't understand what you are trying to accomplish with this. Seems 
> gratuitous. Can you explain more?

We don't really need a list when we already know the size of the list.
An array it's much more efficient than a list and it's also more
consistent with the poppler code. 

> 
> 
> >From 157d763c3e0e5432cce4da391fab99d0854a63f5 Mon Sep 17 00:00:00 2001
> From: Carlos Garcia Campos <carlosgc at gnome.org>
> Date: Sun, 2 Mar 2008 19:52:37 +0100
> Subject: [PATCH] Fix memory leaks
> I don't see the leaks. Again, can you explain why this change is needed?

m_orderArray and m_rBGroupsArray are never freed. By using objects
instead of pointers you avoid allocations/deallocations too. 

> Brad
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler

-- 
Carlos Garcia Campos
   elkalmail at yahoo.es
   carlosgc at gnome.org
   http://carlosgc.linups.org
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada
	digitalmente
Url : http://lists.freedesktop.org/archives/poppler/attachments/20080308/f46bc8e0/attachment.pgp