-1

I have a problem with modifying my code based on Bézier curve (which works) to B-spline. This is how it draws:

Text

Basic formula: Text

void MainWindow::rysujKrzywa(int koniec){
    double x, y;
    tm = m - 3;
    for( double t = 0; t <= m-2; t += 0.01){
        x = ((((-1 * pow(t-tm,3)) + (3 * pow(t-tm,2)) - (3 * (t-tm)) + 1) / 6) * punkty[koniec - 2]) +
                ((((3 * pow(t-tm,3)) - (6* pow(t-tm,2)) + 4) / 6) * punkty[koniec - 4]) +
                ((((-3 * pow(t-tm,3)) +(3 * pow(t-tm,2)) + 3*(t-tm) +1) / 6) * punkty[koniec - 6]) +
                (((pow(t-tm,3))/6) * punkty[koniec - 8]);
        y = ((((-1 * pow(t-tm,3)) + (3 * pow(t-tm,2)) - (3 * (t-tm)) + 1) / 6) * punkty[koniec - 1]) +
                ((((3 * pow(t-tm,3)) - (6* pow(t-tm,2)) + 4) / 6) * punkty[koniec - 3]) +
                ((((-3 * pow(t-tm,3)) +(3 * pow(t-tm,2)) + 3*(t-tm) +1) / 6) * punkty[koniec - 5]) +
                (((pow(t-tm,3))/6) * punkty[koniec - 7]);
        drawPixel( int(x), int(y), 255, 0, 0);
    }
    update();
}

a small dictionary:
punkty - saved points
koniec - end

Full code at the repository: https://gitlab.com/Sempron/b-spline

Sempron
  • 29
  • 1
  • 4
  • 1
    no wonder you made a mistake in this monster expressions. Better slip them up in smaller parts, test them individually, put them back together, profit – 463035818_is_not_an_ai Feb 27 '20 at 21:20
  • I agree with @idclev463035818. Some parts of this expression are clearly repeated and should be given a clear name. Just replacing `t-tm` with something descriptive would help. – Ted Lyngmo Feb 27 '20 at 21:36
  • "slip" ?!? I meant "split" of course. And "monster" wasnt meant as offense, but seriously for me those lines are far too complicated, if I wrote them I would make more than one mistake for sure – 463035818_is_not_an_ai Feb 27 '20 at 21:38
  • m describes a segment - we start with 3 and we increment it with every segment. tm = m - 3, cause we start with 0 – Sempron Feb 27 '20 at 21:38
  • @Sempron You do the same calculations many times. Break each one out and give it a name. Sometimes that's all one needs to find the error. What _is_ `t-tm`? Name it properly and put that in the expression. Then take the next etc. – Ted Lyngmo Feb 27 '20 at 21:45
  • 1
    One suggestion: Instead of providing a dictionary (which is better than nothing) - translate the code _before_ posting it so that no dictionary is needed. – Ted Lyngmo Feb 27 '20 at 21:49

1 Answers1

1

If code looks complicated then quite likely it really is too complicated.

Lets concentrate on this part:

x = ((((-1 * pow(t-tm,3)) + (3 * pow(t-tm,2)) - (3 * (t-tm)) + 1) / 6) * punkty[koniec - 2]) +
            ((((3 * pow(t-tm,3)) - (6* pow(t-tm,2)) + 4) / 6) * punkty[koniec - 4]) +
            ((((-3 * pow(t-tm,3)) +(3 * pow(t-tm,2)) + 3*(t-tm) +1) / 6) * punkty[koniec - 6]) +
            (((pow(t-tm,3))/6) * punkty[koniec - 8]);

Then you can introduce a 

auto temp = t-tm;

Next, in the formula you have four different terms. I do not see them in your code. Make them explicit

auto x_term1 = - pow(temp,3) + (3 * pow(temp,2)) - (3 * temp + 1);    
x_term1 *= punkty[koniec - 2] / 6;
auto x_term2 = ...
...
x = x_term1 + x_term2 + x_term3 + x_term4;

It is likely that while refactoring code to be readable you will already fix the problem. If not use pen and paper (or similar) to get example output for example input for each individual term (you should do this in any case so you can test your code without inspecting the image). Then use a debugger to see which term is off.

PS: Better dont use pow to square numbers. pow is for floating point exponents which is more costly than what you need here. I just didnt want to change your actual calculations.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185