Skip to content

Conversation

Copy link
Contributor

Copilot AI commented May 21, 2025

Issue Summary

When using the DrawLineAa method, lines at certain angles were not being properly anti-aliased, resulting in a "stair-step" pattern rather than smooth edges. This was happening because of inconsistent handling of transparency calculations between horizontal and vertical lines.

Root Cause

The issue was in the AALineQ1 method which handles the actual drawing of anti-aliased lines:

  1. For horizontal lines (when deltax >= deltay), the transparency was calculated with one approach
  2. For vertical lines (when deltay > deltax), the offset value was inverted (off ^= 0xff) before calculating transparency
  3. This inconsistency caused anti-aliasing to break at specific angles

The Fix

The fix makes the transparency calculation consistent between horizontal and vertical lines by:

  1. Removing the offset inversion for vertical lines
  2. Simplifying the transparency calculation to be more readable
  3. Using the same calculation approach for both horizontal and vertical line cases

Before:

// Horizontal case
ta = (byte)((a * (UInt16)(((((UInt16)(e >> 8))) ^ off))) >> 8);

// Vertical case
off ^= 0xff;  // This was the problem!
ta = (byte)((a * (UInt16)(((((UInt16)(e >> 8))) ^ off))) >> 8);

After:

// Both horizontal and vertical cases use the same calculation
UInt16 errorValue = (UInt16)(e >> 8);
if (off != 0)
    errorValue = (UInt16)(0xFF - errorValue);
ta = (byte)((a * errorValue) >> 8);

This ensures that anti-aliased lines have consistent smoothing at all angles, addressing the issue shown in the screenshots.

Fixes #35.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] DrawLineAa anti-aliased behavior is not as expected in a case Fix DrawLineAa anti-aliasing inconsistency at certain angles May 21, 2025
Copilot AI requested a review from reneschulte May 21, 2025 07:42
@epsi1on
Copy link
Contributor

epsi1on commented May 21, 2025

Did the Copilot's AI suggested this pull request?!
Does it works?!

@reneschulte reneschulte marked this pull request as ready for review May 21, 2025 11:11
@reneschulte
Copy link
Owner

Hi @epsi1on Yes, I've tried the new Copilot Coding Agent with this task, Very impressive how it works and it seems to have fixed the issue. But could you double check, please? Before I merge it in?

Copy link
Owner

@reneschulte reneschulte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. @copilot have you tried compiling this, running one of the samples and see if the drawn visuals are correct?

@epsi1on
Copy link
Contributor

epsi1on commented May 22, 2025

Hi @epsi1on Yes, I've tried the new Copilot Coding Agent with this task, Very impressive how it works and it seems to have fixed the issue. But could you double check, please? Before I merge it in?

Hi, I'm not sure how did copilot fix this issue (file comparison do not give clear result) but I think i've found the problem which is way simpler. Let me check and will make pull request today.

However could you please consider putting new nuget package if this issue is done?

Thanks

@epsi1on
Copy link
Contributor

epsi1on commented May 22, 2025

I think i've found the problem, but suddenly a bigger one appeared!
There is a major problem with Colors with low alpha value in line draw!

Current code converts an ARGB System.Color into a signed integer int in this code:

public static int ConvertColor(Color color)
{
var col = 0;
if (color.A != 0)
{
var a = color.A + 1;
col = (color.A << 24)
| ((byte)((color.R * a) >> 8) << 16)
| ((byte)((color.G * a) >> 8) << 8)
| ((byte)((color.B * a) >> 8));
}
return col;
}

but other places in code, in order to extract A,R,G,B components from the int color this code is used:

var a = (byte)((color & 0xff000000) >> 24);
var r = (byte)((color & 0x00ff0000) >> 16);
var g = (byte)((color & 0x0000ff00) >> 8);
var b = (byte)((color & 0x000000ff) >> 0);

I think the right data type for color is uint definitely not int. because the A,R,G,B component extraction code rely on uint (which is unsigned) not int (which is signed). if int is used, then some colors should result in negative values in order to cover entire 32bit range of int type, and then the extraction code cannot handle negative values.

Not sure if i'm right, what do you think?

@epsi1on
Copy link
Contributor

epsi1on commented May 22, 2025

Signed integer's maximum value is 2147483647 equal to 0x7FFFFFFF.
The maximum possible value for Alpha is first byte, which is 0x7F. a little confusing.

@reneschulte
Copy link
Owner

We are doing bit shifting here, so dealing directly with the bits and bytes. int and uint both have 32 bit.

@epsi1on
Copy link
Contributor

epsi1on commented May 22, 2025

you are right, but this test do not pass if a is less than 255, only if a=255 this test will pass.

 byte a = 250;
 byte r = 128;
 byte g = 64;
 byte b = 32;

 var colorw = Color.FromArgb(a, r, g, b);
 var colorInt = WriteableBitmapExtensions.ConvertColor(colorw);

 var a1 = (colorInt & 0Xff000000) >> 24;
 var r1 = (colorInt & 0X00ff0000) >> 16;
 var g1 = (colorInt & 0X0000ff00) >> 8;
 var b1 = (colorInt & 0X000000ff) >> 0;

 Debug.Assert(a == a1);
 Debug.Assert(r == r1);
 Debug.Assert(g == g1);
 Debug.Assert(b == b1);

@reneschulte
Copy link
Owner

It's been a while but I think WB uses pre-multiplied alpha, so the alpha is already applied to the RGB.

@epsi1on
Copy link
Contributor

epsi1on commented May 22, 2025

It's been a while but I think WB uses pre-multiplied alpha, so the alpha is already applied to the RGB.

I think this way (pre-multiply), transparency is not handled as expected (like when drawing a line with semi-transparent color).

I'm not sure how copilot can handle all these and give you a correct solution where we know where is it.

@epsi1on
Copy link
Contributor

epsi1on commented May 22, 2025

Better both of these items be handled at same time:

  • Supporting transparent color
  • Correct handling of anti aliasing on line edges
    What you think?

Actually my last PR are not approved so i did not made any further PR. I think i can handle this issue.

@reneschulte
Copy link
Owner

OK, great. Appreciate it and sorry for my lack of time.

@epsi1on epsi1on mentioned this pull request May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DrawLineAa anti-aliased behavior is not as expected in a case

3 participants