Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve differences after agg 2.2 and agg 2.4 migration #67

Open
djhoese opened this issue May 17, 2020 · 3 comments
Open

Resolve differences after agg 2.2 and agg 2.4 migration #67

djhoese opened this issue May 17, 2020 · 3 comments
Assignees

Comments

@djhoese
Copy link
Member

djhoese commented May 17, 2020

In #50 aggdraw master was updated to use the agg C++ library by @dov. Later it was pointed out by @a-hurst that there were some differences between aggdraw using agg 2.2 and aggdraw using 2.4 in issue #61 which was resolved in #62. At the end of #61 @a-hurst and I started discussing some of the other differences we were noticing with the C++ library changes. As it stands, the master branch (unreleased) does not produce matching results to the aggdraw 1.3.x release. I'd like to figure this out once and for all so I've broken my example from the pycoast package down in to a simpler case. Here is the big pycoast-based result difference from #61:

aggdraw 1.3.x:

image

aggdraw 1.4 (master):

image

Look at the smaller polygons (islands) for the biggest difference. If I strip this down to this code example:

import aggdraw
from PIL import Image
import os
width = 200
height = 200
img = Image.new('RGB', (width, height))
draw = aggdraw.Draw(img)

pen = aggdraw.Pen('white', 1, 255)
coordinates = [166.56794437411008, 77.25389224364426, 48.00197334018185, 78.96339024123063, 81.28185670937546, 83.30298429919458, 89.3666036554605, 154.98793226592443, 88.69627544716832, 84.44108529130517]
draw.line(coordinates, pen)
draw.flush()
img.save('new.png')

And you run that with both versions (obviously change the filename), you will see extremely small differences like the below.

aggdraw 1.3.x:

old

aggdraw 1.4 (master):

new

Note that including the change for line:

as suggested by @a-hurst in #61 from 1.0 to 0.5 doesn't seem to have an effect on this use case.

So something has changed and on the small scale it is almost impossible to see but for many lines (the coastlines above) it is much more apparent. Does anyone have any ideas what other values we could play with to make aggdraw produce equivalent results? @a-hurst sorry if I'm forgetting some suggestion from #61.

@djhoese
Copy link
Member Author

djhoese commented May 17, 2020

And something I didn't make very clear before, the drawing issues get harder and harder to see the more pixels you give it to do the antialiasing (I assume). Like, the agg library might be rounding up too much when it doesn't have a lot of pixels to perform antialiasing so you get images like my coastline examples. Meaning, the antialiasing for a particular line has less than 1 pixel to antialias the line but it is rounding up to 1 (or 2 or whatever) and drawing many more pixels than it should. This results in much thicker lines than it should.

@a-hurst
Copy link
Contributor

a-hurst commented May 29, 2020

Hey @djhoese, just remembered I got an email notification about this a while ago and forgot to respond!

I'm quite busy with academic stuff over the next few days (trying to get some papers out for publication while all the labs are closed), but can definitely make some time next week to try and help out with this. I'm motivated to get this fixed so that we can get a new aggdraw PyPi release, since the current one doesn't have any binaries for Python 3.8 yet.

@djhoese
Copy link
Member Author

djhoese commented May 8, 2023

I've been contacted by someone over email (aapost) who doesn't use github, but has given me permission to repeat parts of our conversation here. They have been combing through the differences between AGG 2.2 and 2.4 and trying to nail down what can be done to get aggdraw back to or closer to the results it used to produce. They aren't sure we can ever get back to exactly what aggdraw used to produce, but maybe we can get close.

One of their test cases is:

Test Code
try:
    import aggdraw
except ImportError:
    print("===", "this demo requires the aggdraw library")
    print("===", "see http://effbot.org/zone/aggdraw.htm")
    raise

from PIL import Image
from math import *

def color(r, g, b):
    r = min(255 * r, 255)
    g = min(255 * g, 255)
    b = min(255 * b, 255)
    return "#{:02x}{:02x}{:02x}".format(int(r), int(g), int(b))

if __name__ == "__main__":
    w, h = (500, 500)
    img = Image.new("RGB", (w, h), "#808080")
    d = aggdraw.Draw(img)
    # "golden section", adapted from a DrawBot demo script
    # (see http://just.letterror.com/ltrwiki/DrawBot)
    cx, cy = w/2, h/2
    s = min(w / 400.0, h / 400.0)
    phi = (sqrt(5) + 1)/2-1
    oradius = 10.0
    for i in range(720):
        c = (0.0, 0.0, 0.0)
        r = s * 1.5*oradius * sin(i * pi/720)
        x = cx + s*0.25*i*cos(phi*i*2*pi)
        y = cy + s*0.25*i*sin(phi*i*2*pi)
        d.ellipse(
            (x-r/2, y-r/2, x+r/2, y+r/2), aggdraw.Brush(color(*c))
            )
        c = (i / 360.0, i / 360.0, 0.25)
        r = s * oradius * sin(i * pi/720)
        d.ellipse(
            (x-r/2, y-r/2, x+r/2, y+r/2), aggdraw.Brush(color(*c))
            )
    d.flush()
    img.show()

One path that was taken to try to resolve some differences was reverting a change made by @dov in #50 regarding contour width:

https://github.com/pytroll/aggdraw/pull/50/files#diff-5bab0e2c45b569c0904044192a1777d5b5abbb06164728cce9d7a0a452c1a684L435-L438

This along with some other changes got aapost's issue mostly resolved (from my understanding) but did not resolve my original pycoast issue mentioned in the original comment when I created this issue. Here is one diff they sent me:

diff --git a/agg/include/agg_curves.h b/agg/include/agg_curves.h
index 1ef02e8..63a4813 100644
--- a/agg/include/agg_curves.h
+++ b/agg/include/agg_curves.h
@@ -96,6 +96,7 @@ namespace agg
     public:
         curve3_div() : 
             m_approximation_scale(1.0),
+            m_distance_tolerance_square(0.0),
             m_angle_tolerance(0.0),
             m_count(0)
         {}
@@ -376,6 +377,7 @@ namespace agg
     public:
         curve4_div() : 
             m_approximation_scale(1.0),
+            m_distance_tolerance_square(0.0),
             m_angle_tolerance(0.0),
             m_cusp_limit(0.0),
             m_count(0)
diff --git a/agg/include/agg_math_stroke.h b/agg/include/agg_math_stroke.h
index 6a9d604..b139711 100644
--- a/agg/include/agg_math_stroke.h
+++ b/agg/include/agg_math_stroke.h
@@ -172,11 +172,9 @@ namespace agg
     {
         double a1 = std::atan2(dy1 * m_width_sign, dx1 * m_width_sign);
         double a2 = std::atan2(dy2 * m_width_sign, dx2 * m_width_sign);
-        double da = a1 - a2;
+        double da = std::acos(m_width_abs / (m_width_abs + 0.125 / m_approx_scale)) * 2;
         int i, n;
 
-        da = std::acos(m_width_abs / (m_width_abs + 0.125 / m_approx_scale)) * 2;
-
         add_vertex(vc, x + dx1, y + dy1);
         if(m_width_sign > 0)
         {
diff --git a/agg/include/agg_pixfmt_rgba.h b/agg/include/agg_pixfmt_rgba.h
index c9172f6..2c7223c 100644
--- a/agg/include/agg_pixfmt_rgba.h
+++ b/agg/include/agg_pixfmt_rgba.h
@@ -2371,7 +2371,7 @@ namespace agg
         //--------------------------------------------------------------------
         AGG_INLINE void copy_pixel(int x, int y, const color_type& c)
         {
-            make_pix(pix_value_ptr(x, y, 1), c);
+            pix_value_ptr(x, y, 1)->set(c);
         }
 
         //--------------------------------------------------------------------
diff --git a/agg/include/agg_trans_affine.h b/agg/include/agg_trans_affine.h
index 90e464b..b3cc1b0 100644
--- a/agg/include/agg_trans_affine.h
+++ b/agg/include/agg_trans_affine.h
@@ -318,8 +318,8 @@ namespace agg
     //------------------------------------------------------------------------
     inline double trans_affine::scale() const
     {
-        double x = 0.707106781 * sx  + 0.707106781 * shx;
-        double y = 0.707106781 * shy + 0.707106781 * sy;
+        double x = 0.70710678118654752440 * sx  + 0.70710678118654752440 * shx;
+        double y = 0.70710678118654752440 * shy + 0.70710678118654752440 * sy;
         return std::sqrt(x*x + y*y);
     }
 
diff --git a/aggdraw.cxx b/aggdraw.cxx
index 25728de..c29cc47 100644
--- a/aggdraw.cxx
+++ b/aggdraw.cxx
@@ -448,6 +448,10 @@ public:
             /* interior */
             agg::conv_contour<agg::path_storage> contour(*p);
             contour.auto_detect_orientation(true);
+            if (pen)
+                contour.width(pen->width / 2.0);
+            else
+                contour.width(0.5);
             rasterizer.reset();
             rasterizer.add_path(contour);
             renderer.color(brush->color);

Lastly, aapost also narrowed down some of the agg 2.4 changes to 2 major areas causing differences. Here's what they said:

2 paths cause differences with the pycoast test. One being #include "agg_path_storage.h" (which gets used quite a bit, but I would say accounts for less than 30% of the variance). The other main one being #include "agg_conv_stroke.h", which is only invoked once with a stroke object, but accounts for I'd say more than 70% of the variance.

If you look at the news notes from the author you can see there was a lot of work done to stroke:

https://agg.sourceforge.net/antigrain.com/news/index.html

(particularly amusing being the statement "Improved the stroker algorithm (it will pursue me for the rest of my life, and I accept my fate).")

Most of the math in question causing the variance now resides in agg_math_stroke.h, (much of it was spread a level higher, mostly in agg_vcgen_stroke.cpp prior).

The functions that were rewritten that cause most of the variance are void math_stroke<VC>::calc_miter, void math_stroke<VC>::calc_join, and I believe a little bit of void math_stroke<VC>::calc_cap

This is some really good detective work and hopefully can lead us to some more consistent results. aapost mentioned they'd still like to look into agg_path_storage more.

@djhoese djhoese pinned this issue May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants