jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.78k stars 1.12k forks source link

JmeSystemDelegate#writeImageFile format conversion #1295

Closed MeFisto94 closed 1 year ago

MeFisto94 commented 4 years ago

So I was quite surprised to see that writeImageFile doesn't just do that: Writing a byte[] as image to disk but instead attempts some conversation through Screenshots#convertScreenShot2.

In fact, this file assumes that the format is "BGRX8"/INT_BGR and not BGR8 as I was passing, but there is no comment and also no way to override the behavior. Even worse: convertScreenShot2 is a no-op, just copying the data from a buffer into an AWT object.

Actual issue:

java.nio.BufferUnderflowException
    at java.nio.IntBuffer.get(IntBuffer.java:688)
    at java.nio.IntBuffer.get(IntBuffer.java:715)
    at com.jme3.util.Screenshots.convertScreenShot2(Screenshots.java:50)
    at com.jme3.system.JmeDesktopSystem.writeImageFile(JmeDesktopSystem.java:94)
    at com.jme3.system.JmeSystem.writeImageFile(JmeSystem.java:134)

This happens because I pass a ByteBuffer in BGR8 format. They get converted into an IntBuffer, i.e. 4 components, where the last one is disregarded i.e. useless (see BufferedImage#TYPE_INT_BGR).

This means, my byte buffer misses 1 byte per pixel information, even though this byte is thrown away. I think:

Even more: Reading the written png file definitely happens in TYPE_3BYTE_BGR format, so we require the user to write the files in a differrent format than they are read in.

In my usecase I can get around the issue by calling AWT APIs directly, since I don't need cross platform support, alternatively one could construct a larger buffer and interleave 0x00s

Ali-RS commented 1 year ago

We should make writeImageFile independant of the type, you just pass it, so that screenshots could run their conversion before calling writeImageFile

Hmm, but shouldn't it know the type to be able to convert it to AWT image? I mean if the caller is supposed to do the conversion then it won't be cross-platform (JmeSyste.writeImageFile() is supposed to be cross-platform). Maybe we should pass it an Image instance instead of a raw ByteBuffer then we can use ImageToAwt on JmeDesktopSystem to convert it to a BufferedImage for exporting via ImageIO.

What do you think?

Ali-RS commented 1 year ago

Please see this fix: https://github.com/jMonkeyEngine/jmonkeyengine/pull/1851