ist mein Code Standardkonform

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!

Ponto

DrGreenthumb schrieb:

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

Ich zitiere:

5.8.2

The value of E1 << E2 is E1 (interpreted as a bit pattern) left-shifted E2 bit positions; vacated bits are zero-filled. If E1 has an unsigned type, the value of the result is E1 multiplied by the quantity 2 raised to the power E2, reduced modulo ULONG_MAX+1 if E1 has type unsigned long, UINT_MAX+1 otherwise. [Note: the constants ULONG_MAX and UINT_MAX are defined in the header <climits>).

Man kann es also nur für unsigned Typen annehmen.

DrGreenthumb

Stammtischler schrieb:

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);

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

so ists richtig!

äh, bin ich jetzt blöd Im Originalcode wird Z auf true gesetzt, wenn die Bedingung da - db >= 0 zutrifft.
Also: Z = da - db >= 0

@Ponto
ah, thx

camper

edit: müll

borg

#include <iostream>

int main( )
{
    bool a = true;
    bool b = false;
    std::cout << (a?4:0) << std::endl;
    std::cout << a*4 << std::endl;
    std::cout << (a?2:0) << std::endl;
    std::cout << a*2 << std::endl;
    std::cout << (b?4:0) << std::endl;
    std::cout << b*4 << std::endl;
    std::cout << (b?2:0) << std::endl;
    std::cout << b*2 << std::endl;

    std::cout << std::endl << (int)a << std::endl;
    std::cout << (int)b << std::endl;
}

ausgabe schrieb:

4
4
2
2
0
0
0
0

1
0

mit gcc und vc++. was mache ich falsch?

jede beliebige zahl ungleich 0 gecastet als bool ist true, true zum int gecastet ist 1. wenn das nicht so ist, sind in ganz vielen programmen von mir fehler

DrGreenthumb

DrGreenthumb schrieb:

Stammtischler schrieb:
DrGreenthumb schrieb:
das würde ich so schreiben:
int da = abs(dx), db = abs(dy);
    if(z = da - db >= 0)
        swap(da, db);
int da = abs(dx), db = abs(dy);
    if(Z = da - db < 0)
        swap(da, db);
so ists richtig!
äh, bin ich jetzt blöd Im Originalcode wird Z auf true gesetzt, wenn die Bedingung da - db >= 0 zutrifft.
Also: Z = da - db >= 0

ah, jetzt seh ich's. swap wird falsch aufgerufen. Wenn ichs jetzt nicht wieder verpeile, müsste es dann aber so aussehen:

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

sonst hat Z den falschen Wert.

Vertexwahn

> Man kann es also nur für unsigned Typen annehmen.

so ein scheiß - in meinem C++ Kompendium steht das eine einfache Linksverschiebung bzw. einfache Rechtverschiebung für einen int (der ja signed ist) gleichbedeutent mit einer Multiplikation bzw. Division durch/mit 2 ist

mmh entweder ich nehm jetzt unsigned int her oder ich multipliziere doch mit 2 und schiebe die arbeit auf den compiler - mmh...

DrGreenthumb

Vertexwahn schrieb:

mmh entweder ich nehm jetzt unsigned int her oder ich multipliziere doch mit 2 und schiebe die arbeit auf den compiler - mmh...

argh, natürlich multiplizierst du einfach mit 2! Das macht den Code leserlicher und kommt aufs selbe raus.

Vertexwahn

> argh

tschuldigung - ok ich sehs ja ein

Michael E.

camper: Bashar: Danke für die Richtigstellung

Vertexwahn

Momentaner Stand

/*
	Beschreibung zum Algorithmus:
	http://turing.fh-landshut.de/~jamann/Bresenham.doc

	Autor: Julian Amann
	Kontakt: vertexwahn@gmx.de
	Erstellt am: 19.02.2005

	Ich setzte ein kartesisches Rechsthaendiges Koordinatensystem und eine SetPixel Funktion voraus.
*/

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 = 2 * db - da; 
    int x = d1x; 
    int y = d1y; 

    SetPixel(x, y); 

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

        SetPixel(x, y); 
    } 
}

camper

wenn du sparen willst, lässt du X,Y,Z weg and benutzt die bedingungen direkt um m zu initialisieren, ausserdem würd ich array static const und m const machen.

static const 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 },        
    };

    const int* const m = array[(dx>=0?1:0) + (dy>=0?2:0) + (da>=db?4:0)];
    if(da<db) swap(da,db);