Nice find on this @pijyoi ...would never have guessed that 50 points along the path to be the "optimum" size to break it up by...

I was really expecting the fill to take a significant performance hit still but it wasn't anywhere near as bad as I thought it would be. On macOS on PyQt6 6.2 I'm seeing a ~25x gain in performance, nice work!

An artificial example that chokes on master (slow to come up with plot, panning not usable) but works fine on this PR.
master works fine if the line y[::2] *= -1 is commented out. i.e. the complexity of the curve plays a part.
I really have no idea how people are making use of fillLevel, but it seems to me that for many kinds of real-world plots, master would work just fine.

import pyqtgraph as pg
import numpy as np

def gaussian(x, mu, sig):
    return np.exp(-np.power(x - mu, 2.) / (2 * np.power(sig, 2.)))

pg.mkQApp()
pw = pg.PlotWidget()
pw.show()
item = pg.PlotCurveItem()
item.setPen(None)
pw.addItem(item)

x = np.linspace(-3, 3, 10001)
mid = (x[:-1] + x[1:])/2
y = gaussian(mid, 0, 1)
y[::2] *= -1
item.setData(x, y, stepMode="center", fillLevel=0, brush='b')

pg.exec()

I really have no idea how people are making use of fillLevel, but it seems to me that for many kinds of real-world plots, master would work just fine.

Whenever I'm curious about this sort of thing, I typically use github search; only "example" I found with filllevel not being 0 or None was this guy:

https://github.com/trieschlab/PymoNNto/blob/759e629568531d35b8370ad8058d4bdaadea00d0/Exploration/Network_UI/Basic_Tabs/fourier_tab.py#L132-L135

What this looks like GUI wise, I'm not sure

@goodboy, you have mentioned before on Slack that you use stepMode together with fillLevel. The artificial example above runs smoothly at 10,000 data points. Would you be able to try out this PR on your code?

For stepMode at least, there seems to be a legitimate use for having fillLevel enabled but brush disabled. (i.e. draw a histogram outline w/o filling) But in this particular case, fillLevel would be a misnomer.

import pyqtgraph as pg
import numpy as np

pg.mkQApp()
glw = pg.GraphicsLayoutWidget(show=True)

x = np.arange(12)
y = np.array([10, 9, 8, 7, 6, 5, 6, 7, 8, 9, 10])

PCI = pg.PlotCurveItem
glw.addPlot().addItem(PCI(x, y, stepMode="center", fillLevel=None))
glw.addPlot().addItem(PCI(x, y, stepMode="center", fillLevel=0))
glw.addPlot().addItem(PCI(x, y, stepMode="center", fillLevel=0, brush='b'))
glw.addPlot().addItem(PCI(x, y, stepMode="center", fillLevel=0, brush='b', fillOutline=True))

pg.exec()

I think that further supports 'fillLevel is not None' as the main toggle switch for handling the area under the curve. (Rather than the brush setting.)

Having "fill" in the name really helps associate it with that, even if it isn't 100% applicable for some less common applications. It might be good to add a reference to fillLevel to the documentation of stepMode:
"When stepmode is active, the fillLevel parameter can be set to add left and right enclosing edges to the curve. fillLevel=0 is commonly used to draw historgrams."
... or something similar.

It might be good to add a reference to fillLevel to the documentation of stepMode:
"When stepmode is active, the fillLevel parameter can be set to add left and right enclosing edges to the curve. fillLevel=0 is commonly used to draw historgrams."

I can't really tell if that isn't documenting accidental behavior and making it official...

I feel that a more general solution would be just a fill=True or False parameter that would take the place of fillLevel, fillOutline. User provides all the x, y coords necessary. PlotCurveItem doesn't add any extra line segments nor does it do any closeSubpath. (API changes are out of scope of this PR of course) EDIT: doesn't quite work, people currently want to fill the area underneath the curve but not have it be enclosed

Alternative approach:

Assume that a future PR might

• alias fillOutline to drawOutline and allow options True/False/'vertical' to handle the edge lines.
• alias fillLevel to baseline, with a default of 0.
• alias brush to fillBrush.

Then the fillBrush parameter (now brush) would work as the main toggle for handling the area under the curve. Compatibility would be maintained, except that the currently unclear case of "fillLevel set, but no brush specified" would take on the clear meaning of "no fillBrush->no fill, even if baseline is set for other purposes".

Turning on the fill would then again require only one argument (since baseline defaults to 0.0, which is the desirable value 90% of the time).

If we go in that direction, then the toggle for fill/no fill can be a check for brush is None (and nothing else). Does that make more sense to you? I still think the code should only check one specific parameter to decide which code-path to run. Everything else is a headache to keep track of and will probably break at some point. :)

I think paint() could be simplified if some of the options were "emulated".
So let's say paint() is made to support (which it already does):

1. fillLevel=None
2. fillLevel=finite_value : close the path with left and right vertical lines and a horizontal line for filling purposes
3. fillLevel='enclosed' : assume (x,y) values already form a closed shape, fill that closed shape(s)

Emulated options:

1. stepMode=None, fillOutline=True
   • on setData(), add to (x,y) values so that we get a closed shape
   • then use the case fillOutline=False, fillLevel='enclosed'
2. stepMode='center', fillLevel=None
   • on setData(), generate the (x, y) step values w/o the left and right vertical lines
   • then use stepMode=None, fillLevel=None
3. stepMode='center', fillLevel=finite_value
   • on setData(), generate the (x, y) step values with the left and right vertical lines
   • then use stepMode=None, fillLevel=finite_value
   • Whether filling occurs depends on brush
4. stepMode='center', fillLevel=finite_value, fillOutline=True
   • on setData(), generate the (x, y) step values and also close the shape
   • then use stepMode=None, fillOutline=False, fillLevel='enclosed'

That sounds like a good strategy.

I suspect that we would still want an adjustable baseline value even in enclosed mode, unless I misunderstand that mode. As above, fillLevel can either be 'enclosed' OR a numerical value. So I would suggest differentiating the cases with "no extra lines", "left and right lines" and "full enclosure" through the parameter that is now fillOutline.

Note that this only affects the first-stage handler that adds the lines. For the second stage, the suggested assignments seem like they should work. I believe the numeric fillLevel option is needed in that case to generate the "filled but not outlined" section, right? In the Qt rendering path, the line on the bottom is handled by setting the "close" flag for drawing the same set of points... I would guess.

Once we know what everything does, we can define and document some aliases :)

I am not confident that it can be easily done without breaking existing behavior (intended or otherwise) after all.
Take fillOutline as an example, which at first glance seems simple enough:

1. We can't just convert it to enclosed in setData(). It would also need to be done in setFillLevel()
2. It changes the meaning of mouseShape() from the un-enclosed curve to the enclosed shape.

Add support for fillOutline.

1. thick pen codepath can have fillOutline
2. shadowPen now knows about fillOutline

import pyqtgraph as pg
import numpy as np

pg.mkQApp()
glw = pg.GraphicsLayoutWidget(show=True)

x = np.arange(12)
y = np.array([10, 9, 8, 7, 6, 5, 6, 7, 8, 9, 10])

pen = pg.mkPen('w', width=1)
spen = pg.mkPen((0,80,0), width=10)

PCI = pg.PlotCurveItem
glw.addPlot().addItem(PCI(x, y, pen=pen, shadowPen=spen, stepMode="center", fillLevel=None))
glw.addPlot().addItem(PCI(x, y, pen=pen, shadowPen=spen, stepMode="center", fillLevel=0))
glw.addPlot().addItem(PCI(x, y, pen=pen, shadowPen=spen, stepMode="center", fillLevel=0, brush='b'))
glw.addPlot().addItem(PCI(x, y, pen=pen, shadowPen=spen, stepMode="center", fillLevel=0, brush='b', fillOutline=True))

pg.exec()

@pijyoi this is remarkable, also this might be the first time I've seen shadowPen in actual use too. I'll try and set some time aside today to break this. The example you give is pretty good, and I'm wondering if we should incorporate it into one of the examples.

EDIT: just a note on the example, we should probably not merge red and green in such a way, they can be difficult to distinguish for individuals that are colorblind.

just a note on the example, we should probably not merge red and green in such a way, they can be difficult to distinguish for individuals that are colorblind.

Thanks, I have modified it.

The example is less an example of how to use and more a check that this PR is maintaining prior behavior (intended or otherwise).
There is a change in behavior for the 4th plot, although it seems safe to assume that the previous behavior of no shadow pen for the fillOutline horizontal segment was an over-sight / bug .

The example is less an example of how to use and more a check that this PR is maintaining prior behavior (intended or otherwise).

That's completely fair. My broader point is I don't think the naming of these options is not clear what the result should look like, I think this is a use-case where just a blurb in the docs in insufficient, and where a picture speaks a thousand words... Added benefit is that they can be checks when making changes (perhaps we should add this to one images in the test suite?

I agree that having plots in the documentation would help in illustrating their use. The issue I have is that there is an inconsistency between stepMode and non-stepMode as shown in the script below.

So it might be a good idea to hold off the documentation till we have a more consistent api as discussed in #2032 (comment)

Interestingly (for me), the bottom-row, 2nd-col plot shows that dataBounds() "knows" about fillLevel. Actually, this particular plot should probably be considered a non-valid use-case.

import pyqtgraph as pg
import numpy as np

pg.mkQApp()
glw = pg.GraphicsLayoutWidget(show=True)

x = np.arange(12)
y = np.array([10, 9, 8, 7, 6, 5, 6, 7, 8, 9, 10])
xc = (x[1:] + x[:-1])/2

pen = pg.mkPen('w', width=1)
spen = pg.mkPen((0,80,0), width=10)

PCI = pg.PlotCurveItem
common = dict(pen=pen, shadowPen=spen, stepMode="center")
glw.addPlot().addItem(PCI(x, y, **common, fillLevel=None))
glw.addPlot().addItem(PCI(x, y, **common, fillLevel=0))
glw.addPlot().addItem(PCI(x, y, **common, fillLevel=0, brush='b'))
glw.addPlot().addItem(PCI(x, y, **common, fillLevel=0, brush='b', fillOutline=True))
glw.nextRow()
common = dict(pen=pen, shadowPen=spen)
glw.addPlot().addItem(PCI(xc, y, **common, fillLevel=None))
glw.addPlot().addItem(PCI(xc, y, **common, fillLevel=0))
glw.addPlot().addItem(PCI(xc, y, **common, fillLevel=0, brush='b'))
glw.addPlot().addItem(PCI(xc, y, **common, fillLevel=0, brush='b', fillOutline=True))

pg.exec()

It seems strangely unsatisfying that the top and bottom version of the third plot (filled, no outline) come out differently in terms of the vertical edge lines.

@pijyoi, can your code give us direct access to switch if these lines are plotted not?
I think defaulting to the version shown in your example is a good idea, but it would be nice to have the option of plotting a filled in stepped plot without the lines.

An example use might be a display for digital I/O signals? A vertical line at start/end might then falsely imply a transition. (I admit that that is not a great example, and I'd just use the current version anyway.)

There would be a nice sense of symmetry if the lines could also be manually turned on for the non-stepped plot... I can't currently come up with a reasonable example for that, but my impression from
#2032 (comment)_
was that you think the edge lines can be factored out of the stepmode code to be a separate processing step.

From an API perspective, I still think the edge line switching could be done through the existing fillOutline parameter, since it seems that it never makes sense to switch on both the edge lines and the full outline at the same time.

But this is all nit-picking. If you don't see a way to handle this in an easy AND satisfying way, then I am good with the functionality as you have it now.

Documentation-wise, I think it might be acceptable to merge the functionality first, since the code seems to provide the same output with the same parameter settings in all typical use cases. I might be able to help with a documentation path after that, but I'd prefer not to add any confusion to the code updates before then.

@j9ac9k , what do you think is the best merging procedure?

So the thing is that PlotCurveItem is suitable for plotting series data (where the x-ordinate increases monotonically), but it can also be used for plotting arbitrary (x, y) and in particular can be used for drawing closed shapes.

From this point of view, stepMode is redundant. The user could just as well provide the (x, y) created by _generateStepModeData().

Even if the current API may seem non-orthogonal, there are users using stepMode="center", fillLevel=True and stepMode="center", fillLevel=True, fillOutline=True. The latter usage probably stems from examples/histogram.py.

There would be a nice sense of symmetry if the lines could also be manually turned on for the non-stepped plot

So this would also be redundant for the same reason.

But this is all nit-picking. If you don't see a way to handle this in an easy AND satisfying way, then I am good with the functionality as you have it now.

It's not easy because the existing behavior is like this:

1. self.path : data provided by the user and takes part in mouse shape.
2. self.fillPath : includes path needed to close the shape in order to fill it; takes part in dataBounds computation.

So stepMode vertical lines belong to self.path due to its original implementation.
Adding vertical lines for non-stepMode should probably belong to self.fillPath because that's not data provided by the user.

Definitely not satisfying...

As for adding more API: https://xkcd.com/927/

(Idle API discussion)
I think the API is actually pretty good. Only the names of the parameters leave room for improvement.
If 'filllevel' was 'baseline', then it makes sense that this affects the shape of the edge lines, and even that auto-scale wants to include this value.
If 'fillOutline' was 'drawOutline' or 'closeOutline', then it would be clearer that the main function is to close the outline, not to fill it.
In this convention, it would be specifically brush=QBrush (or even better fillBrush, which is what PlotDataItem already uses to distinguish this from ScatterPlotItem's point brush) that activates filling, since 'baseline' is disconnected from the concept of filling. That still matches the previous behavior, where -it seems- setting a brush was always required to activate filling.

(Main PR discussion)
Can we look at this from the point of view that

• non-step mode is now healthy and without too many strange compatibility patches (such as patching in the edge lines)
• step mode includes some compromises to keep compatibility
  ?

Then our long-term strategy might indeed be to find a convenient way to move the entire step mode system out of PlotCurveItem, either by moving the functionality to PlotDataItem, or by making a fully-featured API for _generateStepModeData().

Once we figure something out that is good enough to justify the "one more standard" syndrome, we would slowly move to deprecate having this stepmode functionality hard-coded into PlotCurveItem.

In that case, are there still open issues to be solved?
Or is this good to go?

FWIW we ended up implementing a version of the stepMode with similar perf to our version of a "fast update" version of PlotCurveItem and also found this fillLevel stuff to be horrendously slow for any moderate sequence length.

I'm very interested in seeing if these improvements can be swapped in.
Code is in the above linked PR.

I think that further supports 'fillLevel is not None' as the main toggle switch for handling the area under the curve. (Rather than the brush setting.)

I found an in-library usage that assumes the above:
https://github.com/pyqtgraph/pyqtgraph/blob/master/pyqtgraph/graphicsItems/HistogramLUTItem.py#L191-L211

As for adding more API: https://xkcd.com/927/

I need some kind of chrome extension for obscuring xkcd urls as I knew which one this was from the number alone.

I think that further supports 'fillLevel is not None' as the main toggle switch for handling the area under the curve. (Rather than the brush setting.)

I found an in-library usage that assumes the above: https://github.com/pyqtgraph/pyqtgraph/blob/master/pyqtgraph/graphicsItems/HistogramLUTItem.py#L191-L211

I suspect this is probably what we would want (independent of the HistogramLUTItem; but having that be in practice elsewhere certainly helps).

Since creating #2079 to discuss the API inconsistency issues, I think we can merge this PR 👍🏻

Thanks for the PR @pijyoi I think you might be running out of areas to speed up 😆