if-Bedingung wird nie wahr



  • @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



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



  • @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...

    💡



  • @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!



  • @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.



  • @swordfish

    plonk



  • . o O ( wer nicht will der hat schon 👍 )



  • @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...



  • @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ösungtm zu warten 😛 -scnr-



  • @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ösungtm 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



  • 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. ...



  • @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!



  • 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.



  • @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 ...



  • @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.



  • 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.



  • 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.



  • @docshoe sagte in if-Bedingung wird nie wahr:

    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.

    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!



  • @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".



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