Fix the rendering of polygons in basecore2d-based backends by corranwebster · Pull Request #987 · enthought/enable

corranwebster · 2022-08-10T07:57:09Z

This:

consistently uses Nx2 numpy arrays to store path points
doesn't create a new subpath after a LINES function (so consecutive arc, curve, etc. commands add to the subpath).

A drive-by fix gives a default implementation for show_text_at_point().

Image from the benchmark suite (for SVG):

Fixes #986.

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().

mdickinson · 2022-08-10T16:21:26Z

                if func == POINT:
                    self.draw_subpath(mode)
-                    self.add_point_to_subpath(args)
+                    self.add_point_to_subpath(args.reshape(1, 2))


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

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

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.

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.

mdickinson

LGTM besides the possible .reshape on non-arrays issue.

mdickinson

LGTM

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

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

corranwebster added 2 commits August 10, 2022 08:41

Fix the rendering of polygons in basecore2d-based backends

f8526eb

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.

db167f8

corranwebster mentioned this pull request Aug 10, 2022

Ensure all backends implement arc_to and quad_curve_to #988

Merged

corranwebster added 3 commits August 10, 2022 15:07

Fix some unused imports.

c8b7b22

Some of the unused imports are in fact used.

228be7b

Improve the situation of line_state_equal a bit.

b8a010d

mdickinson reviewed Aug 10, 2022

View reviewed changes

mdickinson approved these changes Aug 11, 2022

View reviewed changes

Ensure all subpath arguments are numpy arrays.

5a6405a

mdickinson approved these changes Aug 12, 2022

View reviewed changes

corranwebster merged commit de1170a into main Aug 12, 2022

corranwebster deleted the fix/svg-ps-polygons branch August 12, 2022 09:12

corranwebster mentioned this pull request Aug 12, 2022

Issues with basecore2d GraphicsContext #968

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Fix the rendering of polygons in basecore2d-based backends#987
corranwebster merged 6 commits into
mainfrom
fix/svg-ps-polygons

corranwebster commented Aug 10, 2022 •

edited

Loading

Uh oh!

mdickinson Aug 10, 2022

Uh oh!

mdickinson Aug 10, 2022

Uh oh!

corranwebster Aug 11, 2022

Uh oh!

corranwebster Aug 12, 2022

Uh oh!

mdickinson left a comment

Uh oh!

mdickinson left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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

Conversation

corranwebster commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdickinson Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

mdickinson Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

corranwebster Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

corranwebster Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

corranwebster commented Aug 10, 2022 •

edited

Loading