jackieh / palette-pick Goto Github PK

View Code? Open in Web Editor NEW

0.0 0.0 1.0 4.16 MB

Color picker and color scheme manager.

License: GNU General Public License v3.0

Makefile 6.08% C++ 83.00% Shell 10.91%

palette-pick's People

Contributors

Watchers

Forkers

cowile

palette-pick's Issues

Results of get_hue_spread_colors are discarded.

I saw the results of the hue spread method are not noticeably different from a random spread. I followed this in the code and found a likely bug.

Before being passed into find_clusters, sample_colors is initialized with get_hue_spread_colors. But sample_colors is an output parameter that is cleared by find_clusters, meaning the initialization changes nothing.

palette-pick/src/lib/image.cpp

Lines 107 to 130 in a34485a

 case ImageGetSampleColorsMode::Value::kmeans_hue_spread: 

 sample_colors = get_hue_spread_colors(num_colors); 

 success = ColorKMeans::find_clusters( 

 num_colors, ColorKMeans::SeedMode::keep_existing, 

 get_unique_colors(), sample_colors); 

 break; 

 case ImageGetSampleColorsMode::Value::kmeans_bright_hue_spread: { 

 const std::vector<Color> unique_colors = get_unique_colors(); 

 std::vector<Color> data_colors = get_bright_colors(unique_colors); 

 sample_colors = get_hue_spread_colors(num_colors); 

 success = ColorKMeans::find_clusters( 

 num_colors, ColorKMeans::SeedMode::keep_existing, 

 data_colors, sample_colors); 

 break; 

 } 

 case ImageGetSampleColorsMode::Value::kmeans_saturated_hue_spread: { 

 const std::vector<Color> unique_colors = get_unique_colors(); 

 std::vector<Color> data_colors = 

 get_saturated_colors(unique_colors); 

 sample_colors = get_hue_spread_colors(num_colors); 

 success = ColorKMeans::find_clusters( 

 num_colors, ColorKMeans::SeedMode::keep_existing, 

 data_colors, sample_colors); 

 break;

palette-pick/src/lib/color_k_means.cpp

Line 21 in a34485a

color_centroids.clear();

Reserving space up front is less efficient due to cost of totalColors()

After profiling, removing the lines reserving space gives a 2.4x speed increase in the getcolors tool.

palette-pick/src/lib/image.cpp

Lines 74 to 76 in a34485a

 unique_colors.reserve(image_.totalColors()); 

 std::vector<std::pair<Magick::Color, size_t>> color_histogram; 

 color_histogram.reserve(image_.totalColors());

mkstripes loop is O(n^2) when it could be O(n)

Instead of partitioning the canvas into separate rectangles to fill, this loop fills progressively smaller overlapping rectangles. This results in the loop being quadratic to the number of pixels rather than linear.

After profiling, making the loop linear results in an 8.8x speedup in the mkstripes program.

palette-pick/src/lib/stripes_image.cpp

Lines 70 to 89 in a34485a

 for (const auto &stripe_color : stripe_colors_.get()) { 

 stripes.fillColor(stripe_color.get()); 

 int stripe_width_begin = stripe_idx * stripe_width_; 

 switch (stripe_orientation_.get()) { 

 case Orientation::Value::vertical: 

 stripes.draw(Magick::DrawableRectangle( 

 stripe_width_begin, 0, 

 image_width, image_height)); 

 break; 

 case Orientation::Value::horizontal: 

 stripes.draw(Magick::DrawableRectangle( 

 0, stripe_width_begin, 

 image_width, image_height)); 

 break; 

 default: 

 error_stream << "Unknown stripes orientation" << std::endl; 

 return false; 

 } 

 ++stripe_idx; 

 }

Initialization loop is pointless.

Because color_centroids was cleared before the loop, size will always be zero so c will never be less than.

The rest of the loop initializes all matrix elements to zero. armadillo offers a zeros function to return a zero matrix directly. That should be used instead of eye.

palette-pick/src/lib/color_k_means.cpp

Lines 24 to 36 in a34485a

 arma::mat means = arma::eye(3, num_clusters); 

 for (size_t c = 0; c < num_clusters; ++c) { 

 if (c < color_centroids.size()) { 

 const Color &color = color_centroids.at(c); 

 means(0, c) = static_cast<int>(color.get().quantumRed()); 

 means(1, c) = static_cast<int>(color.get().quantumGreen()); 

 means(2, c) = static_cast<int>(color.get().quantumBlue()); 

 } else { 

 means(0, c) = 0; 

 means(1, c) = 0; 

 means(2, c) = 0; 

 } 

 }

jackieh / palette-pick Goto Github PK

palette-pick's People

Contributors

Watchers

Forkers

palette-pick's Issues

Results of get_hue_spread_colors are discarded.

Reserving space up front is less efficient due to cost of totalColors()

mkstripes loop is O(n^2) when it could be O(n)

Initialization loop is pointless.

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	case ImageGetSampleColorsMode::Value::kmeans_hue_spread:
	sample_colors = get_hue_spread_colors(num_colors);
	success = ColorKMeans::find_clusters(
	num_colors, ColorKMeans::SeedMode::keep_existing,
	get_unique_colors(), sample_colors);
	break;
	case ImageGetSampleColorsMode::Value::kmeans_bright_hue_spread: {
	const std::vector<Color> unique_colors = get_unique_colors();
	std::vector<Color> data_colors = get_bright_colors(unique_colors);
	sample_colors = get_hue_spread_colors(num_colors);
	success = ColorKMeans::find_clusters(
	num_colors, ColorKMeans::SeedMode::keep_existing,
	data_colors, sample_colors);
	break;
	}
	case ImageGetSampleColorsMode::Value::kmeans_saturated_hue_spread: {
	const std::vector<Color> unique_colors = get_unique_colors();
	std::vector<Color> data_colors =
	get_saturated_colors(unique_colors);
	sample_colors = get_hue_spread_colors(num_colors);
	success = ColorKMeans::find_clusters(
	num_colors, ColorKMeans::SeedMode::keep_existing,
	data_colors, sample_colors);
	break;

	unique_colors.reserve(image_.totalColors());
	std::vector<std::pair<Magick::Color, size_t>> color_histogram;
	color_histogram.reserve(image_.totalColors());

	for (const auto &stripe_color : stripe_colors_.get()) {
	stripes.fillColor(stripe_color.get());
	int stripe_width_begin = stripe_idx * stripe_width_;
	switch (stripe_orientation_.get()) {
	case Orientation::Value::vertical:
	stripes.draw(Magick::DrawableRectangle(
	stripe_width_begin, 0,
	image_width, image_height));
	break;
	case Orientation::Value::horizontal:
	stripes.draw(Magick::DrawableRectangle(
	0, stripe_width_begin,
	image_width, image_height));
	break;
	default:
	error_stream << "Unknown stripes orientation" << std::endl;
	return false;
	}
	++stripe_idx;
	}

	arma::mat means = arma::eye(3, num_clusters);
	for (size_t c = 0; c < num_clusters; ++c) {
	if (c < color_centroids.size()) {
	const Color &color = color_centroids.at(c);
	means(0, c) = static_cast<int>(color.get().quantumRed());
	means(1, c) = static_cast<int>(color.get().quantumGreen());
	means(2, c) = static_cast<int>(color.get().quantumBlue());
	} else {
	means(0, c) = 0;
	means(1, c) = 0;
	means(2, c) = 0;
	}
	}