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

stb_image_write: support for 16 bit png #1726

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lyd405121
Copy link

Background

  • stb is my favorite picture saver and loader
  • But recently I found a small bug
  • stb_image_writer.h can not export 16 bit png with right image pitch(stride)
  • So I decided to do this fix

Why is this neccessary

  • In the area of computer graphics, single channel png is used for roughness , metal and height map
  • Especially for high precision height map is needed
  • And sometimes we want to produce an accurate depth map to training robot, 16 bit single channel png is also needed
1.mp4

Bug reproduce

  • I wrote a small test to demenstration it which is known as julia set pattern
#define STB_IMAGE_WRITE_IMPLEMENTATION
#include "stb_image_write.h" //the origin stb code
//#include "stb_image_write_my.h" // my fix

#define WID 1280
#define HGT 720

//https://www.shadertoy.com/view/wtBBWd
template<typename T>
void DrawJuliaSet(T* pImage, int nMax){
    float cx = -0.8f, cy = 0.2f;
    for (int i = 0; i < WID; i++) {
        for (int j = 0; j < HGT; j++) {
            float zx = float(i) / float(HGT) - 1.0f, zy = float(j) / float(HGT) - 0.5f;
            pImage[(i + j * WID)] = 0;
            while (sqrtf(zx * zx + zy * zy) < float(HGT) && pImage[(i + j * WID)] < nMax){
                float temp = zx * zx - zy * zy + cx;
                zy = 2.0f * zx * zy + cy;
                zx = temp;
                pImage[(i + j * WID)]++;
            }
        }
    }
}

int main()
{
    unsigned short* us_pixel= new unsigned short[WID * HGT];
    DrawJuliaSet(us_pixel, 65535);
    stbi_write_png("16bit.png", WID, HGT, 1, us_pixel, WID*sizeof(unsigned short));

    unsigned char* uc_pixel = new unsigned char[WID * HGT];
    DrawJuliaSet(uc_pixel, 255);
    stbi_write_png("8bit.png",  WID, HGT, 1,  uc_pixel, WID*sizeof(unsigned char));
    return 0;
}
  • When I use the origin code of stb, this is the 8bit result, works perfectly!

8bit

  • But when it turns to 16 bit, bug bites!
  • There is empty verticle line pass through the image, and whole picture is sterched

16bit

  • So I dive into the stb origin code and found two mistake
  • ① the png head for bit count is always 8
   STBIW_MEMMOVE(o,sig,8); o+= 8;
   stbiw__wp32(o, 13); // header length
   stbiw__wptag(o, "IHDR");
   stbiw__wp32(o, x);
   stbiw__wp32(o, y);
   *o++ = 8; //here is wrong
   *o++ = STBIW_UCHAR(ctype[n]);
   *o++ = 0;
   *o++ = 0;
   *o++ = 0;
   stbiw__wpcrc(&o,13);
  • ② using picture width*channel count instead of image stride
   filt = (unsigned char *) STBIW_MALLOC((x*n+1) * y); if (!filt) return 0; //better use stride_bytes instead of x*n
   line_buffer = (signed char *) STBIW_MALLOC(x * n); if (!line_buffer) { STBIW_FREE(filt); return 0; }
  • When I fix all this, I test again
  • 8 bit Julia set, is still perfect as before
  • And the 16 bit picture look exactly the same

16bit-my

More test

  • I also test 3 channel png with 16 bit
#define STB_IMAGE_WRITE_IMPLEMENTATION
//#include "stb_image_write.h"
#include "stb_image_write_my.h"

#define WID 256
#define HGT 256

template<typename T>
void DrawColor(T* pImage, int nMax) {
    for (int i = 0; i < WID; i++) {
        for (int j = 0; j < HGT; j++) {
            pImage[(i + j * WID) * 3 + 0] = T(float(i) / float(WID - 1) * float(nMax));
            pImage[(i + j * WID) * 3 + 1] = T(float(j) / float(HGT - 1) * float(nMax));
            pImage[(i + j * WID) * 3 + 2] = 0;
        }
    }
}

int main()
{
    unsigned short* us_pixel = new unsigned short[WID * HGT * 3];
    DrawColor(us_pixel, 65535);
    stbi_write_png("16bit-n3.png", WID, HGT, 3, us_pixel, WID * sizeof(unsigned short) * 3);

    unsigned char* uc_pixel = new unsigned char[WID * HGT * 3];
    DrawColor(uc_pixel, 255);
    stbi_write_png("8bit-n3.png", WID, HGT, 3, uc_pixel, WID * sizeof(unsigned char) * 3);
    return 0;
}
  • The 8-bit test produce the image with origin code:

8bit-n3

  • The 16-bit test produce the image below:

16bit-n3

  • The 8-bit test produce the image with my fix for stb:

8bit-n3-my

  • The 16-bit test produce the image with my fix for stb:

16bit-n3-my

For single channel 16 bit png
@lyd405121 lyd405121 changed the title [FIX] Fix for single channel 16 bit png writer [FIX] Fix for 16 bit png writer Dec 8, 2024
@lyd405121 lyd405121 changed the title [FIX] Fix for 16 bit png writer stb_image_witer: support for 16 bit png Dec 9, 2024
@lyd405121 lyd405121 changed the title stb_image_witer: support for 16 bit png stb_image_write: support for 16 bit png Dec 9, 2024
@lyd405121
Copy link
Author

What's left

  • This fix will not help the case if the picture is not a square and channel is more than one
  • For example the picture's width is 512, and the height is 256

16bit-n3my

Why not fix it

  • It needs to fix too much code in function "stbiw__encode_png_line"
  • The simplest way is to write a template function to handle with unsigned char and short
  • But I think it is only works for C++ compiler
  • So we should add stbiw__encode_png_line_16 like what's in stb_image.h do
  • And futhermore stbiw__paeth and STBIW_UCHAR should be rewritten or make a unsigned short version
  • I am too afraid to modify it , I have no confidence to make it right

This fix is still valuable

  • It will cover single channel with any width/height 16bit picture to export
  • It will cover multi-channel with width==height 16bit picture to export, and most of video games use square picture to improve cache-friendly code
  • Only few lines of code has been modified

@nothings
Copy link
Owner

nothings commented Dec 9, 2024

Releasing this wouldn't make any sense in practice because of the width=height restriction. It may be true that gamedev textures are this way, but (a) a more common use for this library is screenshots, which aren't square, and (b) more importantly it's too weird a restriction. End users won't read the documentation and notice the limitation and would report it as a bug, and we'd have to say it's not a bug, and they'd be annoyed. We either fully support 16-bit writing, or not at all.

@lyd405121
Copy link
Author

Releasing this wouldn't make any sense in practice because of the width=height restriction. It may be true that gamedev textures are this way, but (a) a more common use for this library is screenshots, which aren't square, and (b) more importantly it's too weird a restriction. End users won't read the documentation and notice the limitation and would report it as a bug, and we'd have to say it's not a bug, and they'd be annoyed. We either fully support 16-bit writing, or not at all.

  • Thanks for your reply
  • I am agree with you too, but It will take sometime
  • So if I can fix all size , channel ,bits(32bit too) of image,then it will make sense

Two solution

  • ① make a new entry function stbi_write_png_16 like stbi_load_16 in the saver of stb do
  • ② add new switch branch for 8/16/32 bit in the original function stbi_write_png,alloc unsigned char/short/int memory and postprocess as well.
  • For your opinion, which one is better?

@nothings
Copy link
Owner

nothings commented Dec 9, 2024

It should definitely be a separate function so it can be typesafe (take unsigned short *).

@lyd405121
Copy link
Author

Conclusion

  • After looking around my code,I found nothing wrong with it
  • I finally found maybe the app for displaying png not supporting 16 bit RGB picture is to blame

Prove

int main()
{
    char name[32];
    for (int i = 8; i < 1000; )
    {
        for (int j = 8; j < 1000;)
        {
            sprintf_s(name, "my_16bit_%dx%d.png", i,j);
            unsigned short* us_pixel = new unsigned short[i * j];
            DrawJuliaSet(us_pixel, 255,i,j);
            stbi_write_png(name, i, j, 1, us_pixel, i * sizeof(unsigned short));

            sprintf_s(name, "my_8bit_%dx%d.png", i, j);
            unsigned char* uc_pixel = new unsigned char[i * j];
            DrawJuliaSet(uc_pixel, 255, i, j);
            stbi_write_png(name, i, j, 1, uc_pixel, i * sizeof(unsigned char));

            sprintf_s(name, "my_16bit_n3_%dx%d.png", i, j);
            unsigned short* us_pixel3 = new unsigned short[i * j * 3];
            DrawColor(us_pixel3, 255, i, j);
            stbi_write_png(name, i, j, 3, us_pixel3, i * sizeof(unsigned short) * 3);

            sprintf_s(name, "my_8bit_n3_%dx%d.png", i, j);
            unsigned char* uc_pixel3 = new unsigned char[i * j * 3];
            DrawColor(uc_pixel3, 255, i, j);
            stbi_write_png(name, i, j, 3, uc_pixel3, i * sizeof(unsigned char) * 3);
            i = i + 15;
            j = j + 15;
        }
    }
    return 0;
}
  • So I make a huge test for all size of 16 bit RGB/Gray picture with color range in 0-255 instead of 0-65535
  • The result is all works perfectly without any change of my code
16bit-channel3.mp4
  • And I have tested many apps like gimp photoshop ifran-view
  • When eporxting images, there is no 16 bit option to choose
  • So I guess that when encounter 16-bit rgb picture, they will simply do an operation "&0xff" to get the tail of 16bit data

Why does they support

  • Maybe there is no monitor can support 65536×65536×65536 kinds of color
  • As far as I know, nowadays, 10 bit hdr monitor has most kinds of color support which is 1.07 billion

Overthrow my assumption

  • If there is a 3-channel 16bit picture which has a color range exceed 256
  • And it is support by ifran view or gimp,maybe I can explore the decode data of it

@nothings
Copy link
Owner

nothings commented Dec 9, 2024

Ok, that's good, but you can't change the API the way you have to infer the size of the pixels from the stride. Stride already has a different meaning.

The stride describes the number of bytes between successive rows of an image, sometimes called the pitch; it's not the length of a row. For example, if you have a 1024x1024 1-channel 1-byte image and you want to write out a 512x512 subregion of it, you call the writer with a pointer to the top left corner of the subregion, and you specify the stride as 1024, because that's the distance between rows of the 512x512 subimage in memory.

So you still need a separate interface for writing out 16-bit.

@nothings
Copy link
Owner

nothings commented Dec 9, 2024

Also, you can test the 16-bit output by using stb_image to read the 16-bit PNGs and verify they're the same pixels that you wrote out.

@lyd405121
Copy link
Author

Solid prove

  • I use stb to load the pitcture I export above
  • Here is my result

solid-prove

At last

  • So I should not touch the origin function as not many clients use 16 bit especially for screenshot
  • Just make a new one, is that right?

I have tested images file from 8*8 to 1280 *1280
@lyd405121
Copy link
Author

lyd405121 commented Dec 9, 2024

Seperate Function Done

  • I make a test from 8×8 to 1280×1280 size of picture
  • Looks all good
final-test.mp4

More info

  • I test 0-65535 range for 3 channel 16bit loader for stb

  • And found its low bit is swapped with high bit

  • But if I am using libpng,the value is ok
    libpng-vs-stb

  • When it turns to range 0-255,where every apps can display the 16bit 3channel picture normally

  • But stb still gets a swap

256

FYI

  • All of my test code is here
#define STB_IMAGE_WRITE_IMPLEMENTATION
#define STB_IMAGE_IMPLEMENTATION

#include "stb_image_write.h"
#include "stb_image.h"
#include "stdio.h"
#include "png.h"
#define PNG_BYTES_TO_CHECK 4     //libpng检查文件是否为png所需要BYTE数目

//https://www.shadertoy.com/view/wtBBWd
template<typename T>
void DrawJuliaSet(T* pImage, int nMax, int WID, int HGT){
    float cx = -0.8f, cy = 0.2f;
    for (int i = 0; i < WID; i++) {
        for (int j = 0; j < HGT; j++) {
            float zx = float(i) / float(HGT) - 1.0f, zy = float(j) / float(HGT) - 0.5f;
            pImage[(i + j * WID)] = 0;
            while (sqrtf(zx * zx + zy * zy) < float(HGT) && pImage[(i + j * WID)] < nMax){
                float temp = zx * zx - zy * zy + cx;
                zy = 2.0f * zx * zy + cy;
                zx = temp;
                pImage[(i + j * WID)]++;
            }
        }
    }
}

template<typename T>
void DrawColor(T* pImage, int nMax, int WID, int HGT) {
    for (int i = 0; i < HGT; i++) {
        for (int j = 0; j < WID; j++) {
            pImage[(j + i * WID) * 3 + 0] = T(float(j) / float(WID - 1) * float(nMax));
            pImage[(j + i * WID) * 3 + 1] = T(float(i) / float(HGT - 1) * float(nMax));
            pImage[(j + i * WID) * 3 + 2] = 0;
        }
    }
}

void ReadPng(const char* pFileName)
{

    //初始化各种结构
    png_structp png_ptr = { 0 };
    png_infop   info_ptr = { 0 };
    char        buf[PNG_BYTES_TO_CHECK] = { 0 };
    int         temp0 = 0;

    //创建png数据结构体
    png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, 0, 0, 0);
    //创建png文件信息结构体
    info_ptr = png_create_info_struct(png_ptr);
    setjmp(png_jmpbuf(png_ptr));

    //检测是否为png文件
    FILE* m_pFile = fopen(pFileName, "rb");
    if (NULL == m_pFile)
    {
        return;
    }

    temp0 = (int)fread(buf, 1, PNG_BYTES_TO_CHECK, m_pFile);   //读取四个字节判断是否为Png文件
    temp0 = png_sig_cmp((png_const_bytep)buf, (png_size_t)0, PNG_BYTES_TO_CHECK);
    if (temp0 != 0)
    {
        return;
    }

    //开始读文件
    rewind(m_pFile);                                              //重置文件指针
    png_init_io(png_ptr, m_pFile);                                //初始化png数据结构体
    png_read_png(png_ptr, info_ptr, PNG_TRANSFORM_EXPAND, 0);     //初始化png文件信息结构体

    //获取宽度,高度,位深,颜色类型
    int color_type;

    int nChannel = png_get_channels(png_ptr, info_ptr); //获取通道数
    color_type = png_get_color_type(png_ptr, info_ptr); //颜色类型


    int nPicWid = png_get_image_width(png_ptr, info_ptr);
    int nPicHgt = png_get_image_height(png_ptr, info_ptr);

    int nPicPitch = png_get_rowbytes(png_ptr, info_ptr);
    int nPicDepth = png_get_bit_depth(png_ptr, info_ptr) / 8;


    int nSize = static_cast<long long>(nPicPitch) * static_cast<long long>(nPicHgt);    // 计算图片的总像素点数量 

    png_bytepp row_pointers;// row_pointers里边就是rgba数据 
    row_pointers = png_get_rows(png_ptr, info_ptr);

    long long temp = 0;


    if (nChannel == 3 || color_type == PNG_COLOR_TYPE_RGB)//每个像素点占3个字节内存 
    {
        //由于png文件解析之后的数据按照RGB排列,所以这里直接赋值即可
        for (int i = 0; i < nPicHgt; i++)
        {
            for (int j = 0; j < nPicWid * 3; j = j + 3)
            {
                unsigned short* pData = (unsigned short*)(row_pointers[i] + j * nPicDepth); //存储解析后的数据
                printf("%d %d %d|", pData[0], pData[1], pData[2]);
            }
            printf("\n");
            printf("--------------next line-------------\n");
        }
    }

    //删除数据占用的内存
    png_destroy_read_struct(&png_ptr, &info_ptr, 0);
    fclose(m_pFile);
}

int main()
{
    char name[32];
    for (int i = 8; i <= 1280; )
    {
        for (int j = 8; j <= 1280;)
        {
            sprintf_s(name, "my_16bit_%dx%d.png", i,j);
            unsigned short* us_pixel = new unsigned short[i * j];
            DrawJuliaSet(us_pixel, 65535,i,j);
            stbi_write_png_16(name, i, j, 1, us_pixel, i * sizeof(unsigned short));

            sprintf_s(name, "my_8bit_%dx%d.png", i, j);
            unsigned char* uc_pixel = new unsigned char[i * j];
            DrawJuliaSet(uc_pixel, 255, i, j);
            stbi_write_png(name, i, j, 1, uc_pixel, i * sizeof(unsigned char));

            sprintf_s(name, "my_16bit_n3_%dx%d.png", i, j);
            unsigned short* us_pixel3 = new unsigned short[i * j * 3];
            DrawColor(us_pixel3, 255, i, j);
            stbi_write_png_16(name, i, j, 3, us_pixel3, i * sizeof(unsigned short) * 3);

            sprintf_s(name, "my_8bit_n3_%dx%d.png", i, j);
            unsigned char* uc_pixel3 = new unsigned char[i * j * 3];
            DrawColor(uc_pixel3, 255, i, j);
            stbi_write_png(name, i, j, 3, uc_pixel3, i * sizeof(unsigned char) * 3);
            i = i + 15;
            j = j + 15;
            printf("--------------%dX%d Done-------------\n", i,j);
        }
    }
    
    printf("****************test for stb*********************\n");
    int w, h, n;
    auto pData = stbi_load_16("my_16bit_n3_8x8.png", &w, &h, &n, 3);
    for (int i = 0; i < h; i++)
    {
        for (int j = 0; j < w; j++)
        {
            printf("%d %d %d|", pData[3 * (j + i * w) + 0], pData[3 * (j + i * w) + 1], pData[3 * (j + i * w) + 2]);
        }
        printf("\n");
        printf("--------------next line-------------\n");
    }

    printf("****************test for libpng*********************\n");
    ReadPng("my_16bit_n3_8x8.png");
    return 0;
}
  • Here is a 16bit channel3 1223x1223 png file

my_16bit_n3_1223x1223

  • You can download it from browser and preview it in any apps

files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants