Bug in mpFX x-range calculations causes out-of-range GetY calls

[DRNadler]

I'm trying to graph a time-series with valid indices ~[0-165000].
mpFX correctly sets up m_bound from my GetMinX()/GetMaxX() functions.
But mpFX::DoPlot is indexing before the series (calls GetY with negative x) here:
// Get first point
iylast = w.y2p(DoGetY(w.p2x(m_plotBoundaries.startPx)), m_UseY2Axis);
startPx is set wrong, because m_posX is set to a negative value.
I think this happens because the x scaling factor is computed differently then this line in mpWindow::Fit():
m_posX = (rect.Xmin + rect.Xmax) / 2 - (m_plotWidth / 2 + m_margin.left) / m_scaleX;
Perhaps m_scaleX is computed without the margin?
This may do with the fix introduced here by JL.
Thoughts?
PS: Also indexing past the end of the series several times.

[DRNadler]

I had to write a wrapper class to avoid these bugs as follows:

typedef double TimeseriesValueFetchFunction(const LogInstance_T &logInstance, int idx);
double TEaltitudeAtIdx(const LogInstance_T &logInstance, int idx) { return logInstance.sensorDerivedValues[idx].TEaltitudeM; }
double TEvarioAtIdx(const LogInstance_T &logInstance, int idx) { return logInstance.sensorDerivedValues[idx].TEvario; }

class LogInstanceTimeSeriesPlot_FX: public mpFX
{
    const LogInstance_T &logInstance;
    VisualizationRange vr;
    Generic_3D_BoundingBox<double> limits; // X,Y for current vr
    TimeseriesValueFetchFunction* const V_atIdx;
    double lastSaneValue;
    /// Set bounding box 'limits' for above visualization range 'vr'
    void SetLimits() {
        limits.InitializeBoundingBox(vr.startIdx,GetY(vr.startIdx),0);
        for(int idx=vr.startIdx+1; idx<=vr.endIdx; idx++)
            limits.UpdateBoundingBoxToInclude(idx,GetY(idx),0);
    }
  public:
    LogInstanceTimeSeriesPlot_FX(
        const LogInstance_T &logInstance_,
        const VisualizationRange &vr_,
        TimeseriesValueFetchFunction &V_atIdx_,
        const wxColour& color,
        bool useY2Axis_ = false
    ) :
        logInstance(logInstance_),
        vr(vr_),
        V_atIdx(V_atIdx_),
        mpFX(logInstance_.logfileName,mpALIGN_RIGHT,useY2Axis_)
    {
        // MathPlot initialization
        wxPen FXpen(color, 2, wxPENSTYLE_SOLID);
        SetDrawOutsideMargins(false);
        SetContinuity(true);
        SetPen(FXpen);
        SetSymbol(mpsNone);
        // Set bounding box 'limits' for initial visualization range 'vr'
        SetLimits();
    }
    virtual double GetMinX() { return limits.Xmin; }
    virtual double GetMaxX() { return limits.Xmax; }
    virtual double GetMinY() { return limits.Ymin; }
    virtual double GetMaxY() { return limits.Ymax; }
    virtual double GetY(double x) {
        int idx = (int)x;
        // Avoid MathPlot bugs fetching out-of-range
        if     (idx<vr.startIdx) return GetY(vr.startIdx);
        else if(idx>vr.endIdx)   return GetY(vr.endIdx);
        double r = V_atIdx(logInstance,idx);
        // catch crazy values (would should have been filtered out upstream, except...)
        if(r>10000 || r<-10000) {
            // ooops
            r = lastSaneValue;
        } else {
            lastSaneValue = r;
        }
        return r;
    }
    void SetDefaultVisualizationRange() {
        vr.SetDefaultVisualizationRange(logInstance);
        SetLimits();
    };
    void SetVisualizationRange(const VisualizationRange &newVr) {
        vr = newVr;
        SetLimits();
    };
};

[GitHubLionel]

Well difficult to help you. Could you make a simple test code (like in demo examples) with which I can reproduce the problem.

[DRNadler]

I'll try to do so today, Thanks...

[DRNadler]

Here's a very minimal example, hope it helps!

Important Note: First step to sorting this out is fix documentation of all the mpWindow and maybe plot members where units are not specified, which directly and always leads to bugs like this. I fixed m_bound when it tripped me up, but everything below (ie m_scaleX) should specify units and whether boundary is included... This bug is caused by inconsistent interpretation of what for example m_scaleX really includes...

/// Range within timeseries to visualize for analysis, by observation index
struct TimeseriesRange {
    int startIdx=0, endIdx=0;
    constexpr TimeseriesRange() = default;
    constexpr TimeseriesRange(int s, int e) : startIdx(s), endIdx(e) {};
};

class BuggyTimeseriesPlot_FX: public mpFX
{
    const TimeseriesRange  vr = TimeseriesRange(0,150000);
    const double maxY = 150.0;
  public:
    BuggyTimeseriesPlot_FX() : mpFX("Buggy Timeseries",mpALIGN_RIGHT)
    {
        wxPen FXpen(*wxRED, 2, wxPENSTYLE_SOLID);
        SetPen(FXpen);
        SetDrawOutsideMargins(false);
        SetContinuity(true);
        SetSymbol(mpsNone);
    }
    virtual double GetMinX() { return vr.startIdx; }
    virtual double GetMaxX() { return vr.endIdx;   }
    virtual double GetMinY() { return 0; }
    virtual double GetMaxY() { return maxY; }
    virtual double GetY(double x) {
        int idx = (int)x;
        // Asserts below to easily trap out-of-range indexing
        assert(idx>=vr.startIdx);
        assert(idx<=vr.endIdx);
        return maxY * x / vr.endIdx; 
    }
};

[GitHubLionel]

Ok, I test your sample with a simple :
BuggyTimeseriesPlot_FX *buggy = new BuggyTimeseriesPlot_FX();
m_plot->AddLayer(buggy, true);
and with date format on x axis. I have a bug when I undo zoom about year 3000 ! lol
This is gmtime() function who is crashing. I can just add a try catch for that.

But you have right, documentation is not clear about scale range ...

[DRNadler]

I did not use date format on axis (no axis labels).
Did you observe the assert failures showing the bug I see?

[GitHubLionel]

Well, finally, I think that you use mpFX in a wrong way. This class is for function that they have no x axis limit.
See for example MySIN in Sample.h of demo project.
You can see that we not define GetMinX() or GetMaxX() because there is not limits (the limit is given by the scale of the x axis).

So in your sample, I think that :

• no need to GetMinX() and GetMaxX()
• the GetY(double x) function should be :
  if ((idx > vr.startIdx) && (idx < vr.endIdx)) return maxY * x / vr.endIdx; else return 0;
• In the X axis, you should define :
  bottomAxis->SetAuto(false); bottomAxis->SetMinScale(0); bottomAxis->SetMaxScale(150000);

[DRNadler]

I have no bottomAxis... How do I do set X range within a plot layer?
This is necessary for time-series...

[DRNadler]

If you breakpoint GetMinX in example, you will see GetMinX() and GetMaxX() ARE called for example bug.

[GitHubLionel]

Yes GetMinX is called and it use the default range : [-1, 1] over X and same for Y axis.
Probably, the behaviour is not what you expect. I suppose that you don't want to have plot outside your bound (ie [0,150000] in your example).
For that, mpFX is not the good candidate. mpFXYVector is probably better.

[DRNadler]

As GetMinX() and GetMaxX() are virtual, my implementations are called, so it is not using the default values...