Color ASCII drawing class
$begingroup$
I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl;
I wrote this little class to organize an ASCII-character "framebuffer":
#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>
struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};
class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;
public:
const size_t width;
const size_t height;
Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};
There is a bug: when destructing a Framebuffer
it can happen that the static std::thread printerThread
never gets joined or otherwise terminated.
c++ console linux ascii-art
$endgroup$
add a comment |
$begingroup$
I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl;
I wrote this little class to organize an ASCII-character "framebuffer":
#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>
struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};
class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;
public:
const size_t width;
const size_t height;
Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};
There is a bug: when destructing a Framebuffer
it can happen that the static std::thread printerThread
never gets joined or otherwise terminated.
c++ console linux ascii-art
$endgroup$
1
$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago
$begingroup$
InsetBackgroundColor
, takestd::vector<std::vector<Color>>
by const-ref instead since you don't modify the object. The copy is unnecessary.
$endgroup$
– Juho
15 hours ago
add a comment |
$begingroup$
I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl;
I wrote this little class to organize an ASCII-character "framebuffer":
#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>
struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};
class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;
public:
const size_t width;
const size_t height;
Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};
There is a bug: when destructing a Framebuffer
it can happen that the static std::thread printerThread
never gets joined or otherwise terminated.
c++ console linux ascii-art
$endgroup$
I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl;
I wrote this little class to organize an ASCII-character "framebuffer":
#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>
struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};
class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;
public:
const size_t width;
const size_t height;
Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};
There is a bug: when destructing a Framebuffer
it can happen that the static std::thread printerThread
never gets joined or otherwise terminated.
c++ console linux ascii-art
c++ console linux ascii-art
edited 18 mins ago
Jamal♦
30.4k11121227
30.4k11121227
asked yesterday
Darius DuesentriebDarius Duesentrieb
24018
24018
1
$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago
$begingroup$
InsetBackgroundColor
, takestd::vector<std::vector<Color>>
by const-ref instead since you don't modify the object. The copy is unnecessary.
$endgroup$
– Juho
15 hours ago
add a comment |
1
$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago
$begingroup$
InsetBackgroundColor
, takestd::vector<std::vector<Color>>
by const-ref instead since you don't modify the object. The copy is unnecessary.
$endgroup$
– Juho
15 hours ago
1
1
$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago
$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago
$begingroup$
In
setBackgroundColor
, take std::vector<std::vector<Color>>
by const-ref instead since you don't modify the object. The copy is unnecessary.$endgroup$
– Juho
15 hours ago
$begingroup$
In
setBackgroundColor
, take std::vector<std::vector<Color>>
by const-ref instead since you don't modify the object. The copy is unnecessary.$endgroup$
– Juho
15 hours ago
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
- Order your includes at least by portable / non-portable.
- Not a huge fan of omitting
private
and putting all the private members up top. IMO a class interface should go frompublic
toprivate
which makes for easier reading as a user. - The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.
- Is there a reason not to use
memset
in your clear function? - Pedantic people might complain about the missing header for
size_t
and the missingstd::
qualifier.
std::string output = "";
initializing strings this way always looks weird to me.std::string s;
should suffice but to declare intent more clearly you can dostd::string{""};
. Purely subjective though.- Always a good idea to get into the habit of using prefix operator over postfix operator.
- I do like that you signal intent with
flush
as opposed to relying onendl
- Not sure if you use
Color
elsewhere but it could probably be an implementation detail instead of being free. - You explicitly state this is for linux so you probably know that
system("clear")
is non-portable and are okay with it.
$endgroup$
$begingroup$
#include <sys/ioctl.h>
#include <unistd.h>
are the non-portables, right?
$endgroup$
– Darius Duesentrieb
yesterday
$begingroup$
@DariusDuesentrieb As far as I can tell, yes.
$endgroup$
– yuri
yesterday
add a comment |
$begingroup$
Framebuffer()
Could end with
charBuffer = std::vector<char>(height * width, ' ');
textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
backgroundColorBuffer = std::vector<Color>(height * width);
instead of calling clear.
void clear()
Alternative implementation and let container implementation decide what is most effective.
charBuffer.assign(charBuffer.size(), ' ');
textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});
void setChar()
Don't copy the box in the interface, use a const reference. And don't call size()
more than necessary.
void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor()
Again, don't copy the box on each call. And another use of references inside the loops.
void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
auto & line = box[rowOffset];
for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
}
}
}
void setBackgroundColor()
Similar comments regarding setBackgroundColor
.
void print()
Alternative lambda with std::stringstream
.
auto printer = [this]() {
std::stringstream output;
for (size_t row = 0; row < height; row++) {
for (size_t col = 0; col < width; col++) {
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output << "33[38;2;"
<< static_cast<int>(textColor.r) << ';'
<< static_cast<int>(textColor.g) << ';'
<< static_cast<int>(textColor.b) << "m"
"33[48;2;"
<< static_cast<int>(backgroundColor.r) << ';'
<< static_cast<int>(backgroundColor.g) << ';'
<< static_cast<int>(backgroundColor.b) << 'm'
<< getChar(col, row);
}
if (row != height - 1) {
output << 'n';
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output.rdbuf() << std::flush;
};
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215519%2fcolor-ascii-drawing-class%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
- Order your includes at least by portable / non-portable.
- Not a huge fan of omitting
private
and putting all the private members up top. IMO a class interface should go frompublic
toprivate
which makes for easier reading as a user. - The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.
- Is there a reason not to use
memset
in your clear function? - Pedantic people might complain about the missing header for
size_t
and the missingstd::
qualifier.
std::string output = "";
initializing strings this way always looks weird to me.std::string s;
should suffice but to declare intent more clearly you can dostd::string{""};
. Purely subjective though.- Always a good idea to get into the habit of using prefix operator over postfix operator.
- I do like that you signal intent with
flush
as opposed to relying onendl
- Not sure if you use
Color
elsewhere but it could probably be an implementation detail instead of being free. - You explicitly state this is for linux so you probably know that
system("clear")
is non-portable and are okay with it.
$endgroup$
$begingroup$
#include <sys/ioctl.h>
#include <unistd.h>
are the non-portables, right?
$endgroup$
– Darius Duesentrieb
yesterday
$begingroup$
@DariusDuesentrieb As far as I can tell, yes.
$endgroup$
– yuri
yesterday
add a comment |
$begingroup$
- Order your includes at least by portable / non-portable.
- Not a huge fan of omitting
private
and putting all the private members up top. IMO a class interface should go frompublic
toprivate
which makes for easier reading as a user. - The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.
- Is there a reason not to use
memset
in your clear function? - Pedantic people might complain about the missing header for
size_t
and the missingstd::
qualifier.
std::string output = "";
initializing strings this way always looks weird to me.std::string s;
should suffice but to declare intent more clearly you can dostd::string{""};
. Purely subjective though.- Always a good idea to get into the habit of using prefix operator over postfix operator.
- I do like that you signal intent with
flush
as opposed to relying onendl
- Not sure if you use
Color
elsewhere but it could probably be an implementation detail instead of being free. - You explicitly state this is for linux so you probably know that
system("clear")
is non-portable and are okay with it.
$endgroup$
$begingroup$
#include <sys/ioctl.h>
#include <unistd.h>
are the non-portables, right?
$endgroup$
– Darius Duesentrieb
yesterday
$begingroup$
@DariusDuesentrieb As far as I can tell, yes.
$endgroup$
– yuri
yesterday
add a comment |
$begingroup$
- Order your includes at least by portable / non-portable.
- Not a huge fan of omitting
private
and putting all the private members up top. IMO a class interface should go frompublic
toprivate
which makes for easier reading as a user. - The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.
- Is there a reason not to use
memset
in your clear function? - Pedantic people might complain about the missing header for
size_t
and the missingstd::
qualifier.
std::string output = "";
initializing strings this way always looks weird to me.std::string s;
should suffice but to declare intent more clearly you can dostd::string{""};
. Purely subjective though.- Always a good idea to get into the habit of using prefix operator over postfix operator.
- I do like that you signal intent with
flush
as opposed to relying onendl
- Not sure if you use
Color
elsewhere but it could probably be an implementation detail instead of being free. - You explicitly state this is for linux so you probably know that
system("clear")
is non-portable and are okay with it.
$endgroup$
- Order your includes at least by portable / non-portable.
- Not a huge fan of omitting
private
and putting all the private members up top. IMO a class interface should go frompublic
toprivate
which makes for easier reading as a user. - The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.
- Is there a reason not to use
memset
in your clear function? - Pedantic people might complain about the missing header for
size_t
and the missingstd::
qualifier.
std::string output = "";
initializing strings this way always looks weird to me.std::string s;
should suffice but to declare intent more clearly you can dostd::string{""};
. Purely subjective though.- Always a good idea to get into the habit of using prefix operator over postfix operator.
- I do like that you signal intent with
flush
as opposed to relying onendl
- Not sure if you use
Color
elsewhere but it could probably be an implementation detail instead of being free. - You explicitly state this is for linux so you probably know that
system("clear")
is non-portable and are okay with it.
answered yesterday
yuriyuri
3,63921034
3,63921034
$begingroup$
#include <sys/ioctl.h>
#include <unistd.h>
are the non-portables, right?
$endgroup$
– Darius Duesentrieb
yesterday
$begingroup$
@DariusDuesentrieb As far as I can tell, yes.
$endgroup$
– yuri
yesterday
add a comment |
$begingroup$
#include <sys/ioctl.h>
#include <unistd.h>
are the non-portables, right?
$endgroup$
– Darius Duesentrieb
yesterday
$begingroup$
@DariusDuesentrieb As far as I can tell, yes.
$endgroup$
– yuri
yesterday
$begingroup$
#include <sys/ioctl.h>
#include <unistd.h>
are the non-portables, right?$endgroup$
– Darius Duesentrieb
yesterday
$begingroup$
#include <sys/ioctl.h>
#include <unistd.h>
are the non-portables, right?$endgroup$
– Darius Duesentrieb
yesterday
$begingroup$
@DariusDuesentrieb As far as I can tell, yes.
$endgroup$
– yuri
yesterday
$begingroup$
@DariusDuesentrieb As far as I can tell, yes.
$endgroup$
– yuri
yesterday
add a comment |
$begingroup$
Framebuffer()
Could end with
charBuffer = std::vector<char>(height * width, ' ');
textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
backgroundColorBuffer = std::vector<Color>(height * width);
instead of calling clear.
void clear()
Alternative implementation and let container implementation decide what is most effective.
charBuffer.assign(charBuffer.size(), ' ');
textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});
void setChar()
Don't copy the box in the interface, use a const reference. And don't call size()
more than necessary.
void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor()
Again, don't copy the box on each call. And another use of references inside the loops.
void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
auto & line = box[rowOffset];
for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
}
}
}
void setBackgroundColor()
Similar comments regarding setBackgroundColor
.
void print()
Alternative lambda with std::stringstream
.
auto printer = [this]() {
std::stringstream output;
for (size_t row = 0; row < height; row++) {
for (size_t col = 0; col < width; col++) {
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output << "33[38;2;"
<< static_cast<int>(textColor.r) << ';'
<< static_cast<int>(textColor.g) << ';'
<< static_cast<int>(textColor.b) << "m"
"33[48;2;"
<< static_cast<int>(backgroundColor.r) << ';'
<< static_cast<int>(backgroundColor.g) << ';'
<< static_cast<int>(backgroundColor.b) << 'm'
<< getChar(col, row);
}
if (row != height - 1) {
output << 'n';
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output.rdbuf() << std::flush;
};
$endgroup$
add a comment |
$begingroup$
Framebuffer()
Could end with
charBuffer = std::vector<char>(height * width, ' ');
textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
backgroundColorBuffer = std::vector<Color>(height * width);
instead of calling clear.
void clear()
Alternative implementation and let container implementation decide what is most effective.
charBuffer.assign(charBuffer.size(), ' ');
textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});
void setChar()
Don't copy the box in the interface, use a const reference. And don't call size()
more than necessary.
void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor()
Again, don't copy the box on each call. And another use of references inside the loops.
void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
auto & line = box[rowOffset];
for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
}
}
}
void setBackgroundColor()
Similar comments regarding setBackgroundColor
.
void print()
Alternative lambda with std::stringstream
.
auto printer = [this]() {
std::stringstream output;
for (size_t row = 0; row < height; row++) {
for (size_t col = 0; col < width; col++) {
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output << "33[38;2;"
<< static_cast<int>(textColor.r) << ';'
<< static_cast<int>(textColor.g) << ';'
<< static_cast<int>(textColor.b) << "m"
"33[48;2;"
<< static_cast<int>(backgroundColor.r) << ';'
<< static_cast<int>(backgroundColor.g) << ';'
<< static_cast<int>(backgroundColor.b) << 'm'
<< getChar(col, row);
}
if (row != height - 1) {
output << 'n';
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output.rdbuf() << std::flush;
};
$endgroup$
add a comment |
$begingroup$
Framebuffer()
Could end with
charBuffer = std::vector<char>(height * width, ' ');
textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
backgroundColorBuffer = std::vector<Color>(height * width);
instead of calling clear.
void clear()
Alternative implementation and let container implementation decide what is most effective.
charBuffer.assign(charBuffer.size(), ' ');
textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});
void setChar()
Don't copy the box in the interface, use a const reference. And don't call size()
more than necessary.
void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor()
Again, don't copy the box on each call. And another use of references inside the loops.
void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
auto & line = box[rowOffset];
for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
}
}
}
void setBackgroundColor()
Similar comments regarding setBackgroundColor
.
void print()
Alternative lambda with std::stringstream
.
auto printer = [this]() {
std::stringstream output;
for (size_t row = 0; row < height; row++) {
for (size_t col = 0; col < width; col++) {
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output << "33[38;2;"
<< static_cast<int>(textColor.r) << ';'
<< static_cast<int>(textColor.g) << ';'
<< static_cast<int>(textColor.b) << "m"
"33[48;2;"
<< static_cast<int>(backgroundColor.r) << ';'
<< static_cast<int>(backgroundColor.g) << ';'
<< static_cast<int>(backgroundColor.b) << 'm'
<< getChar(col, row);
}
if (row != height - 1) {
output << 'n';
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output.rdbuf() << std::flush;
};
$endgroup$
Framebuffer()
Could end with
charBuffer = std::vector<char>(height * width, ' ');
textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
backgroundColorBuffer = std::vector<Color>(height * width);
instead of calling clear.
void clear()
Alternative implementation and let container implementation decide what is most effective.
charBuffer.assign(charBuffer.size(), ' ');
textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});
void setChar()
Don't copy the box in the interface, use a const reference. And don't call size()
more than necessary.
void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor()
Again, don't copy the box on each call. And another use of references inside the loops.
void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
auto & line = box[rowOffset];
for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
}
}
}
void setBackgroundColor()
Similar comments regarding setBackgroundColor
.
void print()
Alternative lambda with std::stringstream
.
auto printer = [this]() {
std::stringstream output;
for (size_t row = 0; row < height; row++) {
for (size_t col = 0; col < width; col++) {
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output << "33[38;2;"
<< static_cast<int>(textColor.r) << ';'
<< static_cast<int>(textColor.g) << ';'
<< static_cast<int>(textColor.b) << "m"
"33[48;2;"
<< static_cast<int>(backgroundColor.r) << ';'
<< static_cast<int>(backgroundColor.g) << ';'
<< static_cast<int>(backgroundColor.b) << 'm'
<< getChar(col, row);
}
if (row != height - 1) {
output << 'n';
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output.rdbuf() << std::flush;
};
answered 19 hours ago
Bo RBo R
1611
1611
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215519%2fcolor-ascii-drawing-class%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago
$begingroup$
In
setBackgroundColor
, takestd::vector<std::vector<Color>>
by const-ref instead since you don't modify the object. The copy is unnecessary.$endgroup$
– Juho
15 hours ago