Image class wrongly set mips flags inside the constructor

[Ali-RS]

These two constructors on Image class behave differently!

jmonkeyengine/jme3-core/src/main/java/com/jme3/texture/Image.java

Lines 749 to 762 in f92a73f

	public Image(Format format, int width, int height, int depth, ArrayList<ByteBuffer> data,
	int[] mipMapSizes, ColorSpace colorSpace) {
	this();
	if (mipMapSizes != null) {
	if (mipMapSizes.length <= 1) {
	mipMapSizes = null;
	} else {
	needGeneratedMips = false;
	mipsWereGenerated = true;
	}
	}

jmonkeyengine/jme3-core/src/main/java/com/jme3/texture/Image.java

Lines 805 to 816 in f92a73f

	public Image(Format format, int width, int height, ByteBuffer data,
	int[] mipMapSizes, ColorSpace colorSpace) {
	this();
	if (mipMapSizes != null && mipMapSizes.length <= 1) {
	mipMapSizes = null;
	} else {
	needGeneratedMips = false;
	mipsWereGenerated = true;
	}

Look at needGeneratedMips and mipsWereGenerated flags, if mipMapSizes is null the first one won't modify the flags but the second one will do (I belive it is wrongly setting mipsWereGenerated = true)

Forum post:
https://hub.jmonkeyengine.org/t/issue-in-image-class-bug/46297

[zzuegg]

I wonder if needGeneratedMips should be set to true in the second constructor. I would kind of expect that if i only pass the wanted mip sizes they would be generated.

[zzuegg]

If i follow the code in GLRenderer correctly, mipsWhereGenerated is needed to set to true if the miplevels are generated on the gpu, (they can be trusted to be correct) if not, it makes some checks.

Following that, it should actually set mipsWereGenerated to false in the first constructor(Since they are passed manually), but needGeneratedMips to true in the second one (Since we actually want mips to be generated)

[Ali-RS]

Looked a bit deeper into this

The first constructor which takes ArrayList<ByteBuffer> data seems to be used for texture array and the second constructor which takes ByteBuffer data is for regular images.

if we pass an int[] mipMapSizes it means we have generated mips. All mips data must be packed inside the ByteBuffer data.

This is how mips are generated and stored for an image:

jmonkeyengine/jme3-core/src/main/java/com/jme3/util/MipMapGenerator.java

Lines 96 to 131 in ac8c101

	public static void generateMipMaps(Image image){
	int width = image.getWidth();
	int height = image.getHeight();
	Image current = image;
	ArrayList<ByteBuffer> output = new ArrayList<>();
	int totalSize = 0;
	while (height >= 1 || width >= 1){
	output.add(current.getData(0));
	totalSize += current.getData(0).capacity();
	if (height == 1 || width == 1) {
	break;
	}
	height /= 2;
	width /= 2;
	current = scaleImage(current, width, height);
	}
	ByteBuffer combinedData = BufferUtils.createByteBuffer(totalSize);
	int[] mipSizes = new int[output.size()];
	for (int i = 0; i < output.size(); i++){
	ByteBuffer data = output.get(i);
	data.clear();
	combinedData.put(data);
	mipSizes[i] = data.capacity();
	}
	combinedData.flip();
	// insert mip data into image
	image.setData(0, combinedData);
	image.setMipMapSizes(mipSizes);
	}

GLRenderer uses TextureUtil to upload a texture to GPU, there if an image has mips packed inside it it will be extracted and uploaded to GPU:

jmonkeyengine/jme3-core/src/main/java/com/jme3/renderer/opengl/TextureUtil.java

Lines 245 to 301 in 930809e

	public void uploadTexture(Image image,
	int target,
	int index,
	boolean linearizeSrgb) {
	boolean getSrgbFormat = image.getColorSpace() == ColorSpace.sRGB && linearizeSrgb;
	Image.Format jmeFormat = image.getFormat();
	GLImageFormat oglFormat = getImageFormatWithError(jmeFormat, getSrgbFormat);
	ByteBuffer data = null;
	int sliceCount = 1;
	if (index >= 0) {
	data = image.getData(index);
	}
	if (image.getData() != null && image.getData().size() > 0) {
	sliceCount = image.getData().size();
	}
	int width = image.getWidth();
	int height = image.getHeight();
	int depth = image.getDepth();
	int[] mipSizes = image.getMipMapSizes();
	int pos = 0;
	// TODO: Remove unnecessary allocation
	if (mipSizes == null) {
	if (data != null) {
	mipSizes = new int[]{data.capacity()};
	} else {
	mipSizes = new int[]{width * height * jmeFormat.getBitsPerPixel() / 8};
	}
	}
	int samples = image.getMultiSamples();
	// For OGL3 core: setup texture swizzle.
	if (oglFormat.swizzleRequired) {
	setupTextureSwizzle(target, jmeFormat);
	}
	for (int i = 0; i < mipSizes.length; i++) {
	int mipWidth = Math.max(1, width >> i);
	int mipHeight = Math.max(1, height >> i);
	int mipDepth = Math.max(1, depth >> i);
	if (data != null) {
	data.position(pos);
	data.limit(pos + mipSizes[i]);
	}
	uploadTextureLevel(oglFormat, target, i, index, sliceCount, mipWidth, mipHeight, mipDepth, samples, data);
	pos += mipSizes[i];
	}
	}

If we do not pass int[] mipMapSizes it means there is no mips packed inside image data.

I wonder if needGeneratedMips should be set to true in the second constructor. I would kind of expect that if i only pass the wanted mip sizes they would be generated.

So this is not true.