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

Implemented Step Signal Plot #1128

Merged
merged 4 commits into from Jul 24, 2021
Merged

Implemented Step Signal Plot #1128

merged 4 commits into from Jul 24, 2021

Conversation

EFeru
Copy link
Contributor

@EFeru EFeru commented Jun 20, 2021

Purpose:
Step Display is very useful to show digital signals.

New Functionality: This PR implements #1092

  • added function for StepDisplay (take-over from Scatter Plot)
  • added Cookbook example
  • all tests are passing

image

- added function for `StepDisplay` (take-over from Scatter Plot)
- added Cookbook example
- all tests are passing
@bclehmann
Copy link
Member

The step plot looks a little weird with dots drawn. I don't know if that's something that can/should be changed. Maybe just changing the demo to not draw marker points is all that should be done?

@StendProg
Copy link
Contributor

Hi @EmanuelFeru,

Good improvement, thanks for your contribution.

Unfortunately, SignalXY will inherit StepDisplays, but will not react to it in any way. There is no simple solution, I think it is better to leave it as it is for now.

It seems that the StepDisplay shows no respect for the existing signal fill above/below. At first glance, it looks easy, it is enough to transfer the already formed array to the code below, but in the process, difficulties may arise. At least it looks like it's worth at least a try.
What do you think?

The step plot looks a little weird with dots drawn. I don't know if that's something that can/should be changed. Maybe just changing the demo to not draw marker points is all that should be done?

It also immediately caught my eye, most likely the lack of symmetry gives such an effect.

@EFeru
Copy link
Contributor Author

EFeru commented Jun 20, 2021

Yes, it's the symmetry. I was looking strange at the beginning too 😃 the dots can be hidden by making the Markersize 0. What is your opinion for the example? I can put the Markersize to 0 to have a more natural look.

@StendProg, I did not fully understood. Do you mean if we do surface below for example then it still considers straight line between points?

@StendProg
Copy link
Contributor

@StendProg, I did not fully understood. Do you mean if we do surface below for example then it still considers straight line between points?

Exactly. The fill in the background will connect the dots and will probably be ugly. Maybe we don't need it, but most likely sooner or later this question will come up, it's up to you, I just noticed that the fill uses the old array of points in stepDisplay mode.

@EFeru
Copy link
Contributor Author

EFeru commented Jun 20, 2021

I see... Then it's better to have it done now. I will have a look in the next couple of days.

@EFeru
Copy link
Contributor Author

EFeru commented Jun 24, 2021

I quickly checked FillBelow. Indeed it does not draw well around the steps. Still investigating how to make it plot steps. I think a change is needed on the Rectangle fill:

Rectangle gradientRectangle = new(
x: (int)first.X,
y: (int)minVal - (fillType == FillType.FillAbove ? 2 : 0),
width: (int)(last.X - first.X),
height: (int)(maxVal - minVal) + 2 * (fillType == FillType.FillAbove ? -1 : 1));

image

Code for the above plot:

public void ExecuteRecipe(Plot plt)
{
    double[] ys = DataGen.Sin(51);
    int sampleRate = 1;

    var sig = plt.AddSignal(ys, sampleRate);
    sig.StepDisplay = true;
    sig.MarkerSize = 0;
    sig.FillType = FillType.FillBelow;
    sig.FillColor1 = Color.Blue;

}

Also I am not sure if the fill is done correctly, it behaves strange when I move and looks diagonal. I would expect to fill similar to here:
https://swharden.com/scottplot/cookbooks/4.1.16/category/plottable-fill/#fill-under-curve
image

@EFeru
Copy link
Contributor Author

EFeru commented Jun 24, 2021

Fixed. I will do a commit.
The solution was to use the pointsStep instead of pointsArray, when calling FillAboveOrBelow(...) and FillAboveAndBelow(...)

Edit: Not sure if it is the most efficient implementation but it does the job. I think better would be to plot square rectangles using the current point (X_k, 0) and next point (X_k+1, Y_k+1).

image

- Fill Above Below is consistent with `StepDisplay`
- made `MarkerSize = 0` in the Cookbook for a more natural look
@EFeru
Copy link
Contributor Author

EFeru commented Jun 24, 2021

image

@swharden
Copy link
Member

Hi @EmanuelFeru, thank you for this PR! Thanks @bclehmann and @StendProg for your input along the way.

I'm going to refine this PR and merge it in shortly despite it not looking perfect with filled signal plots. It looks like filled signal plots have slight rendering errors already, and I'll work to investigate and hopefully fix that in a separate issue (#1163)

@swharden swharden linked an issue Jul 24, 2021 that may be closed by this pull request
@swharden
Copy link
Member

Thanks @EmanuelFeru, this looks great! This feature will be in the next release 👍🚀

I'll fix the error affecting the alignment of fills shortly (#1163)

@swharden swharden merged commit d1ace02 into ScottPlot:master Jul 24, 2021
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.

Add "step" option for low density rendering mode in Signal plots
4 participants