Skip to content

Adding 7 new markers#222

Merged
corranwebster merged 7 commits into
enthought:masterfrom
jonathanrocher:feature/add_new_markers
Apr 29, 2016
Merged

Adding 7 new markers#222
corranwebster merged 7 commits into
enthought:masterfrom
jonathanrocher:feature/add_new_markers

Conversation

@jonathanrocher

@jonathanrocher jonathanrocher commented Apr 26, 2016

Copy link
Copy Markdown
Contributor

The new markers proposed are: left and right triangle, star, cross-plus, hexagon1 and hexagon2, and pentagon.

To try them out, you can run:

import numpy as np
from traits.api import HasTraits, Instance
from traitsui.api import View, Item
from chaco.api import Plot, ArrayPlotData
from enable.api import ComponentEditor

class MyPlot(HasTraits):
    plot = Instance(Plot)
    view = View(Item('plot', editor=ComponentEditor(), show_label=False),
                resizable=True)

    def _plot_default(self):
        x = np.linspace(0, 2*np.pi, 20)
        y = np.sin(x)
        inv_y = -y
        half_y = 0.5 * y
        inv_half_y = -0.5 * y
        z = np.zeros(x.shape)
        data = ArrayPlotData(x=x, y=y, inv_y=inv_y, half_y=half_y,
                             inv_half_y=inv_half_y, z=z)
        plot = Plot(data)
        plot.plot(("x", "y"), type="scatter", marker="pentagon",
                  marker_size=10)
        plot.plot(("x", "inv_y"), type="scatter", color="blue", marker="hexagon",
                  marker_size=10)
        plot.plot(("x", "half_y"), type="scatter", color="green", marker="left_triangle",
                  marker_size=10)
        plot.plot(("x", "inv_half_y"), type="scatter", color="red", marker="hexagon2",
                  marker_size=10)
        plot.plot(("x", "z"), type="scatter", color="red", marker="star",
                  marker_size=10)
        return plot


MyPlot().configure_traits()

On OSX, it is giving me:
screen shot 2016-04-26 at 5 08 52 pm

Feedback requested.

@jonathanrocher

Copy link
Copy Markdown
Contributor Author

At first sight, the failures seem to have nothing to do with my code so I won't address them in this PR.

@corranwebster

Copy link
Copy Markdown
Contributor

Thanks for the contributions. Looks like failures are in the svg libraries, so definitely unrelated to this.

Minor criticisms: your hexagons are sufficiently distorted that it bugs me, so you might want to fix. See matplotlib for better ones https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/markers.py#L592. Similarly the pentagons are a bit off in their proportions.

In general for an n-gon rotated by offset radians you want vertices at x = cos(2*pi*i/n + offset)*size and y = sin(2*pi*i/n + offset)*size but for speed you want to hard-code (possibly approximate) values of the sines and the cosines.

Finally, matplotlib's star marker is an actual 5-pointed star: ⭐ I think it makes sense to follow suit, so I'd either remove star from this PR, or follow suit.

Tests would be nice, but I'm not going to insist :)

@jonathanrocher

jonathanrocher commented Apr 27, 2016

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @corranwebster . Agreed with all of the comments and I took the time to do the star properly: Updated plot looks like:
screen shot 2016-04-27 at 2 03 21 pm

Comment thread enable/markers.py Outdated
# How this marker is to be stroked. (Overrides AbstractMarker.)
draw_mode = FILL_STROKE
# Do not render anti-aliased. (Overrides AbstractMarker.)
antialias = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think that we want this: the markers that have this set have corresponding Kiva versions implemented by Agg in C++ without aliasing, so this is to give pixel-to-pixel similarity between the two ways of drawing between different backends. For these, we have no corresponding Agg versions (unless you want to expose the ones here: https://github.com/enthought/enable/blob/dddfac16132051e40d93485333f1c13ea7964631/kiva/agg/agg-24/include/agg_renderer_markers.h by adding them here:

kiva_marker_to_agg = {}
and possibly opengl implementations here:
glMatrixMode(GL_MODELVIEW);
...)

They will draw slightly slower, but look much nicer if antialias is True.

@corranwebster

Copy link
Copy Markdown
Contributor

One last comment above. There are native Agg versions of some of these, so if you are feeling enthusiastic you can expose them and they will render much faster, but that could be a separate PR.

The markers look much nicer.

@jonathanrocher

Copy link
Copy Markdown
Contributor Author

Thanks for the antialiasing comment. Fixed. Thanks for the pointers into Kiva. Will have to delay diving into Kiva and Agg for the future...

@brycehendrix

Copy link
Copy Markdown
Contributor

Excuse me for lurking, but I seem to recall trying to get the Agg markers working many years ago, but they didn't work for some reason. It was probably 6 years ago, so I don't recall the exact reason, though I do know I tried :)

@jonathanrocher

Copy link
Copy Markdown
Contributor Author

Thanks for your lurking and the information @brycehendrix .

@corranwebster corranwebster merged commit d0f1e67 into enthought:master Apr 29, 2016
@jonathanrocher jonathanrocher deleted the feature/add_new_markers branch May 3, 2016 16:22
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.

3 participants