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

Arcs not drawn correctly when start angle less than end angle #960

Open
corranwebster opened this issue Jul 23, 2022 · 5 comments
Open

Arcs not drawn correctly when start angle less than end angle #960

corranwebster opened this issue Jul 23, 2022 · 5 comments

Comments

@corranwebster
Copy link
Contributor

corranwebster commented Jul 23, 2022

Observed behaviour

Drawing from π/2 to 0 (it might be arguable that Agg is correct here):
Agg
image

Quartz
image

Drawing from π/2 to 0 with clockwise True (Agg is clearly not right, Quartz is at least consistent):
Agg
image

Quartz
image

To reproduce

Add these to enable.gcbench.suite and run:

class draw_arc_backwards:
    def __init__(self, gc, module):
        self.gc = gc
    
    def __call__(self):
        with self.gc:
            self.gc.begin_path()
            self.gc.arc(100.0, 100.0, 30.0, np.pi / 2, 0.0)
            self.gc.set_fill_color((0.33, 0.66, 0.99, 1.0))
            self.gc.fill_path()


class draw_arc_backwards_clockwise:
    def __init__(self, gc, module):
        self.gc = gc
    
    def __call__(self):
        with self.gc:
            self.gc.begin_path()
            self.gc.arc(100.0, 100.0, 30.0, np.pi / 2, 0.0, True)
            self.gc.set_fill_color((0.33, 0.66, 0.99, 1.0))
            self.gc.fill_path()

Expected behaviour

Not entirely sure.

Quartz is consistent, but has a significant discontinuity when the end angle sweeps over start angle, which may be problematic for plotting sectors and arcs (need to adjust cw flag appropriately when drawing). Agg avoids this, but the result when drawing "clockwise" doesn't make any sense at all. I suspect that the way that Quartz does it will be similar in other 2D libraries, since the Quartz call is a straight-through call to the underlying CoreGraphics routines.

A different possible interpretation is that an arc from one angle to another should be the path swept out between the angles, including windings (with associated effects on EOF vs. winding number fill, so 0 to 4π would be EOF empty), but then the purpose of the cw flag is unclear (one interpretation would be that it means angles are treated as clockwise from the x-axis).

In the end, I would be happy with being consistent with Quartz or HTML Canvas behaviour.

Whatever fix is put in place, it may impact rendering in downstream libraries.

@corranwebster
Copy link
Contributor Author

corranwebster commented Jul 23, 2022

Behaviour when wrapping around is also weird.

Going from -2π to π in normal mode (this is consistent with "wrapping around"):
Agg
image

Quartz
image

Going from -2π to π in clockwise mode:
Agg
image

Quartz
image

@corranwebster
Copy link
Contributor Author

corranwebster commented Jul 23, 2022

Parameter sweeps of arcs of all start/end points from -5π/2 to 10π/2 (inclusive), starts increasing along x-axis, ends up y axis. I am no longer convinced that Quartz is right.

Agg (normal and clockwise)
kiva agg draw_arcs
kiva agg draw_arcs_clockwise

Quartz (normal and clockwise)
quartz draw_arcs
quartz draw_arcs_clockwise

Code to replicate:

class draw_arcs:
    def __init__(self, gc, module):
        self.gc = gc
    
    def __call__(self):
        with self.gc:
            self.gc.set_fill_color((0.33, 0.66, 0.99, 1.0))
            for i in range(-5, 11):
                x = (i + 5) * 32 + 16.0
                start = i * np.pi / 2
                for j in range(-5, 11):
                    y = (j + 5) * 32 + 16.0
                    end = j * np.pi / 2
                    with self.gc:
                        self.gc.begin_path()
                        self.gc.arc(x, y, 12.0, start, end)
                        self.gc.fill_path()


class draw_arcs_clockwise:
    def __init__(self, gc, module):
        self.gc = gc
    
    def __call__(self):
        with self.gc:
            self.gc.set_fill_color((0.33, 0.66, 0.99, 1.0))
            for i in range(-5, 11):
                x = (i + 5) * 32 + 16.0
                start = i * np.pi / 2
                for j in range(-5, 11):
                    y = (j + 5) * 32 + 16.0
                    end = j * np.pi / 2
                    with self.gc:
                        self.gc.begin_path()
                        self.gc.arc(x, y, 12.0, start, end, True)
                        self.gc.fill_path()

@corranwebster
Copy link
Contributor Author

corranwebster commented Jul 23, 2022

Agg's "clockwise" seems to be consistent with "subtract 2π from the start angle"

Edit: this line doesn't seem right when sweeping backwards:

sweep_angle = -(2*agg24::pi - sweep_angle);

@corranwebster
Copy link
Contributor Author

Decision, following discussion:

  • start and end angles represent points on the circle
  • clockwise/anti-clockwise determine which path between those two points

This is the behaviour that the HTML Canvas follows.

@jwiggins
Copy link
Member

If you're curious what HTML Canvas does, here's some code that can be plugged into CodePen.

HTML:

<canvas id="canvas" width="1000" height="1000"></canvas>

JS:

const canvas = document.getElementById("canvas");
const ctx = canvas.getContext("2d");
ctx.fillStyle = "#55aaff";

const idxs = Array.from({ length: 16 }, (_, x) => x - 5);
idxs.map((i) => {
  const x = (i + 5) * 32 + 16.0;
  const start = (i * Math.PI) / 2;
  idxs.map((j) => {
    const y = (j + 5) * 32 + 16.0;
    const end = (j * Math.PI) / 2;

    ctx.beginPath();
    ctx.arc(x, y, 12.0, start, end); // A final argument of `true` can be added here
    ctx.fill();
  });
});

Which gives this output:
Screen Shot 2022-07-28 at 18 06 38

corranwebster added a commit that referenced this issue Aug 4, 2022
corranwebster added a commit that referenced this issue Aug 8, 2022
* Correct first point and orientation of Kiva QPainter arcs.

This fixes #962 but not #960.

* Update kiva/qpainter.py
corranwebster added a commit that referenced this issue Aug 8, 2022
* Correct first point and orientation of Kiva QPainter arcs.

This fixes #962 but not #960.

* Update kiva/qpainter.py
corranwebster added a commit that referenced this issue Aug 12, 2022
* Use the "sphinx-copybutton" extension in documentation (#948)

* DOC: Use the sphinx-copybutton extension in documentation

	modified:   ci/edmtool.py
	modified:   docs/source/conf.py
	modified:   enable/__init__.py

* Update ci/edmtool.py

* Get things working on wxPython again (#950)

Addresses these warnigns/exceptions:
- TypeError: BitmapFromImage() got an unexpected keyword argument 'depth'
- AttributeError: 'PaintDC' object has no attribute 'BeginDrawing'
- wxPyDeprecationWarning: Call to deprecated item EmptyImage. Use :class:`Image` instead.
    image = wx.EmptyImage(*sz)
- wxPyDeprecationWarning: Call to deprecated item BitmapFromImage. Use :class:`wx.Bitmap` instead
    bmp = wx.BitmapFromImage(image, depth=-1)

See https://forums.wxwidgets.org/viewtopic.php?t=39930 why the Begin/EndDrawing calls can simply be removed.

Related issues: #798 and #531

* Add SWIG to pyproject.toml build dependencies (#954)

* Experiment with installing SWIG from PyPI

* Update the EDM-based workflow

* Remove restriction on SWIG version

* Add Cython back as a development dependency; remove verify_swig_install

* Remove generated C++ file. (#958)

* Fix Kiva Quartz backend string encoding (#966)

* Add regression test for Enable #964.

* Properly decode strings in Kiva Quartz font code.

* Properly guard against running Quartz tests on non-Mac systems.

* Ensure Wx Quartz test terminates.

* Make test success externally visible.

* Fix tests, use UTF-8.

* Add a github workflow that publishes to sdists to PyPI on release (#967)

* Add a github workflow that publishes to sdists to PyPI on release.

* Use build to build the sdist.

* Take into account all font features when drawing SVG text. (#980)

* Fix Quartz font rendering (#978)

* Use font family in quartz when no font name given.

* Render quartz text with fill color rather than stroke color.

* Fix `SCRIPT` font family in Kiva (#975)

* Add a test for #971

This should fail CI.

* Fix bug and clean-up style.

* Correct first point and orientation of Kiva QPainter arcs (#970)

* Correct first point and orientation of Kiva QPainter arcs.

This fixes #962 but not #960.

* Update kiva/qpainter.py

* Render font families better in QPainter backend (#973)

* Fall back to kiva.fonttools if Qt can't come up with a font name.

* Add a comment about setFamilies()

* Fix WEIGHT_EXTRAHEAVY on Qt6 (#990)

Some changes to tests to make sure eveything is tested correctly.

* Fix the rendering of polygons in basecore2d-based backends (#987)

* Fix the rendering of polygons in basecore2d-based backends

This:

- consistently uses Nx2 numpy arrays to store path points
- doesn't create a new subpath after a LINES function

A driveby fix gives a default implementation for show_text_at_point().

* Clean-up comments.

* Fix some unused imports.

* Some of the unused imports are in fact used.

* Improve the situation of line_state_equal a bit.

* Ensure all subpath arguments are numpy arrays.

* Ensure all backends implement `arc_to` and `quad_curve_to` (#988)

* Ensure all backends implement arc_to and quad_curve_to.

* Fix 'infomration' typo in comments in multiple files.

* Fixes suggested from PR review.

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
Co-authored-by: Brecht Machiels <brecht@mos6581.org>
Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
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

No branches or pull requests

2 participants