proposal: add `NoCopy` field to new `ImageOptions`

[tinne26]

With the addition of the new NewImageOptions struct we could also add a flag to try to avoid unnecessary copies during Ebitengine's image creation. It's very common that standard Golang image.Image images are only created to be passed to Ebitengine's image initialization, so allocating a new buffer for the data and copying it, when the format is already compatible (RGBA images), is wasteful.

ebitenutil.NewImageFrom* functions and well as many Ebitengine related libraries could often benefit from this.

The downsides are:

• More surface area for NewImageOptions. But I don't think this is out of place there.
• We may be worrying too much about optimization here. But I'd say seeking to eliminate redundant allocations is quite fair.
• Only works with image.RGBA, and this on itself also kinda exposes internal implementation details.
• Safety: if users continue using the image data slices anyway, they will get in trouble. To me this is the strongest argument against this proposal. I personally don't think anyone would have any right to complain if they misuse this feature, but it's true that Ebitengine typically goes out of its way to avoid such situations, and this kinda opens a door for undetected misuse (well, Golang doesn't really prevent developers from manually creating such doors anyway with raw pointers, but you know...).

[hajimehoshi]

Thanks. This seems to reduce unnecessary copying pixels, but sounds like an expose of implementation details. Let me think more...

[tinne26]

Fair point. Maybe I can benchmark after the game jam is over and see whether the performance implications are actually non-trivial with a synthetic asset loading test. If the impact is very minor it might be better to simply reject this, for a lack of a better solution. I'll also ask SolarLune to chime in, as he may (or may not) be interested in this for Tetra3D when loading textures.

[SolarLune]

Hello!

EDIT: Sorry for the edit, I just realized I do load textures in Tetra3D if the textures are embedded in a model / scene file that's loaded in. That being the case, Tetra could, indeed, make use of such a NoCopy flag (though the difference would probably be not that intense unless the textures are large and numerous, I suppose).

An alternative might be to make a NewImageFromRGBA() function, or something like that, specifically for this kind of use case where the image in the desired format already exists?

[tinne26]

Ughh... I wanted to benchmark, but this is really tricky. I understand we would have to add methods for direct buffer initialization through all the image chain, which is... ebiten.Image -> ui.Image -> mipmap.Mipmap -> buffered.Image -> atlas.Image -> atlas.backend -> restorable.Image -> and now I'm confused with basePixels Pixels vs image *graphicscommand.Image, and any of them still go multiple levels deeper (for Pixels, it is pixelRecords -> pixelRecord, and for graphicscommand.Image is graphicsdriver.Image -> I don't know if graphicsdriver.Graphics or graphicsdriver.framebuffer, and that for each graphical backend). I'm scared now.

EDIT: on the topic of NewImageFromRGBA(), I don't think it's a good option because the name doesn't make it clear that it's taking ownership of the underlying data slice, which makes it dangerous, and would otherwise seem redundant with NewImageFromImage(). But thanks for chiming in, I was just about to ping you in chat as I saw you there.

[hajimehoshi]

You need more documentation?

[tinne26]

Hmm, I'd like to understand the differences / jobs of Pixels vs graphicscommand.Image and graphicsdriver.Graphics vs graphicsdriver.framebuffer.

[hajimehoshi]

Pixels vs graphicscommand.Image

Which package of Pixels are you indicating? The latter is a struct that just enqueues commands to the command queue.

graphicsdriver.Graphics vs graphicsdriver.framebuffer.

The former is an interface for various graphics drivers like OpenGL, DirectX, and Metal. What is the latter?

[tinne26]

Oh, basePixels Pixels is from restorable.Image, so from what you said I understand that this is where the underlying []byte data for the image is stored. And for graphicsdriver.Graphics and graphicsdriver.framebuffer, sorry, I actually was referring to the opengl driver, but I guess drivers simply implement the graphicsdriver.Graphics and graphicsdriver.Image interfaces, so I'll figure that out myself.

[SolarLune]

EDIT: on the topic of NewImageFromRGBA(), I don't think it's a good option because the name doesn't make it clear that it's taking ownership of the underlying data slice, which makes it dangerous, and would otherwise seem redundant with NewImageFromImage(). But thanks for chiming in, I was just about to ping you in chat as I saw you there.

It could be named something else, then, like TakeImageFromRGBA() or something like that. No problem, in any case~ Glad to be able to chime in.