Organization of patches to blenders

[Vinnie-4]

> The patch can be found at http://www.ozonehouse.com/john/agg24_alvyraysmith.patch

This would be really convenient if it was a set of new classes instead of modification to existing classes, for those of us who have already made changes to the originals.


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Vector-agg-general mailing list
Vector-agg-general@...
https://lists.sourceforge.net/lists/listinfo/vector-agg-general

[John Horigan]

Sure, that would also be a good way to handle the small performance  
impact of using more accurate blenders. I would still want to move  
multiplying alpha and cover from the pixfmt class to the blender class.

-- john

On Aug 17, 2009, at 10:09 AM, Vinnie wrote:

>> The patch can be found at http://www.ozonehouse.com/john/agg24_alvyraysmith.patch
>
> This would be really convenient if it was a set of new classes  
> instead of modification to existing classes, for those of us who  
> have already made changes to the originals.
>
>
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008  
> 30-Day
> trial. Simplify your report design, integration and deployment - and  
> focus on
> what you do best, core application coding. Discover what's new with
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Vector-agg-general mailing list
> Vector-agg-general@...
> https://lists.sourceforge.net/lists/listinfo/vector-agg-general

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Vector-agg-general mailing list
Vector-agg-general@...
https://lists.sourceforge.net/lists/listinfo/vector-agg-general

[klaas.holwerda-3]

John Horigan wrote:
> Sure, that would also be a good way to handle the small performance  
> impact of using more accurate blenders. I would still want to move  
> multiplying alpha and cover from the pixfmt class to the blender class.
>  

I think we really must not duplicate many the classes because of this
patch. If people made changes, they should have supplied them as
patches, or do so soon, else the problem is theirs.
We made the Agg fork to start the development again, and that means
changes!

The patch corrects things that are wrong in Agg. And if people want to
keep the old way, very likely the that can be fixed with a simple #define.
So do you think you can make a patch like that John? I can imagine that
int_lerp int_mult etc. In two versions using a #define , will do the
trick already, since it is all inline.
I personally don't mind a small performance impact.

To conclude i suggest to wait to the END of the WEEK, if no serious
issues arise, apply the patch.

So those against, please speak up, and come with suggestion to improve
the patch. Or wait, and supply a patch later to improve Agg even more.

I volunteer to apply the patch, unless someone else wants to do it?
( i am not a SVN hero, but i think i manage).

Regards,

Klaas


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Vector-agg-general mailing list
Vector-agg-general@...
https://lists.sourceforge.net/lists/listinfo/vector-agg-general

[John Horigan]

I don't think that I can recreate the current blenders by redefining  
int_lerp. int_prelerp, etc. The existing blenders don't use a single  
method for multiplying color components or cover values. Many  
different tweaks are used to compensate for the error of treating  
divide by 256 as if it were divide by 255.

I would have to keep the existing code:

         static AGG_INLINE void blend_pix(value_type* p,
                                          unsigned cr, unsigned cg,  
unsigned cb,
                                          unsigned alpha,
                                          unsigned cover)
         {
             alpha = color_type::int_mult_cover(alpha, cover);
#ifdef AGG_Legacy_Blenders
             alpha = color_type::base_mask - alpha;
             cover = (cover + 1) << (base_shift - 8);
             p[Order::R] = (value_type)((p[Order::R] * alpha + cr *  
cover) >> base_shift);
             p[Order::G] = (value_type)((p[Order::G] * alpha + cg *  
cover) >> base_shift);
             p[Order::B] = (value_type)((p[Order::B] * alpha + cb *  
cover) >> base_shift);
             p[Order::A] = (value_type)(base_mask - ((alpha *  
(base_mask - p[Order::A])) >> base_shift));
#else
             cr = color_type::int_mult_cover(cr, cover);
             cg = color_type::int_mult_cover(cg, cover);
             cb = color_type::int_mult_cover(cb, cover);
             p[Order::R] = (value_type)
(color_type::int_prelerp(p[Order::R], cr, alpha));
             p[Order::G] = (value_type)
(color_type::int_prelerp(p[Order::G], cg, alpha));
             p[Order::B] = (value_type)
(color_type::int_prelerp(p[Order::B], cb, alpha));
             p[Order::A] = (value_type)
(color_type::int_prelerp(p[Order::A], alpha, alpha));
#endif
         }

-- john

On Aug 18, 2009, at 9:33 AM, klaas.holwerda wrote:

> John Horigan wrote:
>> Sure, that would also be a good way to handle the small performance
>> impact of using more accurate blenders. I would still want to move
>> multiplying alpha and cover from the pixfmt class to the blender  
>> class.
>>
>
> I think we really must not duplicate many the classes because of this
> patch. If people made changes, they should have supplied them as
> patches, or do so soon, else the problem is theirs.
> We made the Agg fork to start the development again, and that means
> changes!
>
> The patch corrects things that are wrong in Agg. And if people want to
> keep the old way, very likely the that can be fixed with a simple  
> #define.
> So do you think you can make a patch like that John? I can imagine  
> that
> int_lerp int_mult etc. In two versions using a #define , will do the
> trick already, since it is all inline.
> I personally don't mind a small performance impact.
>
> To conclude i suggest to wait to the END of the WEEK, if no serious
> issues arise, apply the patch.
>
> So those against, please speak up, and come with suggestion to improve
> the patch. Or wait, and supply a patch later to improve Agg even more.
>
> I volunteer to apply the patch, unless someone else wants to do it?
> ( i am not a SVN hero, but i think i manage).
>
> Regards,
>
> Klaas
>
>
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008  
> 30-Day
> trial. Simplify your report design, integration and deployment - and  
> focus on
> what you do best, core application coding. Discover what's new with
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Vector-agg-general mailing list
> Vector-agg-general@...
> https://lists.sourceforge.net/lists/listinfo/vector-agg-general

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Vector-agg-general mailing list
Vector-agg-general@...
https://lists.sourceforge.net/lists/listinfo/vector-agg-general

[klaas.holwerda-3]

John Horigan wrote:
> I don't think that I can recreate the current blenders by redefining  
> int_lerp. int_prelerp, etc. The existing blenders don't use a single  
> method for multiplying color components or cover values. Many  
> different tweaks are used to compensate for the error of treating  
> divide by 256 as if it were divide by 255.
>
> I would have to keep the existing code:
>  
Oke if that is the only Place you would need this, that would not be bad.
Or do you mean that you would need that define in all cases?
> #ifdef AGG_Legacy_Blenders
>  
Do you think using this define is an option to get your patch integrated
or not?
Or do you think too that it should become separate classes?
And what about the next in that case?
> move  
> > multiplying alpha and cover from the pixfmt class to the blender class.
I am trying to figure out what is best to do with this patch,  since  
simply apply might not be the best.

Regards,

Klaas




------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Vector-agg-general mailing list
Vector-agg-general@...
https://lists.sourceforge.net/lists/listinfo/vector-agg-general

[John Horigan]

On Aug 18, 2009, at 12:00 PM, klaas.holwerda wrote:

> John Horigan wrote:
>> I don't think that I can recreate the current blenders by redefining
>> int_lerp. int_prelerp, etc. The existing blenders don't use a single
>> method for multiplying color components or cover values. Many
>> different tweaks are used to compensate for the error of treating
>> divide by 256 as if it were divide by 255.
>>
>> I would have to keep the existing code:
>>
> Oke if that is the only Place you would need this, that would not be  
> bad.
> Or do you mean that you would need that define in all cases?

That was just an example. Each blender method would require an #ifdef  
block, 6 in agg_pixfmt_rgba.h, 4 in agg_pixfmt_rgb.h, and 2 in  
agg_pixfmt_gray.h.

>> #ifdef AGG_Legacy_Blenders
>>
> Do you think using this define is an option to get your patch  
> integrated
> or not?
> Or do you think too that it should become separate classes?
> And what about the next in that case?

If we want to keep the current blenders around then I think it would  
be cleaner to have them in separate classes and add more typedefs to  
the end of the agg_x_pixfmt.h files defining pixel formats that use  
the old blenders. Perhaps we could move the old blenders and their  
typedefs into separate header files that include the corresponding  
agg_x_pixfmt.h file.

>> move
>>> multiplying alpha and cover from the pixfmt class to the blender  
>>> class.
> I am trying to figure out what is best to do with this patch,  since
> simply apply might not be the best.

It is possible to implement the improved blenders without moving the  
alpha and cover multiplication from pixfmt_alpha_blend_rgba to the  
blend_pix() methods. My motivation for this interface change is that  
it exposes how parallel blending is for the pre-multiplied pixel  
formats. Multiplying alpha with cover in pixfmt_alpha_blend_rgba  
doesn't provide any benefit and obscures opportunities for optimizing  
blending.

>
> Regards,
>
> Klaas
>
>
>
>
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008  
> 30-Day
> trial. Simplify your report design, integration and deployment - and  
> focus on
> what you do best, core application coding. Discover what's new with
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Vector-agg-general mailing list
> Vector-agg-general@...
> https://lists.sourceforge.net/lists/listinfo/vector-agg-general

-- john



------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Vector-agg-general mailing list
Vector-agg-general@...
https://lists.sourceforge.net/lists/listinfo/vector-agg-general

[klaas.holwerda-3]

John Horigan wrote:
> It is possible to implement the improved blenders without moving the  
> alpha and cover multiplication from pixfmt_alpha_blend_rgba to the  
> blend_pix() methods. My motivation for this interface change is that  
> it exposes how parallel blending is for the pre-multiplied pixel  
> formats. Multiplying alpha with cover in pixfmt_alpha_blend_rgba  
> doesn't provide any benefit and obscures opportunities for optimizing  
> blending.
>  
So it looks it is best to have this change in all blenders.
And next add the improved blenders as separate classes.
I think you best make a patch (maybe two separate pathces) that has
this, so we can simply can apply it.
What that be oke for  you?

Regards,

Klaas

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Vector-agg-general mailing list
Vector-agg-general@...
https://lists.sourceforge.net/lists/listinfo/vector-agg-general