CustomMatPlot icon indicating copy to clipboard operation
CustomMatPlot copied to clipboard

Plotting multiple same Y points causes JUCE assertions, inf values

Open ad3154 opened this issue 1 year ago • 2 comments

There are situations where divisions by 0 occur in the library which cause JUCE assertions, inf or NaN values which could be prevented by some trivial checks.

Take for example the situation where you might plot 2 points, with different X values, but the same Y value and are using linear scaling. In this case for example, in getYScaleAndOffset we can see that y_lim.max - y_lim.min is 0, which causes y_scale and y_offset to be inf.

The subsequent JUCE assertion is in:

void Path::startNewSubPath (const float x, const float y)
{
    JUCE_CHECK_COORDS_ARE_VALID (x, y)

Which occurs because JUCE checks for NaNs: image

I suggest adding some perfunctory checks around these cases to set things like scale factors to sensical values in these cases or to at least prevent the JUCE assertions.

Such as:

y_scale = height / (y_lim.max - y_lim.min);
y_offset = height + (y_lim.min * y_scale);
if (std::isinf(y_scale))
	y_scale = 1.0f;
if (std::isinf(y_offset))
	y_offset = 0.0f;

or if checks for things like if (0 != y_lim.max - y_lim.min) then do something else.

This repo doesn't have a CONTRIBUTING file to explain if/how you accept PRs from random other people, but if you'd accept outside contributions I could put together a PR to catch some of these situations, otherwise checking these cases would be much appreciated, since plotting 2 points with the same Y value is a "normal" or at least "valid" use case I think.

ad3154 avatar Jan 17 '24 20:01 ad3154

Thanks for pointing out the issue. Yes, I agree. We should add some checks for this asap. I will update the CONTRIBUTING this weekend.

FransRosencrantzz avatar Jan 19 '24 00:01 FransRosencrantzz

Sorry for the late response, here is the CONTRIBUTING file: https://github.com/franshej/CustomMatPlot/blob/master/CONTRIBUTING.md

franshej avatar Jan 28 '24 10:01 franshej