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

JmeSystemDelegate#writeImageFile format conversion #1295

Closed
MeFisto94 opened this issue Feb 16, 2020 · 2 comments
Closed

JmeSystemDelegate#writeImageFile format conversion #1295

MeFisto94 opened this issue Feb 16, 2020 · 2 comments
Labels
bug Something that is supposed to work, but doesn't. More severe than a "defect". question

Comments

@MeFisto94
Copy link
Member

MeFisto94 commented Feb 16, 2020

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:

  • Documentation should clearly state that the bytebuffer is assumed to be in TYPE_INT_BGR (or only accept IntBuffers for that matter, then it's clear that you can't pass TYPE_3BYTE_BGR)
  • We should make writeImageFile independant of the type, you just pass it, so that screenshots could run their conversion before calling writeImageFile
  • Screenshots could read the FrameBuffer like I do: using BGR8. That way we could get rid of all the magic.

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

@MeFisto94 MeFisto94 added bug Something that is supposed to work, but doesn't. More severe than a "defect". question labels Feb 16, 2020
@Ali-RS
Copy link
Member

Ali-RS commented Sep 11, 2022

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
Copy link
Member

Ali-RS commented Sep 11, 2022

Please see this fix: #1851

@Ali-RS Ali-RS closed this as completed Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to work, but doesn't. More severe than a "defect". question
Projects
None yet
Development

No branches or pull requests

2 participants