Endianness problems?

[th-otto]

There seem to be some endianness problems. For example

plutovg/source/plutovg-surface.c

Line 47 in 3368ca4

static plutovg_surface_t* plutovg_surface_load_from_image(stbi_uc* image, int width, int height)

takes the image data from the reader (where atleast for png, the channels are stored as R,G,B,A in that order in memory), but https://github.com/sammycage/plutovg/blob/3368ca48b87652c67c3359a30b2125cb81092eba/source/plutovg-surface.c#L271C5-L271C11 accesses that data as uint32_t, and that will get the alpha byte into the bits 24-31 only on little-endian machines.

Also, when trying to render

it renders as

On a big-endian machine.

[sammycage]

Thank you for bringing this up!

I am aware of the potential endianness issues you're mentioning. The challenge I’ve faced is finding a reliable and standard way to handle endianness in a cross-platform manner.

If you know of a robust approach or have any suggestions, I would greatly appreciate your input!

[th-otto]

Thanks for your quick response.

To detect endianness, you can simply do something like

#include <endian.h>
#if BYTE_ORDER == BIG_ENDIAN
...
#endif

Or, if <endian.h> is not available, simply use the builtin definitions of gcc/clang

#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
...
#endif

To swap words, you can use something like

uint32_t swap32(uint32_t l)
{
	return ((l >> 24) & 0xffL) | ((l << 8) & 0xff0000L) | ((l >> 8) & 0xff00L) | ((l << 24) & 0xff000000UL);
}

uint16_t swap16(uint16_t w)
{
	return (w >> 8) | (w << 8);
}

Note that you don't need to worry about any specialized, architecture-dependent instructions to optimize these. The compiler already has patterns to detect these, and eg. on x86 will use bswap

Now for the example above, i still have to figure out where that happens. It is rendered from SVG source, so is not caused by the file loaders. I guess, somewhere the wrong byte of the pixel is used as alpha value. Any idea where to look?

[sammycage]

Thank you for the thoughtful explanation!

Now for the example above, i still have to figure out where that happens. It is rendered from SVG source, so is not caused by the file loaders. I guess, somewhere the wrong byte of the pixel is used as alpha value. Any idea where to look?

As you pointed out earlier, the issue might indeed arise from converting RGBA to the native format and vice versa. To avoid such issues, I recommend skipping the current PNG/JPG write functions for now. Instead, after rendering, you could try converting the plutovg_surface_t pixel data manually and then use a library like stb_image to write it to PNG.

By the way, which SVG library are you using? I'd like to understand more about the setup and see how it might relate to this behavior.

[th-otto]

The screenshot above is already from directly displaying the surface data, without writing out any file. I think that code is correct, if it would display the data in wrong order, i would get totally wrong colors. But currently there only seems to be an issue with blending the "knobs" at 3/6/9/12.

I tried both plutosvg and LunaSVG, and get the same results.

Basically, the code using plutosvg looks like:

char *data;
(read svg document into data)
plutosvg_document_t *document = plutosvg_document_load_from_data(data, file_size, -1, -1, 0, NULL);
int width = plutosvg_document_get_width(document);
int height = plutosvg_document_get_height(document);
size_t image_size = (size_t)width * height * 4;
uint8_t *bmap = (uint8_t *)malloc(image_size);
plutovg_surface_t *surface = plutovg_surface_create_for_data(bmap, width, height, (size_t)width * 4);
plutovg_canvas_t *canvas = plutovg_canvas_create(surface);
plutovg_color_t bg;
plutovg_color_init_rgba32(&bg, 0xffffffff);
plutovg_surface_clear(surface, &bg);
plutosvg_document_render(document, NULL, canvas, NULL, 0, NULL);

The reason to do all the steps manually instead of just using plutosvg_document_render_to_surface is because the drawing code just needs the raw image data, and does not know about plutovg_surface_t.

Also note the call to plutovg_surface_clear(). This is because the window system (Atari) actually cannot handle alpha, and this should create a white background.

Code using LunaSVG looks similar:

char *data;
(read svg document into data)
auto svg_image = Document::loadFromData(data, file_size);
int width = svg_image->width();
int height = svg_image->height();
size_t image_size = (size_t)width * height * 4;
uint8_t *bmap = (uint8_t *)malloc(image_size);
float xScale = 1.0f;
float yScale = 1.0f;
Matrix matrix(xScale, 0, 0, yScale, 0, 0);
Bitmap bitmap(bmap, width, height, (size_t)width * 4);
bitmap.clear(0xffffffff);
svg_image->render(bitmap, matrix);

[sammycage]

After further investigation, I discovered that the issue originates from converting RGBA image data from the stb image loader to plutovg_surface on a big-endian machine (this conversion works perfectly fine on little-endian systems). The clock.svg file uses a PNG image as a shadow for the clock knobs, and this appears to be the root cause of the issue.

To test if this resolves the problem, you can remove the embedded image by modifying the SVG processing code as follows:

char *data;  
(read svg document into data)  
auto svg_image = Document::loadFromData(data, file_size);  

svg_image->applyStyleSheet("image { display: none }"); // Hides embedded images  
svg_image->updateLayout(); // Updates the layout after applying the style  

int width = svg_image->width();  
int height = svg_image->height();  
size_t image_size = (size_t)width * height * 4;  
uint8_t *bmap = (uint8_t *)malloc(image_size);  
float xScale = 1.0f;  
float yScale = 1.0f;  
Matrix matrix(xScale, 0, 0, yScale, 0, 0);  
Bitmap bitmap(bmap, width, height, (size_t)width * 4);  
bitmap.clear(0xffffffff);  
svg_image->render(bitmap, matrix);  

This code ensures that the embedded PNG image is ignored during rendering, which might help narrow down the problem.

Let me know if the above changes fix the problem or if there’s something else you'd like me to look into for further debugging.

[th-otto]

Yes, thanks, that seems to work:

I had to rebuild the library from current snapshot, because that applyStyleSheet function is not available in the v3.0.1 release.

Is something similar possible using plutosvg? I would prefer using that, since the library is only used by a plugin, and the C++ code currently requires me to link a lot of libstdc++ in (on Atari there are no really shared libs).

OTOH, plutosvg has some other limitations it seems, eg. the text is not rendered there.

[th-otto]

I've come up with the attached patch now. Still have to check whether it does the right for other images like jpeg (are those really used as embedded images in SVG documents?), but atleast it should not break anything on little-endian systems.

bigendian.patch.txt

[sammycage]

Thank you for the patch! I really appreciate your effort. I'll take a closer look and try to implement it when I have time.

[safocl]

I've come up with the attached patch now. Still have to check whether it does the right for other images like jpeg (are those really used as embedded images in SVG documents?), but atleast it should not break anything on little-endian systems.

bigendian.patch.txt

for me it's a bit of a strange correction -- from my point of view it should just be used as a byte array:

void plutovg_convert_rgba_to_argb(unsigned char* dst, const unsigned char* src, int width, int height, int stride)
{
    static const int pixel_num_bytes = 4;
    for(int y = 0; y < height; y++) {
        const unsigned char* src_row = src + stride * y;
        unsigned char*  dst_row = dst + stride * y;
        for(int x = 0; x < width; x+=pixel_num_bytes) {
            unsigned char pixel[] = {src_row[x], src_row[x+1], src_row[x+2], src_row[x+3]};
            unsigned char a = pixel[3];
            if(a == 0) {
                for(int k = 0; k < 4; ++k){
                    dst_row[x+k] = 0;
                }
            } else {
                unsigned char b = pixel[2];
                unsigned char g = pixel[1];
                unsigned char r = pixel[0];
                if(a != 255) {
                    r = (r * a) / 255;
                    g = (g * a) / 255;
                    b = (b * a) / 255;
                }
                
                dst_row[0] = a;
                dst_row[1] = r;
                dst_row[2] = g;
                dst_row[3] = b;
            }
        }
    }
}

[sammycage]

@safocl Thanks for your input and suggestions! The approach you shared makes sense, but do you think it would actually address the endianness issue on big-endian systems?