Skip to content

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

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.

Sign up for GitHub

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

Merged
corranwebster merged 6 commits into from
Aug 12, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f8526eb
Fix the rendering of polygons in basecore2d-based backends
corranwebster Aug 10, 2022
db167f8
Clean-up comments.
corranwebster Aug 10, 2022
c8b7b22
Fix some unused imports.
corranwebster Aug 10, 2022
228be7b
Some of the unused imports are in fact used.
corranwebster Aug 10, 2022
b8a010d
Improve the situation of line_state_equal a bit.
corranwebster Aug 10, 2022
5a6405a
Ensure all subpath arguments are numpy arrays.
corranwebster Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 21 additions & 21 deletions kiva/basecore2d.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

"""
import numpy as np
from numpy import alltrue, array, asarray, concatenate, float64, pi, shape
from numpy import alltrue, array, asarray, float64, pi

from .constants import (
CAP_BUTT, CAP_ROUND, CAP_SQUARE, CLOSE, CONCAT_CTM, EOF_FILL_STROKE,
Expand All @@ -40,7 +40,7 @@
TRANSLATE_CTM,
)
from .abstract_graphics_context import AbstractGraphicsContext
from .line_state import LineState, line_state_equal
from .line_state import LineState, line_state_equal # noqa: F401
from .graphics_state import GraphicsState
from .fonttools import Font
import kiva.affine as affine
Expand Down Expand Up @@ -504,6 +504,8 @@ def line_set(self, starts, ends):
Starts and ends should have the same length.
The current point is moved to the last point in 'ends'.
"""
starts = asarray(starts)
ends = asarray(ends)
self._new_subpath()
for i in range(min(len(starts), len(ends))):
self.active_subpath.append((POINT, starts[i]))
Expand Down Expand Up @@ -1054,7 +1056,7 @@ def show_glyphs(self):
def show_text_at_point(self, text, x, y):
"""
"""
pass
self.show_text(text, (x, y))

def show_glyphs_at_point(self):
"""
Expand Down Expand Up @@ -1129,18 +1131,20 @@ def draw_path(self, mode=FILL_STROKE):
for func, args in subpath:
if func == POINT:
self.draw_subpath(mode)
self.add_point_to_subpath(args)
self.add_point_to_subpath(args.reshape(1, 2))
Copy link
Copy Markdown
Member

@mdickinson mdickinson Aug 10, 2022

Choose a reason for hiding this comment

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

Is it guaranteed that args will always be a NumPy array? I see some examples that are calling line_set with lists of tuples, for example here:

enable/docs/source/kiva_tutorial/tutorial_advanced.py

Lines 226 to 228 in a71a938

thin_starts = [(-20, 0), (-20, -18)]
thin_ends = [(20,0), (20, -18)]
gc.line_set(thin_starts, thin_ends)

(Same question for LINE below.)

Copy link
Copy Markdown
Member

@mdickinson mdickinson Aug 10, 2022

Choose a reason for hiding this comment

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

I guess calling the np.reshape function instead of the method might be a way to fix this.

Copy link
Copy Markdown
Contributor Author

@corranwebster corranwebster Aug 11, 2022

Choose a reason for hiding this comment

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

The values for args are populated from move_to, line_to, etc. which should be populating with arrays, but you're right it looks like there might be some places where this may not be absolutely guaranteed (eg. lines and line_set docstring specifies Nx2 arrays, but probably gets used with lists of points).

Possibly can move the reshaping there as well.

Copy link
Copy Markdown
Contributor Author

@corranwebster corranwebster Aug 12, 2022

Choose a reason for hiding this comment

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

I fixed the one case I could see where lines might add non-array object to the sub-path args, plus a second issue where closing the path could error or give the wrong results.

self.first_point = args
elif func == LINE:
self.add_point_to_subpath(args)
self.add_point_to_subpath(args.reshape(1, 2))
elif func == LINES:
self.draw_subpath(mode)
# add all points in list to subpath.
self.add_point_to_subpath(args)
self.first_point = args[0]
elif func == CLOSE:
self.add_point_to_subpath(self.first_point)
self.draw_subpath(mode)
if self.first_point is not None:
self.add_point_to_subpath(
self.first_point.reshape(1, 2)
)
self.draw_subpath(mode)
elif func == RECT:
self.draw_subpath(mode)
self.device_draw_rect(
Expand All @@ -1149,7 +1153,7 @@ def draw_path(self, mode=FILL_STROKE):
elif func in ctm_funcs:
self.device_transform_device_ctm(func, args)
else:
print("oops:", func)
raise RuntimeError(f"Unrecognised subpath term: {func}")
# finally, draw any remaining paths.
self.draw_subpath(mode)

Expand Down Expand Up @@ -1236,22 +1240,18 @@ def add_point_to_subpath(self, pt):

def clear_subpath_points(self):
self.draw_points = []
self.first_point = None

def get_subpath_points(self, debug=0):
""" Gets the points that are in the current path.
def get_subpath_points(self, debug=False):
""" Gets the points that are in the current subpath as an Nx2 array.

The first entry in the draw_points list may actually
be an array. If this is true, the other points are
converted to an array and concatenated with the first
The draw_points attribute holds the current set of points as a
list of Nx2 arrays.
"""
if self.draw_points and len(shape(self.draw_points[0])) > 1:
first_points = self.draw_points[0]
other_points = asarray(self.draw_points[1:])
if len(other_points):
pts = concatenate((first_points, other_points), 0)
else:
pts = first_points
if self.draw_points:
pts = np.vstack(self.draw_points)
else:
# list is empty, convert to an empty array
pts = asarray(self.draw_points)
return pts

Expand Down
11 changes: 6 additions & 5 deletions kiva/tests/test_basecore2d.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from kiva import affine
from kiva import basecore2d
from kiva import constants
from kiva.line_state import line_state_equal


class TestIsFullyTransparent(unittest.TestCase):
Expand Down Expand Up @@ -70,31 +71,31 @@ def test_color_on_copy(self):
ls1 = self.create_ls()
ls2 = ls1.copy()
ls1.line_color[1] = 10
self.assertTrue(not basecore2d.line_state_equal(ls1, ls2))
self.assertTrue(not line_state_equal(ls1, ls2))

def test_dash_on_copy(self):
ls1 = self.create_ls()
ls2 = ls1.copy()
ls1.line_dash[1][0] = 10
self.assertTrue(not basecore2d.line_state_equal(ls1, ls2))
self.assertTrue(not line_state_equal(ls1, ls2))

def test_cmp_for_different_length_dash_patterns(self):
ls1 = self.create_ls()
ls2 = ls1.copy()
ls1.line_dash = (ls1.line_dash[0], array([10, 10, 10, 10]))
self.assertTrue(not basecore2d.line_state_equal(ls1, ls2))
self.assertTrue(not line_state_equal(ls1, ls2))

def test_cmp(self):
ls1 = self.create_ls()
ls2 = ls1.copy()
self.assertTrue(basecore2d.line_state_equal(ls1, ls2))
self.assertTrue(line_state_equal(ls1, ls2))

# line_dash no longer allowed to be none.
# def test_cmp_with_dash_as_none(self):
# ls1 = self.create_ls()
# ls2 = ls1.copy()
# #ls1.line_dash = None
# assert(not basecore2d.line_state_equal(ls1,ls2))
# assert(not line_state_equal(ls1,ls2))


class GraphicsContextTestCase(unittest.TestCase):
Expand Down