v icon indicating copy to clipboard operation
v copied to clipboard

Image loading with stbi+gfx not working properly (Throwing unhandled exception)

Open MightyPancake opened this issue 2 years ago • 12 comments

Describe the bug

Adding an image with stbi in the following (correct?) way is not working in this version. The issue is related to the size of the image/data given and expected to sokol.gfx. Here's the image I tried to load (although, I tried a few others; all of them didn't work) image in

Expected Behavior

The image should load correctly.

Current Behavior

The program crashes on this function call with a valid image path.

Reproduction Steps

fn new_img(path string) ?Image {
	data := stbi.load(path) or {return err}
	pixels := &u8(data.data)
	println("Image loaded!\nWidth: $data.width\nHeight: $data.height\nChannels: $data.nr_channels\nExtension: $data.ext")
	mut img_desc := gfx.ImageDesc{
		width: data.width,
		height: data.height,
		num_mipmaps: 0,
		min_filter: .linear,
		mag_filter: .linear,
		label: "image".str
	}
	img_desc.data.subimage[0][0] = gfx.Range{
		ptr: pixels
		size: usize(data.width*data.height*data.nr_channels)
	}
	//size: usize(data.width*data.height*data.nr_channels)
	image := gfx.make_image(img_desc)
	return Image(image)
}

Possible Solution

I noticed that in the gg implementation it's stated that the value for size should be: 4 * width * height, however, doing so makes crashes the program. This is possibly due to Sokol trying to read bytes that are out of range of the pointer (array) returned by stbi. A workaround for this might be just creating a more extensive array and pasting the content fetched from stbi while leaving the rest of the array zeroed. That's a waste of space, but it might work :)

Additional Information/Context

This code worked a couple of months ago (4?)

@spytheman On discord advised reverting my V locally to 6e24f7e, which fixed the issue, so something fishy probably happened after that commit.

V version

V 0.3.2 69f7c45

Environment details (OS name and version, etc.)

Windows 11

MightyPancake avatar Dec 14 '22 01:12 MightyPancake

I suggested git revert 6e24f7e which undoes specifically that commit, so I think the problem /difference in expectation between the user code and vlib is in it.

That commit corresponds to https://github.com/vlang/v/pull/16564

spytheman avatar Dec 14 '22 01:12 spytheman

Probably also related https://github.com/vlang/v/discussions/16754

We have situation were the amount of channels (nr_channels) multiplied by width * height returned by stbi works on some devices - and sometimes it doesn't...

larpon avatar Dec 30 '22 15:12 larpon

If I'm not mistaken, img_desc.data.subimage[0][0] = gfx.Range{...}, the data passed in the gfx.Range should match the current sokol context's pixel format - and for gg (which used sokol_gl.h under the hood) that's IIRC SG_PIXELFORMAT_RGBA8 (that's why I argue that we should keep the 4 * width * height code as I reintroduced in #16564) we could try to either zero the pixel data array for 4 * width * height as @MightyPancake suggests or maybe try to zero the whole image description struct as sokol_gl.h is also doing.

larpon avatar Dec 30 '22 16:12 larpon

More info: https://github.com/nothings/stb/issues/335#issuecomment-234749597.

If the stbi folks are correct the data returned from stbi_load* functions will have the 4th channel set if it's passed STBI_rgb_alpha as desired_channels (req_comp) - which is exactly what we do in all stbi related load functions in gg since #16564 - so using nr_channels when nr_channels <= 3 should not work, exactly as the flappylearning example didn't work on my phone... I'm confused, but still believe that #16564 is doing the correct (according to sokol/stbi documentation) - so something else must be tripping us.

Also note the glPixelStorei(GL_UNPACK_ALIGNMENT, 1); suggestion by user nothings, right after the comment I linked above. More on the alignment here: https://stackoverflow.com/a/11045278/1904615

larpon avatar Dec 30 '22 17:12 larpon

@MightyPancake - what pixel format does your graphics pipeline use (the pipeline in which context you're loading the image)? (It's hard for us to replicate the bug since we don't have the rest of your code)

larpon avatar Dec 30 '22 17:12 larpon

On a, somewhat related, side note the attached image works fine in the image viewer on my machine image

larpon avatar Dec 30 '22 18:12 larpon

btw, https://github.com/nothings/stb/blob/master/stb_image.h is already on v2.27, it may be a good idea to update our copy as well. Unlike some other thirdparty libraries, they are usually very careful to not make breaking API changes, so it should be relatively painless.

edit: done in https://github.com/vlang/v/pull/16825

spytheman avatar Dec 31 '22 16:12 spytheman

Is this still an issue with current V? Without a complete working example, I can't test...

JalonSolov avatar Mar 26 '23 14:03 JalonSolov

Reports have been in the few and very unclear / hard to reproduce since #16564 - I think we can close this. At least until we have something concrete, code to test against

larpon avatar Mar 26 '23 17:03 larpon