Multiply images - problem z wydajnością programu - pomoc w lepszej implementacji

Dzień dobry. Napisałem w c# program który generuje z bitmapy kolorowe ASCII arty. Mam problem z wydajnością podczas kolorowania obrazka ( w rozdzielczości 1920x1080 generowanie trwa około 10 sekund ). Realizuję operację w ten sposób, że bitmapę z szarym ASCII artem mnożę (jak w PS) razy piksele obrazka kolorowego. Zaznaczyłem w kodzie ten fragment w którym odbywają się najdłuższe obliczenia, modyfikacja innych nie wpływa znacząco na wydajność.

public static Bitmap MultiplyImages(Bitmap OriginalImage, Bitmap AsciiImage, int width)
        {
            float scale = (OriginalImage.Width / width);
            int newheight = (int)(OriginalImage.Height / scale) / 2;
            int newwidth = (int)(OriginalImage.Width / scale);
            Bitmap NewImage = new Bitmap(OriginalImage);
            Bitmap resized = new Bitmap(OriginalImage, new Size(newwidth, newheight));
            Bitmap OriginalImage2 = new Bitmap(resized, new Size(AsciiImage.Width, AsciiImage.Height));
            Color temp = new Color();
            Color temp2 = new Color();
            Color NewColor = new Color();
            int[] CurrentColor = new int[3]; 

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//TUTAJ ZACZYNA SIĘ MNOŻENIE PIKSELI 
            for (int i = 0; i < OriginalImage2.Width; i++)
            {
                for (int j = 0; j < OriginalImage2.Height; j++)
                {
                    temp = OriginalImage2.GetPixel(i, j);
                    temp2 = AsciiImage.GetPixel(i, j);
                    CurrentColor[0] = (int)(temp.R * temp2.GetBrightness());
                    CurrentColor[1] = (int)(temp.G * temp2.GetBrightness());
                    CurrentColor[2] = (int)(temp.B * temp2.GetBrightness());
                    NewColor = Color.FromArgb(CurrentColor[0], CurrentColor[1], CurrentColor[2]);
                    NewImage.SetPixel(i, j, NewColor);
                }
            }
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

            return NewImage;
        } 

Jak widać pobieram kolory pikseli obydwu obrazów za pomocą Bitmap.Getpixel(), mnożę składowe kolorów razy jasność piksela bitmapy z ASCII Artem, później tworzę nową zmienną typu System.Drawing.Color i zapisuję piksel do mapy wynikowej za pomocą Bitmap.Setpixel(). Jest to najbardziej intuicyjny sposób z użyciem klasy Bitmap. Bardzo proszę o poradę jak można to zrobić bardziej optymalnie. Mam nadzieję że wyjaśniłem problem w sposób jasny i rozumiecie o co chodzi;) Dziękuję za każdą pomoc o pozdrawiam.

Przeglądanie bit mapy w względem osi y jest kilka razy wolniejsze iż przeglądanie jej po x, bo 'źle trafia' do cache, to jest ważne jeśli korzystać ze wskaźników.
Możesz to zrobić korzystając z unsafe code i wskaźników. By odblokować unsafe musisz wejść do właściwośći projektu-> build i znaczyć odpowiednią opcje. Przy okazji pozwól kompilatorowi optymalizować kod i korzystaj z najnowszej wersji VS bo może ma lepszy kompilator(przynajmniej vs 2015 miało lepszy niż VS 2013). Relese mode jest podobno szybszy. Tu masz przykład ze wskaźnikami. Jak już z nich korzystasz, to kolory wydobywaj ze wskaźników za pomocą operacji binarnych, nie dotykaj klasy Color i jeśli nie musisz nie twórz żadnych zmiennych w pętli. Pętla chodzi ~ 2 miliony razy, wiec deklaracja inta co tykniecie wymaga alokacji 8MB gdzie faktycznie potrzebujesz 4 bajtów...., z tego samego powodu warto zastanowić się nad metodami w pętli, 95/100 przypadków nie robią one różnicy, ale czasem spowalniają kod kilku krotnie. Tutaj ciężko zgadnąć JIT i spółka zrobi musisz sam sprawdzić.
Zmienne dostępne przez this.Variable lub obiect.Variable są wolniejsze od lokalnych, wiec może opłacać się je przepisać. Unsafe code jest raczej nie chętnie optymalizowany wiec staraj się mieć te bloki możliwie krótkie.
Generalnie to powiano wystarczyć by zejść do setek milisekund, na pewno poniżej sekundy. Możesz też użyć task.Run( () =>... i przetwarzać obraz w kilku wątkach
http://stackoverflow.com/questions/28323448/fast-bitmap-modifying-using-bitmapdata-and-pointers-in-c-sharp

Dzięki za podpowiedź. Dowiedziałem się dzięki temu co to jest Lockbits i UnlockBits (choć wciąż nie rozumiem dlaczego nie ma w standardzie metod bez tego rozwiązania). Napisałem coś takiego - zdaje się fajnie działać, bo udało mi się zbić czas o połowę.

public static Bitmap MultiplyImages(Bitmap OriginalImage2, Bitmap AsciiImage, int width)
        {
            Bitmap OriginalImage = new Bitmap(OriginalImage2);
            Rectangle rect = new Rectangle(0, 0, OriginalImage.Width, OriginalImage.Height);
            System.Drawing.Imaging.BitmapData Data = OriginalImage.LockBits(rect, System.Drawing.Imaging.ImageLockMode.ReadWrite, OriginalImage.PixelFormat);
            IntPtr ptr = Data.Scan0;
            int bytes = Math.Abs(Data.Stride) * OriginalImage.Height;
            byte[] rgbvalues = new byte[bytes];
            System.Runtime.InteropServices.Marshal.Copy(ptr, rgbvalues, 0, bytes);

            Rectangle rect2 = new Rectangle(0, 0, AsciiImage.Width, AsciiImage.Height);
            System.Drawing.Imaging.BitmapData Data2 = AsciiImage.LockBits(rect2, System.Drawing.Imaging.ImageLockMode.ReadWrite, OriginalImage.PixelFormat);
            IntPtr ptr2 = Data2.Scan0;
            int bytes2 = Math.Abs(Data2.Stride) * AsciiImage.Height;
            byte[] rgbvalues2 = new byte[bytes2];
            System.Runtime.InteropServices.Marshal.Copy(ptr2, rgbvalues2, 0, bytes2);

            byte[] rgbvalues3 = new byte[bytes];
            for (int i = 0; i < rgbvalues.Length; i += 4)
            {
                byte luminance = (byte)((rgbvalues2[i] * 2 + rgbvalues2[i + 1] * 3 + rgbvalues2[i + 2]) / 6);
                rgbvalues3[i] = (byte) (rgbvalues[i] * luminance/255);
                rgbvalues3[i + 1] = (byte)(rgbvalues[i+1] * luminance/255);
                rgbvalues3[i + 2] = (byte)(rgbvalues[i+2] * luminance/255);
            }
            System.Runtime.InteropServices.Marshal.Copy(rgbvalues3, 0, ptr, bytes);
            OriginalImage.UnlockBits(Data);
            return OriginalImage;
        } 

Jak widać program robi zasadniczo to samo, używa tylko metody System.Runtime.InteropServices.Marshal.Copy() do zrobienia z Bitmap zwykłej tablicy typu byte. Zastanawiam się czy da się jeszcze w jakiś sposób zbić czas wykonywania programu. Będę wdzięczny za podpowiedzi.

ImageLockMode.ReadWrite

Używaj ReadOnly/WriteOnly zgodnie z potrzebą. Unikaj ReadWrite jeśli nie jest konieczne.
Staraj się mieć zawsze taki nawyk tam gdzie masz takie flagi dostępu.

System.Runtime.InteropServices.Marshal.Copy(ptr2, rgbvalues2, 0, bytes2);

System.Runtime.InteropServices.Marshal.Copy(rgbvalues3, 0, ptr, bytes);

Mógłbyś jeszcze czas zbić pozbywając się tego kopiowania. Można to zrobić za pomocą wskaźników. We właściwościach projektu trzeba włączyć "allow unsafe code" i dodać unsafe do nagłówka funkcji (public static unsafe Bitmap MultiplyImages(...)).
Teraz zamiast

IntPtr ptr2 = Data2.Scan0;

coś takiego

byte* scanline = (byte*)(void*)Data2.Scan0;

i możesz pisać po scanline jak po tablicy bez kopiowania. Tylko nie wyjedź poza zakres danych bo wskaźniki nie mają żadnej kontroli zakresu (dlatego są “unsafe”).

Dziękuję;) Przepisałem wszystkie funkcje na modłę z postu #3 i udało się zjechać poniżej pół sekundy. Spróbuję jeszcze wieczorem zmodyfikować kod tak jak piszesz, ale najpierw poczytam na ten temat. Jeszcze raz dzięki i pozdrawiam;)

Możesz przepisać to:

  byte luminance = (byte)((rgbvalues2[i] * 2 + rgbvalues2[i + 1] * 3 + rgbvalues2[i + 2]) / 6);

na coś takiego.

  byte luminance = (byte)((rgbvalues2[i] <<1)
                                     + rgbvalues2[i + 1] + ( rgbvalues2[i + 1]<<1) 
                                     + rgbvalues2[i + 2]/6) ;

przy 500ms zrobi to różnice.

Dzięki jeszcze raz. Mam pytanie odnoszące się do paralellizacji obliczeń w c#. Przepisałem sobie funkcję do pętli Parallel.For:

  Parallel.For(0, rgbvalues.Length, i =>
            {
                if (i % 4 == 0)
                {
                    luminance = (byte)((rgbvalues2[i] * 2 + rgbvalues2[i + 1] * 3 + rgbvalues2[i + 2]) / 6);
                    rgbvalues3[i] = (byte)(rgbvalues[i] * luminance / 255);
                    if (luminance > 32) { rgbvalues3[i] = (byte) Clamp(rgbvalues3[i] * 2, 0, 255); }
                }
                else if (i % 4 == 1)
                {
                    luminance2 = (byte)((rgbvalues2[i - 1] * 2 + rgbvalues2[i] * 3 + rgbvalues2[i + 1]) / 6);
                    rgbvalues3[i] = (byte)(rgbvalues[i] * luminance2 / 255);
                    if (luminance2 > 16) { rgbvalues3[i] = (byte)Clamp(rgbvalues3[i] * 2, 0, 255); }
                }
                else if (i % 4 == 2)
                {
                    luminance3 = (byte)((rgbvalues2[i - 2] * 2 + rgbvalues2[i - 1] * 3 + rgbvalues2[i]) / 6);
                    rgbvalues3[i] = (byte)(rgbvalues[i] * luminance3 / 255);
                    if (luminance3 > 32) { rgbvalues3[i] = (byte)Clamp(rgbvalues3[i] * 2, 0, 255); }
                }
                else rgbvalues3[i] = rgbvalues[i];
            });

Ze względu na to że luminancja jest taka sama dla 4 elementów (bo są to składowe piksela) liczę luminancję za każdym razem. Zysk szybkościowy faktycznie jest. Czy da się tę pętlę zapisać w ten sposób, żeby każdy wątek liczył 4 przejścia pętli? Mógłbym w ten sposób zaoszczędzić liczenia za każdym razem osobno luminancji piksela.

Niestety te przesunięcia bitowe wywalają mi błędy, pewnie ze względu na to że rgbvalues są typu byte. Zapewne podczas liczenia luminancji są faktycznie konwertowane na int, ale nie wiem jak miałby wyglądać kod żeby faktycznie zyskać trochę czasu na tej operacji;(