Chronological Thread 

< Chronological >      < Thread >

---

Hi

I'm guessing this is your "isNan" test: "if (FirstPoint.X != FirstPoint.X)"

I think it would be preferable to refactor this via extract method to an
isNan method, residing in a numerical utilities module. Or, to use the std
library isNan.

Also, I think the "magic numbers: 6 (used for drawEvery) and 1024 " should
be refactored via extract constant.

Overall, I think this Draw method could do with a lot of refactoring
targeted at improving readability, and hopefully make it easier to maintain.

Regards,
Randall

> -----Original Message-----
> From: automated-notifications-bounces at cellml.org [mailto:automated-
> notifications-bounces at cellml.org] On Behalf Of CellML Automated
> Notifications
> Sent: Thursday, 10 April 2008 10:32 p.m.
> To: automated-notifications at cellml.org
> Subject: r2194 - pce/trunk/DataCollector
>
> Author: jmar150
> Date: 2008-04-10 22:31:44 +1200 (Thu, 10 Apr 2008)
> New Revision: 2194
>
> Modified:
> pce/trunk/DataCollector/DataCollector.cpp
> Log:
> Fixed some problems with interpolating over small discontinuities in
> graphing.
>
>
>
> Modified: pce/trunk/DataCollector/DataCollector.cpp
> ===================================================================
> --- pce/trunk/DataCollector/DataCollector.cpp 2008-04-09 22:25:42 UTC
> (rev 2193)
> +++ pce/trunk/DataCollector/DataCollector.cpp 2008-04-10 10:31:44 UTC
> (rev 2194)
> @@ -490,7 +490,7 @@
> NS_ENSURE_TRUE(mRenderContext,);
> NS_ENSURE_TRUE((mxVariable < nvar) && (myVariable < nvar),);
>
> - PRUint32 drawEvery = 4;
> + PRUint32 drawEvery = 6;
> while (PR_TRUE)
> {
> mRenderContext->BeginPath();
> @@ -514,6 +514,7 @@
> };
>
> IndexedPoint ExtremePoints[4];
> + IndexedPoint FirstPoint, LastPoint;
>
> if (mGlyph == IDataToCanvasRequest::GLYPH_LINES)
> {
> @@ -529,6 +530,9 @@
> mRenderContext->BeginPath();
> double lastX = std::numeric_limits<double>::quiet_NaN(),
> lastY = std::numeric_limits<double>::quiet_NaN();
> + FirstPoint.X = std::numeric_limits<double>::quiet_NaN();
> + FirstPoint.Y = std::numeric_limits<double>::quiet_NaN();
> + FirstPoint.index = start;
> for (i = start; i <= end - skip; i += skip, p += gap, q += gap)
> {
> PRFloat64 x1 = p[mxVariable], y1 = p[myVariable];
> @@ -615,61 +619,77 @@
> }
> }
> }
> -
> + bool discontinuity(false);
> if (validPointCount != 2)
> {
> - continue;
> + if (!countOnlyMode || drawEvery == 6)
> + continue;
> + discontinuity = true;
> }
>
> // Here I am using countOnlyMode to signal the beginning of a
> sampling cycle, and
> // pointsSoFar to determine both distance through a sample set
> and number of samples
> - // at the current granularity. drawEvery is the sampling
> period; at 4, this equates to drawing
> - // every point
> + // at the current granularity. drawEvery is the sampling
> period; at 6, we may as well draw every point.
> pointsSoFar++;
> if (pointsSoFar > 1024)
> {
> pointsSoFar = pointsSoFar % drawEvery;
> - drawEvery+=4;
> + drawEvery+=6;
> }
>
> - if (!countOnlyMode && drawEvery != 4)
> + if (!countOnlyMode && drawEvery != 6)
> {
> countOnlyMode = PR_TRUE;
> - ExtremePoints[MIN_X].X = ExtremePoints[MAX_X].X =
> - ExtremePoints[MIN_Y].X = ExtremePoints[MAX_Y].X = xI[1];
> - ExtremePoints[MIN_X].Y = ExtremePoints[MAX_X].Y =
> - ExtremePoints[MIN_Y].Y = ExtremePoints[MAX_Y].Y = yI[1];
> - ExtremePoints[MIN_X].index = ExtremePoints[MAX_X].index =
> - ExtremePoints[MIN_Y].index = ExtremePoints[MAX_Y].index =
> i;
> + if (FirstPoint.X != FirstPoint.X)
> + {
> + FirstPoint.X = xI[1];
> + FirstPoint.Y = yI[1];
> + FirstPoint.index = i;
> + }
> + LastPoint.X =
> + ExtremePoints[MIN_X].X = ExtremePoints[MAX_X].X =
> + ExtremePoints[MIN_Y].X = ExtremePoints[MAX_Y].X = xI[1];
> + LastPoint.Y =
> + ExtremePoints[MIN_X].Y = ExtremePoints[MAX_X].Y =
> + ExtremePoints[MIN_Y].Y = ExtremePoints[MAX_Y].Y = yI[1];
> + LastPoint.index =
> + ExtremePoints[MIN_X].index = ExtremePoints[MAX_X].index =
> + ExtremePoints[MIN_Y].index = ExtremePoints[MAX_Y].index
> = i;
> }
> if (countOnlyMode)
> {
> - if (xI[1] < ExtremePoints[MIN_X].X)
> + if (!discontinuity)
> {
> - ExtremePoints[MIN_X].X = xI[1];
> - ExtremePoints[MIN_X].Y = yI[1];
> - ExtremePoints[MIN_X].index = i;
> + if (xI[1] < ExtremePoints[MIN_X].X)
> + {
> + ExtremePoints[MIN_X].X = xI[1];
> + ExtremePoints[MIN_X].Y = yI[1];
> + ExtremePoints[MIN_X].index = i;
> + }
> + if (xI[1] > ExtremePoints[MAX_X].X)
> + {
> + ExtremePoints[MAX_X].X = xI[1];
> + ExtremePoints[MAX_X].Y = yI[1];
> + ExtremePoints[MAX_X].index = i;
> + }
> + if (yI[1] < ExtremePoints[MIN_Y].Y)
> + {
> + ExtremePoints[MIN_Y].X = xI[1];
> + ExtremePoints[MIN_Y].Y = yI[1];
> + ExtremePoints[MIN_Y].index = i;
> + }
> + if (yI[1] > ExtremePoints[MAX_Y].Y)
> + {
> + ExtremePoints[MAX_Y].X = xI[1];
> + ExtremePoints[MAX_Y].Y = yI[1];
> + ExtremePoints[MAX_Y].index = i;
> + }
> + LastPoint.X = xI[1];
> + LastPoint.Y = yI[1];
> + LastPoint.index = i;
> }
> - if (xI[1] > ExtremePoints[MAX_X].X)
> + if (pointsSoFar % drawEvery == 0 || discontinuity)
> {
> - ExtremePoints[MAX_X].X = xI[1];
> - ExtremePoints[MAX_X].Y = yI[1];
> - ExtremePoints[MAX_X].index = i;
> - }
> - if (yI[1] < ExtremePoints[MIN_Y].Y)
> - {
> - ExtremePoints[MIN_Y].X = xI[1];
> - ExtremePoints[MIN_Y].Y = yI[1];
> - ExtremePoints[MIN_Y].index = i;
> - }
> - if (yI[1] > ExtremePoints[MAX_Y].Y)
> - {
> - ExtremePoints[MAX_Y].X = xI[1];
> - ExtremePoints[MAX_Y].Y = yI[1];
> - ExtremePoints[MAX_Y].index = i;
> - }
> - if (pointsSoFar % drawEvery == 0)
> - {
> // Now we render the maximal and minimal points among the
> sample set
> // First, calculate an order permutation...
> PRUint32 tb[4] = {0, 1, 2, 3};
> @@ -679,34 +699,53 @@
> ExtremePoints);
>
> #define CHECK_DUP(a, b, c) \
> -if (!((ExtremePoints[tb[a]].X==ExtremePoints[tb[b]].X)&& \
> - (ExtremePoints[tb[a]].Y==ExtremePoints[tb[b]].Y))) \
> +if(!((a.X==b.X)&&(a.Y==b.Y))) \
> c;
> -
> - if (lastX != ExtremePoints[tb[0]].X || lastY !=
> ExtremePoints[tb[0]].Y)
> - mRenderContext->LineTo(mxMultiplier *
> ExtremePoints[tb[0]].X +
> +#define CHECK_DUPE(a, b, c) \
> +CHECK_DUP(ExtremePoints[tb[a]], ExtremePoints[tb[b]], c)
> + if (lastX != FirstPoint.X || lastY != FirstPoint.Y)
> + mRenderContext->MoveTo(mxMultiplier * FirstPoint.X +
> mxOffset,
> - myMultiplier *
> ExtremePoints[tb[0]].Y +
> + myMultiplier * FirstPoint.Y +
> myOffset);
> - CHECK_DUP(0, 1,
> + CHECK_DUP(FirstPoint, ExtremePoints[tb[0]],
> mRenderContext->LineTo
> + (mxMultiplier * ExtremePoints[tb[0]].X +
> mxOffset,
> + myMultiplier * ExtremePoints[tb[0]].Y +
> myOffset));
> + CHECK_DUPE(0, 1,
> + mRenderContext->LineTo
> (mxMultiplier * ExtremePoints[tb[1]].X +
> mxOffset,
> myMultiplier * ExtremePoints[tb[1]].Y +
> myOffset));
> - CHECK_DUP(1, 2,
> + CHECK_DUPE(1, 2,
> mRenderContext->LineTo
> (mxMultiplier * ExtremePoints[tb[2]].X +
> mxOffset,
> myMultiplier * ExtremePoints[tb[2]].Y +
> myOffset));
> - CHECK_DUP(2, 3, mRenderContext->LineTo
> + CHECK_DUPE(2, 3, mRenderContext->LineTo
> (mxMultiplier * ExtremePoints[tb[3]].X +
> mxOffset,
> myMultiplier * ExtremePoints[tb[3]].Y +
> myOffset));
> - lastX = ExtremePoints[tb[3]].X;
> - lastY = ExtremePoints[tb[3]].Y;
> + CHECK_DUP(ExtremePoints[tb[3]], LastPoint,
> + mRenderContext->LineTo
> + (mxMultiplier * LastPoint.X + mxOffset,
> + myMultiplier * LastPoint.Y + myOffset));
> +
> + lastX = LastPoint.X;
> + lastY = LastPoint.Y;
> + if (!discontinuity)
> + {
> + FirstPoint = LastPoint;
> + }
> + else
> + {
> + FirstPoint.X = std::numeric_limits<double>::quiet_NaN();
> + FirstPoint.Y = std::numeric_limits<double>::quiet_NaN();
> + }
> countOnlyMode = PR_FALSE;
> continue;
> +#undef CHECK_DUPE
> #undef CHECK_DUP
> }
> }
> - if (drawEvery == 4)
> + if (drawEvery == 6)
> {
> // In this case, we render every point anyway
> // We now have two points in the boundary. Stroke a path...
>
> _______________________________________________
> automated-notifications mailing list
> automated-notifications at cellml.org
> http://www.cellml.org/mailman/listinfo/automated-notifications

---

• [cellml-dev] r2194 - pce/trunk/DataCollector, Randall Britten, 04/10/2008

---

Archive powered by MHonArc 2.6.18.