ist mein Code Standardkonform

net 0

ausserdem ist der algo viel zu umständlich. hier eine version ohne multiplikationen und ohne lookup tables:

void Circle (int x, int y, int radius)
{
  int balance, xoff, yoff;

  xoff = 0;
  yoff = radius;
  balance = -radius;

  do 
  {
    SetPixel(x+xoff, y+yoff);
    SetPixel(x-xoff, y+yoff);
    SetPixel(x-xoff, y-yoff);
    SetPixel(x+xoff, y-yoff);
    SetPixel(x+yoff, y+xoff);
    SetPixel(x-yoff, y+xoff);
    SetPixel(x-yoff, y-xoff);
    SetPixel(x+yoff, y-xoff);

    if ((balance += xoff++ + xoff) >= 0)
      balance -= --yoff + yoff;

  } while (xoff <= yoff);
}

Ponto

net schrieb:

balance += xoff++ + xoff
balance -= --yoff + yoff;

Beide Ausdrücke haben undefined behaviour und sind damit nicht mehr standardkonform.

Der Standard sagt:

Between the previous and next sequence point a scalar object shall have its stored value modified at most once by the evaluation of an expression. Furthermore, the prior value shall be accessed only to determine the value to be stored.

Im ersten Fall wird xoff einmal modifiziert, aber es wird zweimal auf xoff zugegriffen. Beim zweiten Mal wird der Wert aber nicht verwendet, um den modifizierten Wert zu bestimmen.

net 0

Ponto schrieb:

net schrieb:
balance += xoff++ + xoff
balance -= --yoff + yoff;
Beide Ausdrücke haben undefined behaviour und sind damit nicht mehr standardkonform.

ja, stimmt. das kommt davon wenn man einfach was von fremden webseiten kopiert

borg

Vertexwahn schrieb:

if(dx >= 0) 
        X = true; 
    else 
        X = false; 

    if(dy >= 0) 
        Y = true; 
    else 
        Y = false;

na dann mach doch lieber

X = (dx >= 0);
Y = (dy >= 0);

das ist doch viel schöner und für den leser auch nicht schwerer zu verstehen. und weil du X und Y vorher noch nicht benutzt hast, mach doch lieber

bool X = (dx >= 0);
bool Y = (dy >= 0);

und mach die deklerationen oben weg.

dann ist da noch

m = array[(X?1:0) + (Y?2:0) + (Z?4:0)];

ich würd ja eher schreiben

m = array[X + Y*2 + Z*4];

Vertexwahn

> außerdem ist der algo viel zu umständlich

Augen auf beim Eierklau
mein Programm rastert Geradenstücke – deines einen Kreis

> hier eine version ohne multiplikationen

Mein Code arbeitet doch auch ohne Multiplikation – Bitshiften ist doch schneller als mit 2 multiplizieren

> Wenn du die Variablen gesammelt definierst und nicht erst unmittelbar vor Gebrauch,
dann solltest du sie zumindest initialisieren.

Ich weiß
@Chew-Z:
Du hast natürlich absolut recht – mein Code ist furchtbar – das ist mir durchaus bewusst – aber er ist auch mit 20 Seiten Dokumentiert – wenn man den Bresenham Algo verstanden hat ist der Programmcode leicht nachvollziehbar – wenn ich den Variablen schöne Namen verpasse, dann wird man den Algorithmus auch nicht nachvollziehen können. Dazu muss man sich einen Haufen Skizzen machen um das verstehen zu können (darum 20 Seiten ;))

@borg:

X?(1:0) + (Y?2:0) + (Z?4:0) != X + Y*2 + Z*4;

Da kommen verschiednen Werte raus - die Klammern sind erforderlich - die kann man nicht einfach weglassen - (oder die zwei verwendeten Compiler mit dennen ich es getestet habe weichen vom C++ Standard ab)

Vertexwahn

Nach den vorgeschlagenen Änderungen sieht der Code jetzt wie folgt aus:

void Bresenham(int d1x, int d1y, int d2x, int d2y)
	{	
		bool Z;			// zur Bestimmung der Bewegunsrichtung
		int da, db;		// delta_a und delta_b
		int *m;			// Bewegungsrichtung/Vektoren

		// delta a, delta b und Bewegungsrichtung bestimmen 
		int dx = abs(d2x) - abs(d1x);
		int dy = abs(d2y) - abs(d1y);

		if(abs(dx)-abs(dy)>=0)
		{
			da = abs(dx);
			db = abs(dy);
			Z = true;
		}
		else
		{
			da = abs(dy);
			db = abs(dx);
			Z = false;
		}

		bool X = (dx >= 0); 
		bool Y = (dy >= 0);

		int array[8][4] =
		{
			{ 0,-1,-1,-1 },		
			{ 0,-1,1,-1 },		
			{ 0,1,-1,1 },		
			{ 0,1,1,1 },		
			{ -1,0,-1,-1 },		
			{ 1,0,1,-1 },		
			{ -1,0,-1,1 },		
			{ 1,0,1,1 },		
		};

		m = array[(X?1:0) + (Y?2:0) + (Z?4:0)];

		int gradient = (db<<1) - da;
		int x = d1x;
		int y = d1y;

		SetPixel(x, y, 255, 0, 0);

		for(int i = 0; i < da; i++)
		{
			if(gradient >= 0)
			{
				x = x + m[2];
				y = y + m[3];
				gradient = gradient + (db<<1) - (da<<1);
			}
			else
			{
				x = x + m[0];
				y = y + m[1];
				gradient = gradient + (db<<1);
			}

			SetPixel(x, y, 255, 0, 0);
		}
	}

Michael E.

AFAIK ist nicht festgelegt, dass bool true == int 1 ist.

Vertexwahn

> AFAIK ist nicht festgelegt, dass bool true == int 1 ist.

im C++ Kompendium steht:
Der Integer Wert 0 wird als false interpretiert

Ein Integer Wert ungleich 0 wird als true interpretiert

Das Buch bezieht sich auf ISO/IEC 14882 von 1998(?)
hat sich da was geändert?

Michael E.

Hast du mich falsch verstanden? Ich wollte sagen, dass bar hier nicht 1 sein muss:

bool foo = true;
int bar = foo;

Vertexwahn

wo spielt das in meinem Code eine Rolle?

bool X = (dx >= 0); 
bool Y = (dy >= 0);

wird aus (dx >= 0) nicht automatisch ein boolscher Wert?

PS: ja ich hab dich falsch verstanden - dachte da besteht eine Äquivalenzrelation

Michael E.

Das war ursprünglich für borg gedacht.

Vertexwahn

aso - wollte nur sichergehen das mein code standardkonform ist - nicht das ich etwas veröffenltiche was dann nicht mal von einem Standardkonformen C++ Compiler übersetzt werden kann - wäre schon peinlich

Michael E.

Ich denk mal, wenn Comeau es ohne Warnung kompiliert, wird es schon standardkonform genug sein. Google doch mal danach.

DrGreenthumb

Vertexwahn schrieb:

if(abs(dx)-abs(dy)>=0)
		{
			da = abs(dx);
			db = abs(dy);
			Z = true;
		}
		else
		{
			da = abs(dy);
			db = abs(dx);
			Z = false;
		}

das würde ich so schreiben:

int da = abs(dx), db = abs(dy);
    if(z = da - db >= 0)
        swap(da, db);

Und das geschifte ist auch unnötig (wenn nicht sogar unstandardkonform), das optimiert der Compiler schon.

Vertexwahn

@DrGreenthumb:
Du hast einen kleinen Logikfehler gemacht
Das Bitshiften hab ich drin gelassen - denk dir einfach eine Multiplikation mit 2 - ich will mich jetzt nicht streiten - oder ist es wirklich so schlimm und ich sollte das lieber nicht machen?

Der Code sieht jetzt so aus:

void Bresenham(int d1x, int d1y, int d2x, int d2y) 
    {    
        bool Z = true;     // zur Bestimmung der Bewegunsrichtung 
        int *m;            // Bewegungsrichtung/Vektoren 

        // delta a, delta b und Bewegungsrichtung bestimmen 
        int dx = abs(d2x) - abs(d1x); 
        int dy = abs(d2y) - abs(d1y); 

		int da = abs(dx), db = abs(dy); 

		if(da-db<0)
		{
			int tmp = da;
			da = db;
			db = tmp;
			Z = false;
		}

		bool X = (dx >= 0); 
        bool Y = (dy >= 0); 

        int array[8][4] = 
        { 
            { 0,-1,-1,-1 },        
            { 0,-1,1,-1 },        
            { 0,1,-1,1 },        
            { 0,1,1,1 },        
            { -1,0,-1,-1 },        
            { 1,0,1,-1 },        
            { -1,0,-1,1 },        
            { 1,0,1,1 },        
        }; 

        m = array[(X?1:0) + (Y?2:0) + (Z?4:0)]; 

        int gradient = (db<<1) - da; 
        int x = d1x; 
        int y = d1y; 

        SetPixel(x, y, 255, 0, 0); 

        for(int i = 0; i < da; i++) 
        { 
            if(gradient >= 0) 
            { 
                x = x + m[2]; 
                y = y + m[3]; 
                gradient = gradient + (db<<1) - (da<<1); 
            } 
            else 
            { 
                x = x + m[0]; 
                y = y + m[1]; 
                gradient = gradient + (db<<1); 
            } 

            SetPixel(x, y, 255, 0, 0); 
        } 
    }

Der Code wird ja immer kürzer bald passt er auf eine Seite

Michael E.

Bitshifts sind eigentlich unnötig geworden. Fast jeder Compiler optimiert das.

camper

Michael E. schrieb:

Hast du mich falsch verstanden? Ich wollte sagen, dass bar hier nicht 1 sein muss:
bool foo = true;
int bar = foo;

doch muss - stichwort integral promotions (4.5)

An rvalue of type bool can be converted to an rvalue of type int, with false becoming zero and true becoming one.

DrGreenthumb

Vertexwahn schrieb:

@DrGreenthumb:
Du hast einen kleinen Logikfehler gemacht
Das Bitshiften hab ich drin gelassen - denk dir einfach eine Multiplikation mit 2 - ich will mich jetzt nicht streiten - oder ist es wirklich so schlimm und ich sollte das lieber nicht machen?

hm, ich sehe keinen logikfehler.

Das bitshiften ist IMHO schon schlimm. Der Compiler generiert eh denselben Code.

Was sagt eigentlich der C++ Standard über die interne Repräsentation der Zahlen, ergibt so ein shift zwingend auch die multiplikation?

Bashar

Michael E. schrieb:

AFAIK ist nicht festgelegt, dass bool true == int 1 ist.

Doch.

DrGreenthumb schrieb:

Vertexwahn schrieb:
if(abs(dx)-abs(dy)>=0)
		{
			da = abs(dx);
			db = abs(dy);
			Z = true;
		}
		else
		{
			da = abs(dy);
			db = abs(dx);
			Z = false;
		}
das würde ich so schreiben:
int da = abs(dx), db = abs(dy);
    if(z = da - db >= 0)
        swap(da, db);
Und das geschifte ist auch unnötig (wenn nicht sogar unstandardkonform), das optimiert der Compiler schon.

int da = abs(dx), db = abs(dy);
    if(Z = da - db < 0)
        swap(da, db);

so ists richtig!