if-Bedingung wird nie wahr

[Swordfish]

OMG war ich blind. Naja, es war spät.

[Yadgar]

@th69
Mein Compiler (g++ 6.3.0 für Debian Linux) zeigte beim Kompilieren keine Warnungen an!

[Swordfish]

@yadgar sagte in if-Bedingung wird nie wahr:

Mein Compiler (g++ 6.3.0 für Debian Linux) zeigte beim Kompilieren keine Warnungen an!

Warnungen einschalten!? https://godbolt.org/g/YHS43u

[Yadgar]

@swordfish

<anKopfklatsch>
Natürlich! Klassischer Anfängerfehler! Vergleichsoperator == mit Zuweisungsoperator = verwechselt!
</anKopfklatsch>

Das Problem war also gar nicht die if-Verzweigung, sondern die darauf folgende vermeintliche Zuweisung... das kommt davon, wenn man zu selten in C++ programmiert! Ich sollte eigentlich wenigstens vier Tage in der Woche an yip arbeiten, dann käme ich auch schneller voran!

Danke, Swordfish! Ich werde dich umgehend in die Credit-Liste am Anfang des Codes eintragen!

Und jetzt als nächstes erstmal diverse "Max Canada Lynx"-Videos von Bernadette "WildlifeBernie" Hoffman (natürlich mit ihrer Erlaubnis, die sie mir schon vor einigen Monaten gegeben hat!) vervierundsechzigern...

Und dann die nächste Funktion in yip implementieren: Bildarithmetik! Oder doch lieber erst die beiden Atari ST-Farbmodi?

Bis bald im Khyberspace!

Yadgar

[Swordfish]

@yadgar sagte in if-Bedingung wird nie wahr:

Danke, Swordfish! Ich werde dich umgehend in die Credit-Liste am Anfang des Codes eintragen!

Ähm. @Th69 hat es gesehen.
Wie gesagt: In Zukunft bitte mit -Wall kompilieren.

[Yadgar]

Das war leider (wie eigentlich fast immer in der Programmiererei!) zu früh gefreut: solange ich nur einen einzigen (von insgesamt 1000) 4 x 8-Pixel-Bereich überprüfe, funktioniert die Ersetzung der überschüssigen Farben einwandfrei - sobald ich aber das gesamte Bild durchgehe, passiert bei Bereichen mit mehr als 4 Farben rein gar nichts!

Ich war davon ausgegangen, dass es wohl an der fehlenden Neuinitialisierung der Vektoren fourcolours und excesscolors für die 4 häufigsten bzw. überschüssigen Farben gelegen haben könnte - leider änderte eine entsprechende Code-Änderung auch nichts am Ergebnis!

Hier der aktuelle Code:

 void c64multicolor_correct(vector<vector<pixel> >& c64img, vector<vector<pixel> >& c64_area)
{
  short a, b, i, j, k, l, m, n, colnum_area, freq;
  pixel p;
  rgb c;
  vector<rgb> areacols;
  vector<short> colfreqs;
  vector<rgb> fourcolors;
  vector<rgb> excesscolors;    
  bool match;
  
  
  
  
  for (a=0; a<200; a+=8) // zu Testzwecken wird nur ein einzelner 4 x 8-Bereich eingelesen! Bitte anschließend korrigieren!
  {  
    for (b=0; b<160; b+=4)
    {
      c64_area.resize(8);
      for (i=a; i<a+8; i++) // pixels from image are copied into temporary 4 by 8 pixel vector
      {
        for (j=b; j<b+4; j++)
        {
          p.set_red((unsigned char)c64img[i].at(j).get_red());
          p.set_green((unsigned char)c64img[i].at(j).get_green());
          p.set_blue((unsigned char)c64img[i].at(j).get_blue());
          c64_area[i-a].push_back(p);
        }
      }
      areacols.resize(0);
      for (k=0; k<8; k++) // vector containing different colors in temporay 4 by 8 pixel vector is created
      {
        for (l=0; l<4; l++)
        {
          c.red = c64_area[k].at(l).get_red();
          c.green = c64_area[k].at(l).get_green();
          c.blue = c64_area[k].at(l).get_blue();
          match=false;
          if (k==0 && l==0)
	  {  
            areacols.push_back(c);
	  }
          else
          {
            for (m=0; m<areacols.size(); m++)
            {
              if (c.red == areacols[m].red && c.green == areacols[m].green && c.blue == areacols[m].blue)
              {
                match=true;
              }  
            } 
            if (match == false) // new color found
            {
              areacols.push_back(c);
            }
          }
	  
        } 
      }
      colnum_area = areacols.size();
      if (colnum_area > 4)
      {
        fourcolors.resize(4);
        excesscolors.resize(colnum_area-4);
        colfreqs.resize(colnum_area);
        for (m=0; m<colfreqs.size(); m++)
        colfreqs[m] = 0;
        m=0;
        for (k=0; k<8; k++) 
        {
        for (l=0; l<4; l++)
        {
            if ( k == 0 && l == 0)
            {
            colfreqs[m]++;
            }
            else
            {
            c.red = c64_area[k].at(l).get_red();
            c.green = c64_area[k].at(l).get_green();
            c.blue = c64_area[k].at(l).get_blue();
                while (!(c.red == areacols[m].red && c.green == areacols[m].green && c.blue == areacols[m].blue))
            m++;
            colfreqs[m]++;
            m = 0;
            }
        }
        }
	/* for (m = 0; m < colnum_area; m++)
	  cout << "Farbe #" << m << ": rgb <" << areacols[m].red << "," << areacols[m].green << "," << areacols[m].blue << "> - " << colfreqs[m] << " Pixel" << endl; */
	qsrt(colfreqs, areacols, 0, colfreqs.size()-1);  
	/* for (m = 0; m < colnum_area; m++)
	  cout << colfreqs[m] << " Pixel: rgb <" << areacols[m].red << "," << areacols[m].green << "," << areacols[m].blue << ">" << endl; */
        
	
	for (m = 0; m < 4; m++)
	  fourcolors.push_back(areacols[m]);
	for (m = 4; m < colnum_area; m++)
	  excesscolors.push_back(areacols[m]);
	
	
	
	/* rgb closest = fourcolors[0];
	for (m = 4; m < colnum_area; m++)
	{
	  for (n = 0; n < 4; n++)
	  {  
	    if (coldist(areacols[m], fourcolors[n]) < coldist(areacols[m], closest))
	    {  
	      closest = fourcolors[n];
	      cout << "rgb <" << areacols[m].red << "," << areacols[m].green << "," << areacols[m].blue << "> wird ersetzt durch <"<< fourcolors[n].red << "," << fourcolors[n].green << "," << fourcolors[n].blue << ">" << endl;
	    }  
	  }
	}
	

	cout << Uuml << "bersch" << uuml << "ssige Farben:" << endl;
	for (m = 0; m < colnum_area-4; m++)
	  cout << "rgb <" << excesscolors[m].red << "," << excesscolors[m].green << "," << excesscolors[m].blue << ">" << endl; */
	
	
	for (k=0; k<8; k++) 
	{
	  for (l=0; l<4; l++)
	  {
        match = false;
	    m = 0;
	    c.red = c64_area[k].at(l).get_red();
	    c.green = c64_area[k].at(l).get_green();
	    c.blue = c64_area[k].at(l).get_blue();
	    while (match == false /*&& m < colnum_area-4*/)
	    {
	      // cout << c.red << " " << c.green << " " << c.blue << " | " << excesscolors[m].red << " " << excesscolors[m].green << " " << excesscolors[m].blue << endl;
          if ((c.red == excesscolors[m].red) && (c.green == excesscolors[m].green) && (c.blue == excesscolors[m].blue))
            match = true;
	      m++;
	    }
	    // cout << match << endl;
	    if (match == true)
	    {  
	      rgb closest = fourcolors[0];
	      for (n=0; n<4; n++)
	      {
		  if (coldist(c, fourcolors[n]) < coldist(c, closest))
	          closest = fourcolors[n];
	      }
	      /* cout << a+k << endl;
	      cout << b+l << endl; */
	      c64img[a+k].at(b+l).set_all(closest.red, closest.green, closest.blue);
	    } 
	  }     
	}      
    }
    }
  }
}

Wenn ich vor dem Teil, der nur bei mehr als 4 Farben ausgeführt wird, die Anzahl der Farben ausgeben lasse, wird immer "3" angezeigt - die Farbanzahl des allerersten 4 x 8-Pixel-Bereiches...

[Swordfish]

void c64multicolor_correct(std::vector<std::vector<pixel>>& c64img)
{
 	for (std::size_t a = 0; a < 200; a += 8)
	{
		for (std::size_t b = 0; b < 160; b += 4)
		{
			std::vector<std::vector<pixel>> c64_area(8);
			for (std::size_t i = a; i < a + 8; i++)
				for (std::size_t j = b; j < b + 4; j++)
					c64_area[i - a].push_back(c64img[i][j]);

			std::vector<pixel> areacols;
			areacols.push_back(c64_area[0][0]);

			for (std::size_t k = 0; k < 8; k++)
				for (std::size_t l = 0; l < 4; l++)
					if (k && l && std::find(std::begin(areacols), std::end(areacols), c64_area[k][l]) == std::end(areacols))
						areacols.push_back(c64_area[k][l]);

			if (areacols.size() <= 4)
				continue;

			std::vector<short> colfreqs(areacols.size());
				
			for (std::size_t k = 0; k < 8; k++)
			{
				for (std::size_t l = 0; l < 4; l++)
				{
					if (k == 0 && l == 0) {
						colfreqs[0]++;
						continue;
					}
							
					colfreqs[std::find(std::begin(areacols), std::end(areacols), c64_area[k][l]) - std::begin(areacols)]++;
				}
			}

			qsrt(colfreqs, areacols, 0, colfreqs.size() - 1);

			std::vector<pixel> fourcolors{ std::begin(areacols), std::begin(areacols) + 4 };
			std::vector<pixel> excesscolors{ std::begin(areacols) + 4, std::end(areacols) };

			for (std::size_t k = 0; k < 8; k++)
			{
				for (std::size_t l = 0; l < 4; l++)
				{
					if (std::find(std::begin(excesscolors), std::end(excesscolors), c64_area[k][l]) != std::end(excesscolors) ) {
						pixel closest = fourcolors[0];
						for (std::size_t n = 0; n<4; n++)
							if (coldist(c64_area[k][l], fourcolors[n]) < coldist(c64_area[k][l], closest))
								closest = fourcolors[n];

						c64img[a + k][b + 1] = closest;
					}
				}
			}
		}
	}
}

Das sollte äquivalent zu deinem Spaghetti von oben sein. class pixel braucht dafür noch einen Zuweisungsoperator und Vergleichsoperatoren. Debuggen darfst selbst.

[Yadgar]

@swordfish sagte in if-Bedingung wird nie wahr:

Das sollte äquivalent zu deinem Spaghetti von oben sein. class pixel braucht dafür noch einen Zuweisungsoperator und Vergleichsoperatoren. Debuggen darfst selbst.

Damit fange ich dann auch gleich an... in welcher Bibliothek finde ich z. B. die Funktion find()? Bei mir führte die gerade eben zu diesem Fehler hier:

yip.cc: In function ‘void c64multicolor_correct(std::vector<std::vector<pixel> >&)’:
yip.cc:3288:86: error: no matching function for call to ‘find(std::vector<pixel>::iterator, std::vector<pixel>::iterator, __gnu_cxx::__alloc_traits<std::allocator<pixel> >::value_type&)’
     if (k && l && std::find(std::begin(areacols), std::end(areacols), c64_area[k][l]) == std::end(areacols))

Bis bald im Khyberspace!

Yadgar

[Schlangenmensch]

Steht da doch, in der std. Mit std::find via Google einfach zu finden.
Was fehlt sind die inlcudes. In dem Fall #include <algorithm>

[Swordfish]

@schlangenmensch sagte in if-Bedingung wird nie wahr:

via Google einfach zu finden.

Yadgar googelt nicht gerne da

@yadgar sagte in Spezifikation von 16bit-Graustufen-TIFFs?:

Ich habe leider die Erfahrung gemacht, dass Google nicht mein Freund ist, sondern mir viel zu oft den Bildschirm nur mit nutzlosem Linkmüll vollknattert...

[Yadgar]

@swordfish

Also, #include <algorithm> habe ich eingefügt - jetzt kennt das Programm meine Quicksort-Implementation qsrt() nicht mehr!

Fehlermeldung:

yip.cc: In function ‘void c64multicolor_correct(std::vector<std::vector<pixel> >&)’:
yip.cc:3322:51: error: no matching function for call to ‘qsrt(std::vector<short int>&, std::vector<pixel>&, int, std::vector<short int>::size_type)’
    qsrt(colfreqs, areacols, 0, colfreqs.size() - 1);

...und das ist nur die erste einer langen Latte von Fehlermeldungen, eine kryptischer als die andere!

qsrt ist in zwei Varianten definiert:

void qsrt(vector<float>&, vector<pixel>&, unsigned int, unsigned int); 
void qsrt(vector<short>&, vector<rgb>&, unsigned int, unsigned int); // descending!

Und warum size_type statt Standardtypen für die Zählervariablen? Warum muss immer alles so kompliziert sein?

Geht es nicht einfacher? Ich bin C++-Anfänger! Was ist denn der konkrete Grund dafür, dass colnum_area immer 3 bleibt? Mit einer turbo-optimierten Profi-Lösung (die noch dazu aus didaktischen Gründen fehlerhaft ist) kann ich nichts anfangen, die verstehe ich nämlich nicht!

[Swordfish]

@yadgar sagte in if-Bedingung wird nie wahr:

qsrt ist in zwei Varianten definiert:

void qsrt(vector<float>&, vector<pixel>&, unsigned int, unsigned int); 
void qsrt(vector<short>&, vector<rgb>&, unsigned int, unsigned int); // descending!

Mach

void qsrt(vector<short>&, vector<pixel>&, int, std::size_t);

draus.

Und warum size_type statt Standardtypen für die Zählervariablen?

Weil Speichergrößen nunmal std::size_t groß sein können.

Was ist denn der konkrete Grund dafür, dass colnum_area immer 3 bleibt?

Keine Ahnung. Wie gesagt: Debuggen darfst selbst.

[Yadgar]

@swordfish

plonk

[Swordfish]

. o O ( wer nicht will der hat schon )

[Yadgar]

@swordfish sagte in if-Bedingung wird nie wahr:

. o O ( wer nicht will der hat schon )

Das war etwas voreilig... ich reagierte so angefressen, weil ich davon ausging, dass du der Meinung wärest, dass der Fehler etwas mit meiner veralteten bzw. umständlichen Datenstruktur zu tun hätte! Jetzt, wo ich sehe, dass das offensichtlich zwei ganz verschiedene "Baustellen" sind, würde ich es doch noch einmal versuchen, von Hand den Code durchzugehen...

[Swordfish]

@yadgar sagte in if-Bedingung wird nie wahr:

weil ich davon ausging, dass du der Meinung wärest, dass der Fehler etwas mit meiner veralteten bzw. umständlichen Datenstruktur zu tun hätte!

Ich wollte Dir nur zeigen, wie Du diese Wurst um Häuser vereinfachen und kürzen kannst. Je weniger Code, desto weniger Platz für Fehler. Je prägnanter der Code, desto leichter zu debuggen.

@yadgar sagte in if-Bedingung wird nie wahr:

Jetzt, wo ich sehe, dass das offensichtlich zwei ganz verschiedene "Baustellen" sind, würde ich es doch noch einmal versuchen, von Hand den Code durchzugehen...

Sicher besser, als auf die Lösung^tm zu warten -scnr-

[Yadgar]

@swordfish sagte in if-Bedingung wird nie wahr:

@yadgar sagte in if-Bedingung wird nie wahr:

weil ich davon ausging, dass du der Meinung wärest, dass der Fehler etwas mit meiner veralteten bzw. umständlichen Datenstruktur zu tun hätte!

Ich wollte Dir nur zeigen, wie Du diese Wurst um Häuser vereinfachen und kürzen kannst. Je weniger Code, desto weniger Platz für Fehler. Je prägnanter der Code, desto leichter zu debuggen.

@yadgar sagte in if-Bedingung wird nie wahr:

Jetzt, wo ich sehe, dass das offensichtlich zwei ganz verschiedene "Baustellen" sind, würde ich es doch noch einmal versuchen, von Hand den Code durchzugehen...

Sicher besser, als auf die Lösung^tm zu warten -scnr-

Also, ich bin das Befüllen des Vektors für den jeweils aktuellen 4 x 8-Pixelbereich nochmal durchgegangen, habe Kontrollausgaben sowohl für das Übertragen der einzelnen Pixel aus dem Gesamtbild-Array (in der Funktion: c64img) als auch für das Überprüfen der Pixel aus dem aktuellen 4 x 8-Bereich (Vektor c64_area) eingefügt:

for (a=0; a<200; a+=8) 
  {  
    for (b=0; b<160; b+=4)
    {
      cout << "---------------------------------------------------------------------------------------------" << endl;  
      cout << "4 x 8 - Bereich " << j << "/" << i << " (beginnend links oben bei Pixel " << b << "/" << a << ")" << endl; 
      cout << "---------------------------------------------------------------------------------------------" << endl;
      c64_area.resize(0);
      for (i=a; i<a+8; i++) // pixels from image are copied into temporary 4 by 8 pixel vector
      {
        for (j=b; j<b+4; j++)
        {
          
	  p.set_red(c64img[i].at(j).get_red());
          p.set_green(c64img[i].at(j).get_green());
          p.set_blue(c64img[i].at(j).get_blue());
	  // cout << "Pixel " << j << "/" << i << " eingelesen!" << endl;
	  c64_area[i-a].push_back(p);
        }
      }
      areacols.resize(0);
      for (k=0; k<8; k++) // vector containing different colors in temporary 4 by 8 pixel vector is created
      {
        for (l=0; l<4; l++)
        {
          c.red = c64_area[k].at(l).get_red();
          c.green = c64_area[k].at(l).get_green();
          c.blue = c64_area[k].at(l).get_blue();
          cout << "Farbe von Pixel " << j << "/" << i << ": " << "rgb <" << c.red << "," << c.green << "," << c.blue << ">" << endl;

	  match=false;
          if (k==0 && l==0)
          {  
            areacols.push_back(c);
          }
          else
          {
            for (m=0; m<areacols.size(); m++)
            {
              if (c.red == areacols[m].red && c.green == areacols[m].green && c.blue == areacols[m].blue)
              {
                match=true;
              }  
            } 
            if (match == false) // new color found
            {
              areacols.push_back(c);
            }
            // cout << "Anzahl der Farben: " << areacols.size() << endl;
          }
	  
        } 
      }
      cout << endl;

Ursprünglich wurde schon beim Auslesen der Pixeldaten aus dem Gesamtbild-Vektor immer nur der erste 4 x 8-Bereich angezeigt; das konnte ich beheben, indem ich die explizite Typumwandlung nach unsigned char (ein Relikt aus früheren Programmversionen) in den drei Zuweisungen entfernte.

In der zweiten Doppelschleife (nach areacols.resize()) wird aber nach wie vor über alle 1000 4 x 8-Bereiche hinweg nur der Inhalt des ersten Bereichs angezeigt! Habe ich irgendetwas im Zusammenhang mit den Vektor-Methoden resize (in diesem Fall c64_area.resize(0)) und push_back nicht verstanden? Wie initialisiere ich einen zweidimensionalen Vektor neu?

(etwas später)

Natürlich, ich muss alle "Teilvektoren" mit resize(0) löschen! Also

for (i=0; i<8; i++)
	c64_area[i].resize(0);

statt

c64_area.resize(0);

Dann werden auch alle Pixel des Bildes aus dem aktuellen 4 x 8-Bereich heraus angezeigt - und die Farbzählroutine zeigt ebenfalls korrekte Werte an! Trotzdem macht die Funktion als Ganzes noch immer nicht das, was sie soll - es werden keine überzähligen Farben im Bild ersetzt! Aber darum kümmere ich mich morgen...

Bis bald im Khyberspace!

Yadgar

[Swordfish]

Und ich hab mich schon der Hoffnung hingegeben, Du hättest die grundlegenden Unterschiede zwischen Deinem und meinem Code erkannt endlich damit angefangen, Variablen so lokal wie möglich zu deklarieren. Damit werden die ganzen resizes überflüssig. Wenn Du dann noch endlich struct rgb wegwirfst und durchgängig pixel nimmst fällt eine menge sinnlose komponentenweise kopiererei weg. Dann noch die einzelnen Vektoren über Iteratorpaare initialisieren statt umständlich über Schleifen zu kopieren. ...

[Yadgar]

@swordfish

Anderswo (de.comp.lang.iso-c++) ist mir schon voriges Jahr sogar geraten worden, im Interesse der Geschwindigkeit ganz auf höhere Datenstrukturen wie Vektoren zu verzichten und stattdessen nur C-Arrays und Zeigerarithmetik zu verwenden - und das werde ich wohl früher oder später in Angriff nehmen!

[DocShoe]

Hab außer Stilfragen leider nicht viel zu deinem Problem beizutragen, aber:

1. Wenn du die Größen der Vektoren bereits kennst dann benutze reserve() , resize() oder inititalisiere die Vektoren mit der richtigen Größe, um unnötige Allokationen zu vermeiden.
2. Vermeide Elementzugriffe mit at(), benutz´ stattdessen den Indexoperator[]
3. Kopier´ ganze RGB Objekte statt jedes Attribute einzeln
4. benutz´ keine handgeschriebenen Schleifen über Vektorelemente, dazu gibt es in der STL passende Funktionen.

Ansonsten kann ich Swordfishs Anregungen nur unterstützen: Halte deine Variablen so lokal wie möglich.

Nachtrag:
Wenn du häufig mit rechteckigen Blöcken arbeitest kann es sinnvoll sein, sich dazu eine Klasse zu basteln. N Vektoren aus M Elementen lässt sich als Vektor mit M*N Elementen realisieren. Vektoren aus Vektoren haben das Problem, dass der benutzte Speicher nicht zusammenhängend ist und der Zugriff auf Elemente möglicherweise langsamer ist als der Zugriff auf zusammenhängenden Speicher.