if-Bedingung wird nie wahr

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:

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.
Vermeide Elementzugriffe mit at(), benutz´ stattdessen den Indexoperator[]
Kopier´ ganze RGB Objekte statt jedes Attribute einzeln
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.

@yadgar sagte in if-Bedingung wird nie wahr:

im Interesse der Geschwindigkeit ganz auf höhere Datenstrukturen wie Vektoren zu verzichten und stattdessen nur C-Arrays und Zeigerarithmetik zu verwenden

Tja, es wird schon viel Mist verbreitet ...

theta

@manni66 sagte in if-Bedingung wird nie wahr:

@yadgar sagte in if-Bedingung wird nie wahr:

im Interesse der Geschwindigkeit ganz auf höhere Datenstrukturen wie Vektoren zu verzichten und stattdessen nur C-Arrays und Zeigerarithmetik zu verwenden

Tja, es wird schon viel Mist verbreitet ...

... und leider auch noch geglaubt.

DocShoe

Wenn man auf den Elementzugriff mit at() macht ist das schon deutlich langsamer. Die Performance Märchen kommen aus der Ecke, wo die Leute die STL falsch benutzen und dann pauschalisieren.

hustbaer

Vor allem gibt es IMO keinen Grund auf std::vector zu verzichten, selbst wenn man bei operator[] oder den Iteratoren Bedenken hat. Gibt ja immer noch die Möglichkeit die performancekritischen Zugriffe über einen Zeiger zu machen den man sich 1x vor der Schleife mit vector::data holt.

Bzw. idealerweise macht man nen Vergleich (mit aktiven Optimierungen!), und sieht dann dass es eben doch keinen Unterschied macht.

Yadgar

@docshoe sagte in if-Bedingung wird nie wahr:

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

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.

Vermeide Elementzugriffe mit at(), benutz´ stattdessen den Indexoperator[]

Kopier´ ganze RGB Objekte statt jedes Attribute einzeln

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.

Das klingt alles vernünftig - aber wenn ich jetzt mitten im manuellen Debugging auch noch die grundlegende Architektur meines Programms umbaue, kommt nur noch mehr Durcheinander bei heraus - erstmal will ich überhaupt eine funktionierende Version haben! Um Optimierung kann ich mich dann hinterher kümmern... das Programm ist ja kein Selbstzweck, sondern soll durchaus praktische (wenn auch nicht bezahlte) Arbeit verrichten!

Swordfish

@yadgar sagte in if-Bedingung wird nie wahr:

Um Optimierung kann ich mich dann hinterher kümmern...

Bei den Punkten, die Dir @docshoe, @hustbaer und ich ans Herz gelegt haben handelt es sich nicht um "Optimierungen" sondern um "schreib sauberen, wartbaren Code".

Yadgar

Hi(gh)!

Beim weiteren Debugging habe ich einen neuen, völlig absurden Fehler festgestellt: zwar werden jetzt 4 x 8-Bereiche mit mehr als 4 Farben richtig erkannt, bei der Bestimmung der Häufigkeiten kommt es aber immer im Zusammenhang mit schwarzen (rgb <0, 0, 0>) Pixeln zu Fehlern. Schwarz ist der erste Eintrag im globalen rgb-Vektor palette; die Umrechnung des ursprünglichen True Color-Bildes auf die 16farbige C 64-Palette fand bereits vorher statt, in der gegenwärtigen Version von yip wird dieses umgerechnete Bild ohne Prüfung auf maximal 4 Farben pro 4 x 8-Bereich auf der Festplatte gespeichert.

Ein Beispiel: im 4 x 8-Bereich 8/128 bis 12/136 sollten laut gespeichertem "C 64-Rohbild" gefunden werden:
1 x türkisgrün - rgb <48,230,198>
4 x "weiß" - rgb <253,254,252>
12 x hellgrau - rgb <164,167,162>
10 x mittelgrau - rgb <112,116,111>
4 x schwarz - rgb <0, 0, 0>
1 x dunkelblau - rgb <33,27,174>

Tatsächlich findet c64multicolor_correct() (Abweichung gefettet):
1 x türkisgrün - rgb <48,230,198>
4 x "weiß" - rgb <253,254,252>
12 x hellgrau - rgb <164,167,162>
10 x mittelgrau - rgb <112,116,111>
4 x dunkelgrau - rgb <66,69,64>
1 x dunkelblau - rgb <33,27,174>

Ein anderes Beispiel, 4 x 8-Bereich 16/128 bis 20/136:
Sollwerte laut gespeichertem Bild:
14 x mittelgrau - rgb <112,116,111>
8 x schwarz - rgb <0, 0, 0>
7 x dunkelgrau - rgb <66,69,64>
2 x hellrot - rgb <254, 74, 87>
1 x orange - rgb <184, 65, 4>

Tatsächlich gefunden:
14 x mittelgrau - rgb <112,116,111>
4 x schwarz - rgb <0,0,0>
7 x dunkelgrau - rgb <66,69,64>
4 x hellgrau - rgb <164,167,162>
2 x hellrot - rgb <254,74,87>
1 x orange - rgb <184,65,4>

hier stimmt nicht einmal die Anzahl der Farben überein!

Ich bin völlig ratlos und zunehmend verwirrt...

Es sollte doch auch mit meiner ineffizienten Programmierweise möglich sein, diesen Fehler zu vermeiden... leider habe ich nicht die geringste Ahnung, was in meinem Code logisch falsch sein könnte! Wieso stehen in ein und demselben 4 x 8-Bereich im Vektor areacols[] andere Farben als in c64_area[][]?

Oder könnte es sein, dass das ganze Vorhaben für mich mindestens eine Nummer zu groß ist und ich yip besser vergessen sollte?

Ich habe mir testhalber noch mal die Farbwerte für den jeweiligen c64_area-Vektor anzeigen lassen - und bekomme jetzt nur noch einen konfusen Wust von short-Überläufen!

Bitte helft mir, ich blicke nicht mehr durch!!!

Hier ist der Link zum Gesamtprogramm yip (tar.gz-Archiv mit allen Source-Dateien, der Logdatei und einem Beispiel-Bild): http://www.rock-o-data.de/yip_20180707.tar.gz (habe die Veränderungen, die zu dem Überlauf-Chaos führten, wieder rückgebaut)

Yadgar

Dieser Beitrag wurde gelöscht!

DocShoe

Dann ist es vielleicht doch an der Zeit den Code umzustrukturieren. Lokale Variablen machen das Debugging oft einfacher, da man ganz sicher nicht mit Altlasten arbeitet.

Yadgar

Hi(gh)!

@docshoe sagte in if-Bedingung wird nie wahr:

Dann ist es vielleicht doch an der Zeit den Code umzustrukturieren. Lokale Variablen machen das Debugging oft einfacher, da man ganz sicher nicht mit Altlasten arbeitet.

Nun gut, ich sehe es ein, dass der einzige realistische Weg zur Lösung meines Problems in der Umsetzung des Code-Vorschlags von Swordfish vom letzten Dienstag liegt - und werde wohl noch heute damit anfangen. Allerdings vorab noch: diese Konstruktionen mit std:: finde ich verwirrend - wird das Kompilat kleiner, wenn man keine Bibliotheken inkludiert? Oder was ist sonst der Vorteil?

Bis bald im Khyberspace!

Yadgar

Th69

Und du solltest deinen Code in kleinere (inline) Funktionen unterteilen, dann ist der Code auch besser test- und wartbar. Wenn du schon in der großen Funktion den Fehler nicht per Debugger rausfindest, vllt. helfen dir dann Unit-Tests für die Teilfunktionen?!

Yadgar

@th69 sagte in if-Bedingung wird nie wahr:

Und du solltest deinen Code in kleinere (inline) Funktionen unterteilen, dann ist der Code auch besser test- und wartbar. Wenn du schon in der großen Funktion den Fehler nicht per Debugger rausfindest, vllt. helfen dir dann Unit-Tests für die Teilfunktionen?!

Debugger? Ich weiß gar nicht, wie man so ein Ding bedient, ich habe bis jetzt immer von Hand debuggt... aber was ist denn jetzt mit std::? Wozu dieses Vorgehen?

Martin Richter

Was ist "von Hand debuggen" ?

Ich benutze immer meine Hände, entweder and der Tastatur oder der Maus.... Sprachsteuerung ist da nicht so angesagt...

Swordfish

@martin-richter sagte in if-Bedingung wird nie wahr:

Was ist "von Hand debuggen" ?

Ich schätze er meint Konsolenausgaben um Variablenwerte zu kennen.

@th69 sagte in if-Bedingung wird nie wahr:

dann ist der Code auch besser test- und wartbar.

Das ganze Ding ist unlesbar und unwartbar. Alleine die main() ist gefühlte 1.000 Zeilen lang und durchsetzt mit #ifdefs für lokalisierte Ausgaben und Variablen, die alle am Anfang deklariert werden um dann irgendwo in Zeile 784 benutzt zu werden ...

@yadgar sagte in if-Bedingung wird nie wahr:

Debugger? Ich weiß gar nicht, wie man so ein Ding bedient

lernen?

@yadgar sagte in if-Bedingung wird nie wahr:

aber was ist denn jetzt mit std::? Wozu dieses Vorgehen?

std ist der Namensraum in dem so ziemlich alles der Standardbibliothek liegt. Wenn Du die Angabe eines Namensraums vor einem Symbol (std::whatever) nicht kennst, dann wurde Dir wahrscheinlich beigebracht in allen Deinen Programmen using namespace std; zu verwenden ohne zu verstehen, warum.