phoboslab / qoi

The “Quite OK Image Format” for fast, lossless image compression
MIT License
6.96k stars 330 forks source link

Seeking code critique of QOI implementation #153

Closed n8bot closed 2 years ago

n8bot commented 2 years ago

[Edited to update link with single commit showing my final implementation. Critiques welcomed from anyone. Thank you!]

Hellow. I've been asked to implement QOI into an app. I'm kind of a newb, though. It seems to work, but I was wondering if you could tell me if I'm doing something stupid?

https://github.com/n8bot/PrusaSlicer/commit/7ba27a29a9f96a407a18a8a5043884dbb6bd6599

Thank you!

n8bot commented 2 years ago

I've been tinkering with it. I have finished implementing it, but the images are output upside down!?

https://github.com/n8bot/PrusaSlicer/compare/n8-240r...n8bot:n8-240-qoitest

image

n8bot commented 2 years ago

Sorry, I think the upside down image is because the app is providing pixels in reverse order. Let me try...

n8bot commented 2 years ago

Welp. The app refuses to let me flip the pixels using std::reverse. But this has nothing to do with QOI!

But, perhaps it would be nice to have the feature to flip the input pixels on encode? The PNG encoding function in the app has this feature for some reason.

But, other than that, I'd appreciate if you critique my implementation of QOI. I'm very new to software development.

Thank you! And I'll now discontinue flooding you with messages.

[Edit: I fixed the flipped image! Thanks GitHub copilot for reversing the vector while preserving the subpixel order! my brain couldn't work around how to do that math. Took copilot like 20 seconds, with the right comment.)

// Take vector of RGBA pixels and flip the image vertically
    std::vector<uint8_t> rgba_pixels(data.pixels.size() * 4);
    for (size_t y = 0; y < data.height; ++y)
        for (size_t x = 0; x < data.width; ++x)
            for (size_t c = 0; c < 4; ++c)
                rgba_pixels[(data.height - y - 1) * data.width * 4 + x * 4 + c] = data.pixels[y * data.width * 4 + x * 4 + c];
Kristine1975 commented 2 years ago

You will probably reach a bigger audience if you post your code here: https://codereview.stackexchange.com

n8bot commented 2 years ago

You will probably reach a bigger audience if you post your code here: https://codereview.stackexchange.com

Thank you. I will try that.