aa_test is not antiliased (jagged), a bug?

[jay3d]

The aa-test demo in the latest svn is not antialiased!

is it a known bug?

using VS2008 SP1

Thanks!


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

[Jim Barry-2]

jay3d wrote:
 > The aa-test demo in the latest svn is not antialiased!
 >
 > is it a known bug?

Seems to be a problem with the recent "accurate blending" patch.

In pixfmt_alpha_blend_rgb::copy_or_blend_pix, we used to have:

   if (c.a)
   {
     calc_type alpha = (calc_type(c.a) * (cover + 1)) >> 8;
     if(alpha == base_mask)
     {
       p[order_type::R] = c.r;
       p[order_type::G] = c.g;
       p[order_type::B] = c.b;
     }
     else
     {
       m_blender.blend_pix(p, c.r, c.g, c.b, alpha, cover);
     }
   }

The patch changed this to:

   if (c.a)
   {
     if(c.a == base_mask && cover == cover_mask)
     {
       p[order_type::R] = c.r;
       p[order_type::G] = c.g;
       p[order_type::B] = c.b;
     }
     else
     {
       m_blender.blend_pix(p, c.r, c.g, c.b, c.a, cover);
     }
   }

Which is not correct. I think it should be something like this:

   if (c.a)
   {
     calc_type alpha = color_type::int_mult_cover(c.a, cover);
     if(alpha == base_mask)
     {
       p[order_type::R] = c.r;
       p[order_type::G] = c.g;
       p[order_type::B] = c.b;
     }
     else
     {
       m_blender.blend_pix(p, c.r, c.g, c.b, alpha, cover);
     }
   }

The same applies to blend_hline, blend_vline, blend_solid_hspan, and
blend_solid_vspan.

- Jim

------------------------------------------------------------------------------
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 believe that the patch is correct:
int_mult_cover(255, 254) == 254 (rgb8)
int_mult_cover(65534, 255) == 65534 && int_mult_cover(65535, 254) ==  
65278 (rgb16)

int_mult_cover(c.a, cover) equals base_mask if and only if c.a equals  
base_mask and cover equals cover_mask. So the two if statement  
conditions are equivalent.

-- john

On Nov 9, 2009, at 10:48 AM, Jim Barry wrote:

> jay3d wrote:
>> The aa-test demo in the latest svn is not antialiased!
>>
>> is it a known bug?
>
> Seems to be a problem with the recent "accurate blending" patch.
>
> In pixfmt_alpha_blend_rgb::copy_or_blend_pix, we used to have:
>
>   if (c.a)
>   {
>     calc_type alpha = (calc_type(c.a) * (cover + 1)) >> 8;
>     if(alpha == base_mask)
>     {
>       p[order_type::R] = c.r;
>       p[order_type::G] = c.g;
>       p[order_type::B] = c.b;
>     }
>     else
>     {
>       m_blender.blend_pix(p, c.r, c.g, c.b, alpha, cover);
>     }
>   }
>
> The patch changed this to:
>
>   if (c.a)
>   {
>     if(c.a == base_mask && cover == cover_mask)
>     {
>       p[order_type::R] = c.r;
>       p[order_type::G] = c.g;
>       p[order_type::B] = c.b;
>     }
>     else
>     {
>       m_blender.blend_pix(p, c.r, c.g, c.b, c.a, cover);
>     }
>   }
>
> Which is not correct. I think it should be something like this:
>
>   if (c.a)
>   {
>     calc_type alpha = color_type::int_mult_cover(c.a, cover);
>     if(alpha == base_mask)
>     {
>       p[order_type::R] = c.r;
>       p[order_type::G] = c.g;
>       p[order_type::B] = c.b;
>     }
>     else
>     {
>       m_blender.blend_pix(p, c.r, c.g, c.b, alpha, cover);
>     }
>   }
>
> The same applies to blend_hline, blend_vline, blend_solid_hspan, and
> blend_solid_vspan.
>
> - Jim
>
> ------------------------------------------------------------------------------
> 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

[Jim Barry-2]

John Horigan wrote:
> I believe that the patch is correct:

Compiling and running aa_test proves otherwise ;-)

> int_mult_cover(255, 254) == 254 (rgb8)
> int_mult_cover(65534, 255) == 65534 && int_mult_cover(65535, 254) ==  
> 65278 (rgb16)
>
> int_mult_cover(c.a, cover) equals base_mask if and only if c.a equals  
> base_mask and cover equals cover_mask. So the two if statement  
> conditions are equivalent.

Yes, but the problem is that you pass c.a to blend_pix instead of (c.a *
cover / cover_full). So you *could* write:

   if (c.a)
   {
     if(c.a == base_mask && cover == cover_mask)
     {
       p[order_type::R] = c.r;
       p[order_type::G] = c.g;
       p[order_type::B] = c.b;
     }
     else
     {
       calc_type alpha = color_type::int_mult_cover(c.a, cover);
       m_blender.blend_pix(p, c.r, c.g, c.b, alpha, cover);
     }
   }

Is this more or less efficient than the version I proposed before? I
don't know. I suppose it depends how many of the source pixels are fully
opaque.

- Jim

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

In the blender class blend_pix() method you will find:
alpha = color_type::int_mult_cover(alpha, cover);

This is part of the patch. The int_mult_cover(alpha, cover) has been moved from pixfmt_alpha_blender_* to the blender class blend_pix () method. This was done to improve efficiency and expose parallelism.

My own code uses renderer_scanline_aa_solid<> and it produces anti-aliased output. I will pull the latest copy of agg from svn and see if it is different than what I am compiling my code against.

-- john

On Nov 10, 2009, at 3:33 AM, Jim Barry wrote:

> John Horigan wrote:
>> I believe that the patch is correct:
>
> Compiling and running aa_test proves otherwise ;-)
>
>> int_mult_cover(255, 254) == 254 (rgb8)
>> int_mult_cover(65534, 255) == 65534 && int_mult_cover(65535, 254) ==  
>> 65278 (rgb16)
>>
>> int_mult_cover(c.a, cover) equals base_mask if and only if c.a equals  
>> base_mask and cover equals cover_mask. So the two if statement  
>> conditions are equivalent.
>
> Yes, but the problem is that you pass c.a to blend_pix instead of (c.a *
> cover / cover_full). So you *could* write:
>
>   if (c.a)
>   {
>     if(c.a == base_mask && cover == cover_mask)
>     {
>       p[order_type::R] = c.r;
>       p[order_type::G] = c.g;
>       p[order_type::B] = c.b;
>     }
>     else
>     {
>       calc_type alpha = color_type::int_mult_cover(c.a, cover);
>       m_blender.blend_pix(p, c.r, c.g, c.b, alpha, cover);
>     }
>   }
>
> Is this more or less efficient than the version I proposed before? I
> don't know. I suppose it depends how many of the source pixels are fully
> opaque.
>
> - Jim
>
> ------------------------------------------------------------------------------
> 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

[Jim Barry-2]

John Horigan wrote:
> In the blender class blend_pix() method you will find:
> alpha = color_type::int_mult_cover(alpha, cover);
>
> This is part of the patch. The int_mult_cover(alpha, cover) has been moved from pixfmt_alpha_blender_* to the blender class blend_pix () method. This was done to improve efficiency and expose parallelism.

OK, I see. So it's just that you forgot to add an int_mult_cover call to
blender_rgb_gamma::blend_pix.

Actually it did seem like a bit of an anomaly that the original version
of pixfmt_alpha_blend_rgb[a] prelerped the alpha before calling
blend_pix - other pixel formats don't do that.

- Jim

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

Ah, I missed that one. I can't see any more that I've missed. Can you give a look?

I'll send out patch fixing this bug as soon as we get to the bottom of this aa_test failure. I can't even get it to build on my system (a mac).

-- john

On Nov 10, 2009, at 11:13 AM, Jim Barry wrote:

> John Horigan wrote:
>> In the blender class blend_pix() method you will find:
>> alpha = color_type::int_mult_cover(alpha, cover);
>>
>> This is part of the patch. The int_mult_cover(alpha, cover) has been moved from pixfmt_alpha_blender_* to the blender class blend_pix () method. This was done to improve efficiency and expose parallelism.
>
> OK, I see. So it's just that you forgot to add an int_mult_cover call to
> blender_rgb_gamma::blend_pix.
>
> Actually it did seem like a bit of an anomaly that the original version
> of pixfmt_alpha_blend_rgb[a] prelerped the alpha before calling
> blend_pix - other pixel formats don't do that.
>
> - Jim
>
> ------------------------------------------------------------------------------
> 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

[Jim Barry-2]

John Horigan wrote:
 > Ah, I missed that one. I can't see any more that I've missed. Can you
give a look?

I haven't spotted any others yet, but I found another problem. The
bottom of the "ice lolly sticks" are not rendered correctly (see
attached image). It took me a little while to figure out the reason, but
in rgba8 we have:

   AGG_INLINE self_type gradient(const self_type& c, double k) const
   {
     self_type ret;
     calc_type ik = uround(k * base_scale);
     ret.r = (value_type)int_lerp(r, c.r, ik);
     ret.g = (value_type)int_lerp(g, c.g, ik);
     ret.b = (value_type)int_lerp(b, c.b, ik);
     ret.a = (value_type)int_lerp(a, c.a, ik);
     return ret;
   }

In 8-bit formats, you're lerping in the range 0-256 instead of 0-255.
You need to use "base_mask" instead of "base_scale". The same applies to
rgba16, of course.

- Jim




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

[Stephan Assmus]

On 2009-11-11 at 16:58:05 [+0100], Jim Barry <jim@...> wrote:

> John Horigan wrote:
>  > Ah, I missed that one. I can't see any more that I've missed. Can you
> give a look?
>
> I haven't spotted any others yet, but I found another problem. The bottom
> of the "ice lolly sticks" are not rendered correctly (see attached
> image). It took me a little while to figure out the reason, but in rgba8
> we have:
>
>    AGG_INLINE self_type gradient(const self_type& c, double k) const {
>      self_type ret;
>      calc_type ik = uround(k * base_scale);
>      ret.r = (value_type)int_lerp(r, c.r, ik);
>      ret.g = (value_type)int_lerp(g, c.g, ik);
>      ret.b = (value_type)int_lerp(b, c.b, ik);
>      ret.a = (value_type)int_lerp(a, c.a, ik);
>      return ret;
>    }
>
> In 8-bit formats, you're lerping in the range 0-256 instead of 0-255. You
> need to use "base_mask" instead of "base_scale". The same applies to
> rgba16, of course.

<nugde>

John, have you found any time to look into this already?

Best regards,
-Stephan

------------------------------------------------------------------------------
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 can't get aa_test to build on my system (Mac OS X 10.6.2). I will try it in a Ubuntu VM. Meanwhile, renderer_scanline_aa_solid<> generates properly anti-aliased output in my own application. And the image posted by Jim Barry is properly anti-aliased too (although the end-caps are the wrong color).

-- john


On Nov 16, 2009, at 11:05 AM, Stephan Assmus wrote:

>
> On 2009-11-11 at 16:58:05 [+0100], Jim Barry <jim@...> wrote:
>> John Horigan wrote:
>>> Ah, I missed that one. I can't see any more that I've missed. Can you
>> give a look?
>>
>> I haven't spotted any others yet, but I found another problem. The bottom
>> of the "ice lolly sticks" are not rendered correctly (see attached
>> image). It took me a little while to figure out the reason, but in rgba8
>> we have:
>>
>>   AGG_INLINE self_type gradient(const self_type& c, double k) const {
>>     self_type ret;
>>     calc_type ik = uround(k * base_scale);
>>     ret.r = (value_type)int_lerp(r, c.r, ik);
>>     ret.g = (value_type)int_lerp(g, c.g, ik);
>>     ret.b = (value_type)int_lerp(b, c.b, ik);
>>     ret.a = (value_type)int_lerp(a, c.a, ik);
>>     return ret;
>>   }
>>
>> In 8-bit formats, you're lerping in the range 0-256 instead of 0-255. You
>> need to use "base_mask" instead of "base_scale". The same applies to
>> rgba16, of course.
>
> <nugde>
>
> John, have you found any time to look into this already?
>
> Best regards,
> -Stephan
>
> ------------------------------------------------------------------------------
> 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

[John Horigan]

It was very painful, but I got aa_test to build under Ubuntu and reproduced jay3d's issue.

I applied fixes for the two bugs pointed out by Jim Barry. Fixing these two bugs restored anti-aliasing to aa_test.

Here is a patch:




-- john

On Nov 16, 2009, at 3:25 PM, John Horigan wrote:

> I can't get aa_test to build on my system (Mac OS X 10.6.2). I will try it in a Ubuntu VM. Meanwhile, renderer_scanline_aa_solid<> generates properly anti-aliased output in my own application. And the image posted by Jim Barry is properly anti-aliased too (although the end-caps are the wrong color).
>
> -- john
>
>
> On Nov 16, 2009, at 11:05 AM, Stephan Assmus wrote:
>
>>
>> On 2009-11-11 at 16:58:05 [+0100], Jim Barry <jim@...> wrote:
>>> John Horigan wrote:
>>>> Ah, I missed that one. I can't see any more that I've missed. Can you
>>> give a look?
>>>
>>> I haven't spotted any others yet, but I found another problem. The bottom
>>> of the "ice lolly sticks" are not rendered correctly (see attached
>>> image). It took me a little while to figure out the reason, but in rgba8
>>> we have:
>>>
>>>  AGG_INLINE self_type gradient(const self_type& c, double k) const {
>>>    self_type ret;
>>>    calc_type ik = uround(k * base_scale);
>>>    ret.r = (value_type)int_lerp(r, c.r, ik);
>>>    ret.g = (value_type)int_lerp(g, c.g, ik);
>>>    ret.b = (value_type)int_lerp(b, c.b, ik);
>>>    ret.a = (value_type)int_lerp(a, c.a, ik);
>>>    return ret;
>>>  }
>>>
>>> In 8-bit formats, you're lerping in the range 0-256 instead of 0-255. You
>>> need to use "base_mask" instead of "base_scale". The same applies to
>>> rgba16, of course.
>>
>> <nugde>
>>
>> John, have you found any time to look into this already?
>>
>> Best regards,
>> -Stephan
>>
>> ------------------------------------------------------------------------------
>> 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

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

[Stephan Assmus]

On 2009-11-17 at 08:01:49 [+0100], John Horigan <john@...> wrote:
> It was very painful, but I got aa_test to build under Ubuntu and
> reproduced jay3d's issue.
>
> I applied fixes for the two bugs pointed out by Jim Barry. Fixing these
> two bugs restored anti-aliasing to aa_test.

Thanks a bunch!

Best regards,
-Stephan

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