Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color channels swapped and unpremultiplied #73

Closed
fdwr opened this issue Feb 26, 2022 · 4 comments
Closed

Color channels swapped and unpremultiplied #73

fdwr opened this issue Feb 26, 2022 · 4 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested

Comments

@fdwr
Copy link
Contributor

fdwr commented Feb 26, 2022

Sammy, your library was very easy to integrate 👍, building and displaying something in less than 20 minutes.

image

Though at first, it didn't look so good 😅, when attempting to draw icons with transparency as I got swapped color channels and ugly fringes around the borders back from lunasvg::Document::render() 😥.

image

So I wrote a function to swap the returned channels to BGRA order (and by BGRA, I mean logical memory order, where byte 0=blue, 1=green, 2=red, and 3=alpha), which is compatible with many (if not more?..) graphics APIs, including Windows GDI SetDIBitsToDevice / CreateDIBSection and Direct2D DrawBitmap and Cairo CAIRO_FORMAT_ARGB32. I also premultiplied the colors to be directly compatible with most rendering API's, but after stepping through the code, I observed that lunasvg was actually undoing what plutovg was already appropriately returning in the first place 🙃. So I just commented out that one line, and then everything worked beautifully 🎨🖼 (and I could delete my helper function too).

image

void Document::render(Bitmap bitmap, const Matrix& matrix, std::uint32_t backgroundColor) const
{
    RenderState state(nullptr, RenderMode::Display);
    state.canvas = Canvas::create(bitmap.data(), bitmap.width(), bitmap.height(), bitmap.stride());
    state.transform = Transform(matrix.a, matrix.b, matrix.c, matrix.d, matrix.e, matrix.f);
    state.canvas->clear(backgroundColor);
    root->render(state);
    // LunaSvg is unpremultiplying all the BGRA values /:-/, which makes them
    // useless for callers to composite icons against a background.
    // Additionally it's swapping all the color channels, which leads to blue and red
    // being backwards when trying to draw them to GDI SetDIBitsToDevice and D2D DrawBitmap.
    // So disable this line and return the true original pixel values.
    // state.canvas->rgba();
}

Shall we add a bool parameter to avoid unpremultiplying the values? Otherwise we end up unpremultiplying them only to re-premultiply them again in common rendering scenarios - lots of extra divisions. Obviously straight alpha would still need to be an option too for some other libraries, like libpng which only accepts unassociated alpha, but I feel like straight alpha is less and less useful over the decades in the general case. Thanks from Seattle.

@sammycage sammycage added documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested labels Feb 27, 2022
@sammycage
Copy link
Owner

sammycage commented Feb 27, 2022

Added : 0d3e6a2

You can now convert bitmap to any format after rendering.

Example :

auto document = Document::loadFromFile("hello.svg");

Bitmap bitmap(48, 48);
bitmap.clear(0x00FF00FF); // Green background
document->render(bitmap);

bitmap.convert(0, 1, 2, 3, true); // convert To RGBA unpremulitied
bitmap.convert(0, 1, 2, 3, false); // convert To RGBA premultiplied

bitmap.convert(3, 2, 1, 0, true); // convert To ABGR unpremultiplied
bitmap.convert(3, 2, 1, 0, false); // convert To ABGR premultiplied

bitmap.convert(2, 1, 0, 3, true); // convert To BGRA unpremultiplied
bitmap.convert(2, 1, 0, 3, false); // convert To BGRA premultiplied

@fdwr
Copy link
Contributor Author

fdwr commented Feb 28, 2022

Great - nice approach too. Thanks.

So in my case, to avoid extra work, I simply don't call convert :b.

@fdwr
Copy link
Contributor Author

fdwr commented Mar 28, 2022

@sammycage : Btw, I published my little sample app now: https://github.com/fdwr/LunaSvgSampleTest

@sammycage
Copy link
Owner

@fdwr Interesting 👍👍👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants