Fix GLES3 packed sRGB texture transfers

[toaster0123]

Summary

GLES3 accepts only UNSIGNED_BYTE transfers for SRGB8 and SRGB8_ALPHA8, but RGB565 and RGB5A1 sRGB images were registered with packed unsigned-short types. Uploading those combinations produces GL_INVALID_OPERATION.

• Register legal GLES3 sRGB transfer types and expand packed pixels into RGB/RGBA byte buffers, preserving encoded channel values and alpha
• Interpret packed words in native byte order, matching OpenGL client-memory semantics regardless of Java buffer byte-order metadata
• Convert only the requested pixels for each mip/slice, leaving source data, buffer state, image format, color space, and mip-size metadata unchanged
• Use the destination color space for renderer subimage updates, including mixed-color-space copies; keep desktop and linear-destination packed transfers unchanged
• Match RGB5A1 sRGB renderability to its actual SRGB8_ALPHA8 storage

The transfer restrictions are specified in OpenGL ES 3.0.6 Table 3.2 and §3.8.3. Subimage transfers are checked against the existing destination storage (§3.8.5). This preserves sRGB storage without a silent linear fallback or CPU gamma conversion.

Validation

• Added 19 recording-GL tests, including all 65,536 packed values for each format, explicit encoded midtones and alpha, both byte-order metadata choices, read-only buffers, source state/mark preservation, mip levels, volumes, array slices, cubemap faces, null allocation, both subimage APIs, cropped row strides, mixed color spaces, and desktop/linear/fallback controls
• Unmodified upstream baseline: 16 regression failures and 3 passing controls; fixed branch: all 19 pass
• Full jme3-core:test: 502 cases, zero failures, one skipped
• Full jme3-effects:test: 6 cases, zero failures
• Renderer Checkstyle tasks passed, with zero warnings on added lines; existing warnings remain elsewhere
• No real-GPU validation or performance measurements

This branch starts directly from e8cf975be on upstream master and contains only this fix and its tests.

[jaime-jmebot]

One thing I want to double-check here: this method used to end with data.position(cpos);, so the caller's Image buffer was handed back at the position it came in with. With that line gone, even the plain non-sRGB paths (desktop / linear destination, where nothing is converted) now leave the source buffer already consumed — GLRenderer.modifyTexture passes in a buffer the caller still holds.

Does the new packed-sRGB helper restore the position, or should we keep the restore at the end of this method so the buffer state contract stays as it was for every caller?

[jaime-jmebot]

Nullability heads-up for the new argument: Image.getColorSpace() can legitimately return null. Image.read() restores it with a null default (colorSpace = capsule.readEnum("colorSpace", ColorSpace.class, null);), and ColorSpace only has sRGB/Linear.

If the packed-transfer check does anything other than destColorSpace == ColorSpace.sRGB, an image loaded from a .j3f that never wrote a color space would throw an NPE mid-upload. Can you make that comparison null-safe (a null/unknown color space then just means "don't convert")?

[jaime-jmebot]

Nice, well-scoped fix — expanding the packed pixels on the CPU keeps the SRGB8_ALPHA8 storage instead of silently falling back to linear, and the byte-order handling looks right.

A few things to look at (details inline):

• The data.position(cpos) restore at the end of uploadSubTexture is gone; worth confirming the caller's buffer still comes back where it started on the non-sRGB paths.
• The new destination ColorSpace argument should be null-safe — Image.read() can restore it as null.
• Sanity-check memory on mobile: expanding a full mip level allocates a fresh byte buffer per level/slice. A reused scratch buffer (or converting row-by-row) would keep peak memory down for large textures.

The test coverage looks great — full 65,536-value sweeps per format, both byte-order metadata settings, cropped strides, volumes/array/cubemap slices and the desktop/linear controls are exactly the cases that are easy to regress.

[toaster0123]

Additional offscreen pixel validation of 48ccd63 against baseline e8cf975:

The actual pinned Java GLRenderer/TextureUtil classes ran through a test-only JNA bridge to surfaceless EGL and Mesa 25.0.7 llvmpipe (LLVM 19.1.7, OpenGL ES 3.2). A fixture shader sampled the textures into a linear RGBA8 framebuffer for readback.

• Baseline RGB565/RGB5A1 2D and cube uploads, and both renderer subimage APIs, reproduce GL_INVALID_OPERATION
• All eight fixed regression cases and two linear controls pass without GL errors: 44 pixel readbacks, RGB within one byte of the expected value and alpha exact
• Driver queries confirm SRGB8/SRGB8_ALPHA8 storage. Cases include Linear-source to sRGB-destination copies, cropped source data, null allocation and opposite ByteBuffer byte-order metadata
• Baseline subimage failures were isolated with legally seeded destination storage; the portable reproduction repeated the same before/after outcomes

This is software-driver upload/sampling validation. Production LWJGL/Android integration, hardware GPUs and a forced ES3.0-only context remain untested here. Mip, array and volume paths remain covered by the separate recording-GL tests. No performance or CI result is claimed.

[toaster0123]

Checked all three points and pushed b2d7a5b:

• Caller buffer state was already preserved: uploadSubTexture duplicates the source before changing its position/limit. The removed restore only became redundant. Added tests for desktop and linear paths with read-only sources, preserved marks, and GL stubs that consume the buffer or throw.
• The destination comparison was already null-safe (destinationColorSpace == ColorSpace.sRGB). Added renderer-level null source/destination color-space cases through both modifyTexture overloads; an unknown destination keeps the packed linear transfer.
• Conversion now reuses one private, capacity-based scratch buffer across levels, faces, slices and subimage calls. Growth releases the old owned allocation before replacement, and renderer cleanup releases it idempotently. Tests cover exact upload limits, reuse after a consumed/cropped transfer, growth and cleanup/re-upload. An isolated counting allocator also verifies allocation/release ordering and that source buffers are never released.

The scratch retains the largest requested expansion until cleanup; cropped updates still expand the full source image. Release uses the existing BufferUtils allocator. The default ReflectionAllocator cannot explicitly free on this modular JVM, so this is not a hard native-memory bound or a mobile memory/performance measurement.

Validation: all 24 focused tests, full core tests (506 passed, one skipped), six effects tests and renderer Checkstyle pass. The same 10-case Mesa llvmpipe ES3.2/JNA smoke was rerun against these renderer sources: all 44 readbacks pass with the previously documented RGB tolerance and exact alpha.

[riccardobl]

In this PR the scratch buffer grows and never shrinks until cleanup. Cropped subimages uploads also counts the entire source image instead of the region, wasting memory. I think you should allocate on first upload a scratch buffer with a reasonable size (eg. 32 mb) and keep it forever. When it needs a bigger scratch buffer you should allocate a temporary one on-demand and destroy it after use.

[toaster0123]

@riccardobl Addressed in 9d541c8.

The conversion scratch is now allocated lazily at a fixed 32 MiB and retained until renderer cleanup. Larger uploads use an exact-size temporary, released through BufferUtils in finally on success or failure. Cropped subimages now expand only the requested rows and pixels into one contiguous upload, with scratch sizing based on the crop.

All 31 focused tests pass, including boundary sizes, crop-only conversion and exception cleanup. Broader local checks: 512 core tests passed, one existing skip; the network-dependent locator test was excluded after DNS failures. All six effects tests pass, and renderer Checkstyle reports no errors or new warnings.

Release still depends on the configured BufferUtils allocator; the default ReflectionAllocator on this modular JVM cannot guarantee immediate native deallocation. No GPU or performance validation was rerun for this change.