Re: [GSOC] Removing avmedia dependency in drawinglayer

2012-06-28 Thread Rafael Dominguez
On Wed, Jun 27, 2012 at 5:58 AM, Thorsten Behrens 
t...@documentfoundation.org wrote:

 Rafael Dominguez wrote:
   Oops my bad, heres the patch attached
 
 Hi Rafael,

 nice improvement - two nits though:

  @@ -65,6 +67,8 @@ namespace drawinglayer
   /// discrete border (in 'pixels')
   sal_uInt32
  mnDiscreteBorder;
 
  +const Graphic   *mpSnapshot;
  +
 
 Parent classes of MediaPrimitive2D derive from boost::noncopyable -
 that means you don't have to worry about having to modify mpSnapshot,
 which in turn means it could be const Graphic instead.

 But it's even easier, since Graphic has copy-on-write behaviour -
 you can simply hold a Graphic by value in your primitive, it only
 contains a cheap ptr to an impl class, that gets, if necessary,
 copied  duplicated if you modify one of the many shared copies.

 (many bulky objects in LibO have that behaviour, including bitmaps,
 polygons, and metafiles (sorta))

 Cheers,

 -- Thorsten


Thanks for the input, i took your advice and changed it, gonna push it to
my branch then
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [GSOC] Removing avmedia dependency in drawinglayer

2012-06-27 Thread Miklos Vajna
Hi Rafael,

On Tue, Jun 26, 2012 at 05:27:13PM -0430, Rafael Dominguez 
venccsra...@gmail.com wrote:
 This patch remove avmedia dependency in drawinglayer to avoid a circular
 dependency, this is the most clean solution i came up with but i wanted to
 share it before applying it
 The other option was create a new lib drawinglayer_media with just that
 code because moving the class to svx didnt feel right.
 
 The only use of this class came from
 http://opengrok.libreoffice.org/xref/core/svx/source/sdr/contact/viewcontactofsdrmediaobj.cxx#152

Forgot to attach the patch? ;-)

Thanks,

Miklos
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [GSOC] Removing avmedia dependency in drawinglayer

2012-06-27 Thread Cedric Bosdonnat
Hi Rafael,

It seems that you forgot to attach the patch ;)

--
Cedric

On Tue, 2012-06-26 at 17:27 -0430, Rafael Dominguez wrote:
 This patch remove avmedia dependency in drawinglayer to avoid a
 circular dependency, this is the most clean solution i came up with
 but i wanted to share it before applying it
 The other option was create a new lib drawinglayer_media with just
 that code because moving the class to svx didnt feel right.
 
 The only use of this class came from
 http://opengrok.libreoffice.org/xref/core/svx/source/sdr/contact/viewcontactofsdrmediaobj.cxx#152
 ___
 LibreOffice mailing list
 LibreOffice@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/libreoffice


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [GSOC] Removing avmedia dependency in drawinglayer

2012-06-27 Thread Rafael Dominguez


 Oops my bad, heres the patch attached


0001-Remove-dependency-of-avmedia-in-drawinglayer.patch
Description: Binary data
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [GSOC] Removing avmedia dependency in drawinglayer

2012-06-27 Thread Michael Stahl
On 27/06/12 11:54, Rafael Dominguez wrote:
 
 Oops my bad, heres the patch attached

patch looks good, except for this, which should be a boost::scoped_ptr
so you don't need to delete it manually:

 +const Graphic   *mpSnapshot;

also, if the goal is to solve some cyclic dependency then you'll
probably need to remove avmedia from first line of
drawinglayer/prj/build.lst as well.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [GSOC] Removing avmedia dependency in drawinglayer

2012-06-27 Thread Thorsten Behrens
Rafael Dominguez wrote:
  Oops my bad, heres the patch attached

Hi Rafael,

nice improvement - two nits though:

 @@ -65,6 +67,8 @@ namespace drawinglayer
  /// discrete border (in 'pixels')
  sal_uInt32  mnDiscreteBorder;
  
 +const Graphic   *mpSnapshot;
 +

Parent classes of MediaPrimitive2D derive from boost::noncopyable - 
that means you don't have to worry about having to modify mpSnapshot,
which in turn means it could be const Graphic instead.

But it's even easier, since Graphic has copy-on-write behaviour - 
you can simply hold a Graphic by value in your primitive, it only
contains a cheap ptr to an impl class, that gets, if necessary,
copied  duplicated if you modify one of the many shared copies.

(many bulky objects in LibO have that behaviour, including bitmaps,
polygons, and metafiles (sorta))

Cheers,

-- Thorsten


pgptBziKp2SAz.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


[GSOC] Removing avmedia dependency in drawinglayer

2012-06-26 Thread Rafael Dominguez
This patch remove avmedia dependency in drawinglayer to avoid a circular
dependency, this is the most clean solution i came up with but i wanted to
share it before applying it
The other option was create a new lib drawinglayer_media with just that
code because moving the class to svx didnt feel right.

The only use of this class came from
http://opengrok.libreoffice.org/xref/core/svx/source/sdr/contact/viewcontactofsdrmediaobj.cxx#152
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice