Алгоритм сегментации изображения K-Means

Я новый программист на C ++, и у меня есть некоторый опыт работы на Python и C, но я почти полностью самоучился (я изучил C ++ с OpenClassrooms ).

Я хотел бы изучить конвенции и как все должно быть сделано. Но я также хотел бы знать , почему метод должен использоваться над другим.

Это мой первый реальный проект на C ++. Любые советы или исправления приветствуются. Меня больше интересуют методы на C ++, чем конкретно с этим алгоритмом, но если я делаю что-то неэффективно, я предлагаю вам внести исправления.

GitHub

main.cpp

/* K-Means image segmentation
 *
 * This program only works with uncompressed images
 * PPM with type P3
 */

#include "../include/image.h"
#include <iostream>
#include <string>
#include <ctime>
#include <cstdlib>
#include <vector>

using namespace std;

int main()
{
    // Set the seed for better random generation
    srand(time(NULL));

    // Get the path of the original image
    string imageDir;
    cout << "Image path: ";
    cin >> imageDir;
    Image *image(new Image(imageDir));

    // Get the path of the image to save
    string saveImageAs;
    cout << "Save image as: ";
    cin >> saveImageAs;

    // Create each cluster
    int clusterCount;
    cout << "How many colours? ";
    cin >> clusterCount;
    vector<Cluster*> clusters;
    for (int i(0); i < clusterCount; i++)
    {
        clusters.push_back(new Cluster(image));
    }

    // Get the threshold
    double threshold;
    cout << "Threshold: ";
    cin >> threshold;

    // Repeat the algorithm until the average centroid change goes below the threshold
    double averageCentroidChange;
    do
    {
        // Clear all pixels for each cluster
        for (int i(0); i < clusterCount; i++)
        {
            clusters[i]->clearPixels();
        }

        // Go through each pixel in the image
        for (int i(0); i < image->getLength(); i++)
        {
            // Calculate which cluster centroid the pixel is nearest to
            int closestClusterIndex(0);
            double dist;
            for (int j(0); j < clusters.size(); j++)
            {
                dist = clusters[j]->getDistanceTo(image->getPixel(i));
                if (dist < clusters[closestClusterIndex]->getDistanceTo(image->getPixel(i)))
                {
                    closestClusterIndex = j;
                }
            }

            // Add the pixel to the nearest cluster
            clusters[closestClusterIndex]->addPixel(image->getPixel(i));
        }

        // Calculate the average change of the centroids
        averageCentroidChange = 0;
        for (int i(0); i < clusters.size(); i++)
        {
            averageCentroidChange += clusters[i]->adjustCentroid();
        }
        averageCentroidChange /= clusters.size();
        cout << "Average centroid change: " << averageCentroidChange << endl;
    } while (averageCentroidChange > threshold);

    // Change all pixels to the color of the corresponding cluster centroid
    for (int i(0); i < clusters.size(); i++)
    {
        clusters[i]->changeAll();
    }

    // Save the new image
    image->saveImage(saveImageAs);

    return 0;
}

image.h

#ifndef IMAGE_H_INCLUDED
#define IMAGE_H_INCLUDED

#include <string>
#include <vector>

class Pixel
{
    public:
        Pixel(int red, int green, int blue);
        Pixel(Pixel *pixel);

        int getRed() const;
        int getGreen() const;
        int getBlue() const;
        std::string getRGB() const;
        void setRGB(int red, int green, int blue);

    private:
        int m_red;
        int m_green;
        int m_blue;
};

class Image
{
    public:
        Image(int width, int height);
        Image(std::string name);
        ~Image();

        void saveImage(std::string savePath) const;
        Pixel* getRandPixel() const;
        std::vector<Pixel*> getPixels() const;
        Pixel* getPixel(int index) const;
        int getLength() const;

    private:
        int m_width;
        int m_height;
        int m_depth;
        std::vector<Pixel*> m_pixels;
};

class Cluster
{
    public:
        Cluster(Image *image);
        ~Cluster();

        double adjustCentroid();
        double getDistanceTo(Pixel *pixel) const;
        double getDistanceTo(int red, int green, int blue) const;
        void addPixel(Pixel *pixel);
        void clearPixels();
        void changeAll();

    private:
        Image *m_image;
        Pixel *m_centroid;
        std::vector<Pixel*> m_pixels;
};

#endif

image.cpp

#include "../include/image.h"
#include <cmath>
#include <iostream>
#include <string>
#include <vector>
#include <fstream>
#include <cstdlib>

using namespace std;

Pixel::Pixel(int red, int green, int blue) : m_red(red), m_green(green), m_blue(blue) {}

Pixel::Pixel(Pixel *pixel) : m_red(pixel->m_red), m_green(pixel->m_green), m_blue(pixel->m_blue) {}

int Pixel::getRed() const
{
    return m_red;
}

int Pixel::getGreen() const
{
    return m_green;
}

int Pixel::getBlue() const
{
    return m_blue;
}

string Pixel::getRGB() const
{
    return to_string(m_red) + " " + to_string(m_green) + " " + to_string(m_blue);
}

void Pixel::setRGB(int red, int green, int blue)
{
    m_red = red;
    m_green = green;
    m_blue = blue;
}

Image::Image(int width, int height) : m_width(width), m_height(height)
{
    for (int i(0); i < m_width*m_height; i++)
    {
        m_pixels.push_back(new Pixel(0, 0, 0));
    }
}

Image::Image(string imageDir)
{
    ifstream image(imageDir);
    if (image)
    {
        string type;
        image >> type;
        if (type == "P3")
        {
            int red;
            int green;
            int blue;
            image >> m_width;
            image >> m_height;
            image >> m_depth;
            for (int i(0); i < m_width*m_height; i++)
            {
                image >> red;
                image >> green;
                image >> blue;
                m_pixels.push_back(new Pixel(red, green, blue));
            }
        } else {
            cout << imageDir << " is in the wrong format (should be P3)" << endl;
        }
    } else {
        cout << imageDir << " could not be opened!" << endl;
    }
}

Image::~Image()
{
    for (int i(0); i < m_width*m_height; i++)
    {
        delete m_pixels[i];
    }
}

void Image::saveImage(string name) const
{
    ofstream image(name);
    if (image)
    {
        image << "P3" << endl;
        image << m_width << " " << m_height << endl;
        image << m_depth << endl;
        for (int y(0); y < m_width; y++)
        {
            for (int x(0); x < m_height; x++)
            {
                Pixel *pixel = m_pixels[m_height*y + x];
                image << pixel->getRGB() << " ";
            }
            image << endl;
        }
    } else {
        cout << name << ".ppm could not be opened" << endl;
    }
}

Pixel* Image::getRandPixel() const
{
    return m_pixels[rand() % m_width*m_height];
}

vector<Pixel*> Image::getPixels() const
{
    return m_pixels;
}

Pixel* Image::getPixel(int index) const
{
    return m_pixels[index];
}

int Image::getLength() const
{
    return m_width*m_height;
}

Cluster::Cluster(Image *image) : m_image(image), m_centroid(new Pixel(image->getRandPixel()))
{
}

Cluster::~Cluster()
{
    delete m_centroid;
}

double Cluster::adjustCentroid()
{
    float red(0);
    float green(0);
    float blue(0);

    for (int i(0); i < m_pixels.size(); i++)
    {
        red += m_pixels[i]->getRed();
        green += m_pixels[i]->getGreen();
        blue += m_pixels[i]->getBlue();
    }

    int denominator(m_pixels.size());
    if (m_pixels.size() < 1)
    {
        denominator = 1;
    }

    red /= denominator;
    green /= denominator;
    blue /= denominator;

    double change(this->getDistanceTo(red, green, blue));

    m_centroid->setRGB(red, green, blue);

    return change;
}

double Cluster::getDistanceTo(Pixel *pixel) const
{
    int diffRed(pixel->getRed() - m_centroid->getRed());
    int diffGreen(pixel->getGreen() - m_centroid->getGreen());
    int diffBlue(pixel->getBlue() - m_centroid->getBlue());

    return sqrt(pow(diffRed, 2) + pow(diffGreen, 2) + pow(diffBlue, 2));
}

double Cluster::getDistanceTo(int red, int green, int blue) const
{
    int diffRed(red - m_centroid->getRed());
    int diffGreen(green - m_centroid->getGreen());
    int diffBlue(blue - m_centroid->getBlue());

    return sqrt(pow(diffRed, 2) + pow(diffGreen, 2) + pow(diffBlue, 2));
}

void Cluster::addPixel(Pixel *pixel)
{
    m_pixels.push_back(pixel);
}

void Cluster::clearPixels()
{
    m_pixels = {};
}

void Cluster::changeAll()
{
    for (int i(0); i < m_pixels.size(); i++)
    {
        m_pixels[i]->setRGB(m_centroid->getRed(), m_centroid->getGreen(), m_centroid->getBlue());
    }
}
11 голосов | спросил Timéo Pochin 2 PMpMon, 02 Apr 2018 20:26:55 +030026Monday 2018, 20:26:55

2 ответа


10

Я вообще не знаком с обработкой изображений, поэтому я не могу дать вам никаких рекомендаций относительно реализации вашего алгоритма. Однако есть немало вещей, которые я хотел бы сказать о хороших практиках и стиле кода в целом:

  1. Не используйте using namespace std. Вы столкнетесь с проблемами разрешения имен по строке, а дополнительные пять символов (std::), каждая из которых не собирается убивать вы или ваша производительность.
  2. Не используйте относительные включения (например, #include "../include/image.h"). Это может привести к поломке, если вы когда-либо изменили макет проекта или перемещали файлы. Правильное место, чтобы указать, какие каталоги, которые следует учитывать для включения, является вашим сценарием сборки (или в самом основном случае вызовом командной строки вашего компилятора).
  3. Не используйте NULL как константу нулевого указателя, используйте nullptr. Последний имеет дополнительный уровень безопасности типов.
  4. Не используйте rand и друзей. Начиная с C ++ 11, стандартная библиотека содержит гораздо более мелкозернистую и гибкую поддержку для генерации случайных чисел. Взгляните на произвольный заголовок.
  5. Почему вы пишете Image *image(new Image(imageDir)), а не Image image(imageDir)? Здесь нет необходимости использовать указатель. Фактически, в современном C ++ необработанный указатель указывает на то, что в большинстве случаев что-то не так. Это верно для каждого использования указателя во всем коде; ни один из них не является действительно необходимым или полезным.
  6. main слишком длинный и перегружен кодом. Разделите его на несколько меньших функций с четко определенной целью. Это упростит чтение и исправление кода.
  7. Относясь назад к пункту 5, не записывайте конструкторы формы Class(Class* other). Вы хотите скопировать other здесь, поэтому вы должны написать конструктор копирования . Например, подпись для Pixel должна быть Pixel(Pixel const& other)
  8. Типы паролей, которые можно скопировать по ссылке или const. В частности, std::string должен быть передан как std::string const& в большинстве случаев.
  9. Используйте для каждой петли при прохождении через весь контейнер. Например,

    for (int i(0); i < m_pixels.size(); i++)
    {
        red += m_pixels[i]->getRed();
        green += m_pixels[i]->getGreen();
        blue += m_pixels[i]->getBlue();
    }
    

    следует переписать как

    for (auto p : m_pixels)
    {
        red += p->getRed();
        green += p->getGreen();
        blue += p->getBlue();
    }
    

    или аналогичные, чтобы избежать ошибок проверки индексов и границ (например, вы можете столкнуться с проблемами с вашей версией здесь, потому что int может не быть достаточно большой, чтобы удерживать все значения индекса для m_pixels).

  10. Не полагайтесь на неявное преобразование при назначении значений типам. Правильный инициализатор для числа с плавающей точкой с одной точностью имеет вид [0-9]*\.[0-9]*f, например (это означает, что вы должны написать float red(0.0f) вместо float red(0)). Неявное преобразование иногда может погубить ваш день, потому что это может привести к неожиданным результатам, которые трудно диагностировать.
  11. Не используйте std::endl, используйте '\n'. std::endl также очистит базовый буфер потока, который вам обычно не нужно делать и который может нанести вред производительности, если вы много делаете ввода /вывода (также, если вам действительно нужно и нужно сбросить, есть std::flush).
ответил Ben Steffan 2 PMpMon, 02 Apr 2018 22:34:31 +030034Monday 2018, 22:34:31
7

Я хочу подчеркнуть, что Бен Штеффан сделал в своем ответе :

Эта часть действительно bad:

std::vector<Pixel*> m_pixels;

Это было бы намного лучше:

std::vector<Pixel> m_pixels;

Вот некоторые из причин:

  1. Выделяя каждый пиксель отдельно, вы делаете несколько миллионов распределений для изображения нормального размера. Это действительно дорого, это может быть одно выделение.

  2. Выделяя каждый пиксель отдельно и сохраняя их как голые указатели, вам нужно написать деструктор для вашего класса Image , В альтернативном сценарии вы можете позволить компилятору сгенерировать деструктор по умолчанию.

  3. Выделяя каждый пиксель отдельно, они будут (скорее всего) разбросаны по памяти, а не сохраняются смежно. Непрерывное хранилище делает доступ к пикселям намного быстрее. Кроме того, с указателями, для каждого доступа к пикселям у вас есть дополнительный уровень косвенности.

  4. Сохраняя голые указатели, вы, вероятно, создадите утечки памяти. Вместо этого используйте std::unique_ptr или std::shared_ptr ("smart указатели "). Например, здесь у вас есть утечка памяти:

    void Cluster::clearPixels()
    {
        m_pixels = {};
    }
    

    Или нет? Это не ясно, пока вы внимательно прочитаете всю программу и не узнаете, что этот вектор содержит ссылки на пиксели, принадлежащие другому объекту. Собственность важна для определения. Используя интеллектуальные указатели, вы создаете право собственности. При использовании голой указатели, подумайте о том, чтобы документировать в комментариях, кому что принадлежит. Если вы последовательно используете интеллектуальные указатели для владения, вы свободны (IMO), чтобы использовать голой указатель для ссылок, не связанных с владельцем, как в случае с кодом m_pixels в Cluster.

ответил Cris Luengo 3 AMpTue, 03 Apr 2018 00:35:05 +030035Tuesday 2018, 00:35:05

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132