[mondogo24]

At issue #2719, the idea of deprecating CursorLoader was raised. Because CursorLoader is not crossplatform and it support cursor file formats that are no longer commonly used. CursorLoader has also been taken out of the Desktop.cfg file.

Since Desktop.cfg became empty (only including the General.cfg file), Desktop.cfg has been deleted and all the renferences to this file has been changed to point to the General.cfg file. In case it is not a desired change, I have separated the CursorLoader deprecation from the Desktop.cfg file deletion, so it can be easily reverted.

CLOSE #2719

EDIT:
CursorLoader will be deleted from Desktop.cfg. Desktop.cfg won't be dropped.

[gemini-code-assist]

Code Review

This pull request migrates the asset configuration from 'Desktop.cfg' to 'General.cfg' across various test files and system classes. It also deprecates the 'CursorLoader' class due to its lack of cross-platform support and the obsolescence of its supported formats. Feedback was provided to improve the grammar and clarity of the Javadoc in 'CursorLoader.java' through a code suggestion.

[riccardobl]

Let's keep Desktop.cfg even if currently empty 👍 It serves as a good entry point to add desktop only configs in the future

[mondogo24]

Let's keep Desktop.cfg even if currently empty 👍 It serves as a good entry point to add desktop only configs in the future

That was a really fast reply, I didn't have the time to get up from my chair XD. Thanks for the review! I have reverted the Desktop.cfg deletion commit.

[riccardobl]

Thanks. Let's leave this PR open for a while and see if anyone objects.

On another note, did you check whether anything is still using this loader, maybe in the examples?

[mondogo24]

On another note, did you check whether anything is still using this loader, maybe in the examples?

Ups, I did a second round check and just found this test from examples, sorry:

jmonkeyengine/jme3-examples/src/main/java/jme3test/gui/TestCursor.java

Lines 52 to 54 in aefe1c5

	cursors.add((JmeCursor) assetManager.loadAsset("Textures/Cursors/meme.cur"));
	cursors.add((JmeCursor) assetManager.loadAsset("Textures/Cursors/nyancat.ico"));
	cursors.add((JmeCursor) assetManager.loadAsset("Textures/Cursors/monkey.ani"));

It will fail since I deleted CursorLoader. I not sure what to do, I could deprecate and add CursorLoader back to Desktop.cfg, only deprecate and ignore future test fails from it or delete the test. What do you think is best @riccardobl ?

On the other hand. I checked other references to the CursorLoader class and Desktop.cfg file that could affect but nothing that looks like related to the CursorLoader:

So apart of the CursorTest I think I am not missing anything more, but I will keep an eye for if I find something else while we leave the PR open.

[riccardobl]

Mhh, I think we should add an helper to JmeCursor to convert generic textures to cursors

eg.

Texture2D f1 = assetLoader.loadTexture(...);
Texture2D f2 = assetLoader.loadTexture(...);
//...
JmeCursor cursor = JmeCursor.fromTexture(f1,f2...);

and store the cursors as normal pngs in the assets.

GPT 5.5 says this regarding the format:

GLFW wants cursor pixels as 32-bit non-premultiplied RGBA, rows from the top-left.

So imagesData seems to be just the cursor RGBA8 images in sequence. It should be easy to build using JME’s ImageRaster to read the logical pixels from any supported JME Image.
It doesn’t need to be fast, since cursors are loaded rarely during an app’s lifecycle, so I would favor simplicity here.

[mondogo24]

Since JmeCursor serve as a Data Structure, I thought about keeping that class simple and better create a converter. CursorConverter basically takes a Texture2D and gets a JmeCursor from it (as previously suggested) using a deep copy of the Image.

I didn't saw that Image and Texture2D can manage image animations, so the main drawback would be that CursorConverter itself can't handle animated files like the monkey.png and only will show the first frame.

I don't know how often animated cursors are used, but one solution might be just using multiple Texture2D, each one including a single frame, and constantly changing the cursor.

[riccardobl]

Thanks! Separating frames to multiple images (or an atlas?) is an acceptable solution.
But since JmeCursor already supports multiple frames concatenated into imagesData, I think we should make the converter accept an array of Texture2D in fromTexture and make it concatenate its data into imagesData (with a check to make sure all the buffers are the same sizes)

[mondogo24]

I have added the support for cursor animations in the CursorConverter. The user should be able to choose the frames (as long as each frame has the same pixel size) and custom delay for each one (or just use the same delay for every frame) for his animated cursor. This also opens the door for customs animated cursors without depending on an specific file format.

[Copilot]

Pull request overview

This PR addresses issue #2719 by deprecating the desktop/AWT-based CursorLoader (which supports the increasingly obscure .ico, .cur, and .ani formats) and introducing a cross-platform replacement, CursorConverter, that builds a JmeCursor from one or more Texture2D objects. The CursorLoader registration is removed from Desktop.cfg, and the TestCursor example is migrated to PNG assets driven through CursorConverter.

Changes:

• Deprecate CursorLoader and add a class-level Javadoc pointing users to CursorConverter.
• Add new CursorConverter in jme3-core with fromTexture and two fromTextureFrames overloads.
• Remove the CursorLoader registration from Desktop.cfg and update TestCursor to load PNG cursors / animated PNG frames via CursorConverter.

Reviewed changes

Copilot reviewed 4 out of 16 changed files in this pull request and generated 8 comments.

File	Description
jme3-desktop/src/main/java/com/jme3/cursors/plugins/CursorLoader.java	Adds @Deprecated and Javadoc directing users to CursorConverter.
jme3-core/src/plugins/java/com/jme3/cursors/plugins/CursorConverter.java	New cross-platform converter from Texture2D(s) to JmeCursor, supporting static and animated cursors.
jme3-core/src/main/resources/com/jme3/asset/Desktop.cfg	Removes the CursorLoader loader registration; file is now only an INCLUDE of General.cfg.
jme3-examples/src/main/java/jme3test/gui/TestCursor.java	Switches the example to PNG cursors and animated PNG frames via CursorConverter, adding a flipVertically helper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mondogo24]

Solved with commit 40ffe74. Since there are already null checks with Texture2D constructors and Image.clone() with good messages, I thought about deleting the null checks. But I am open to add back the manual null checks if someone disagree.

[mondogo24]

And also including the java's internal null checks when calling methods of classes.

[mondogo24]

I have seen the javadoc check fail. I got confused with something in the google coding style and misused some brackets, Sorry! Ready to continue.

[jaime-jmebot]

The conversion currently packs each pixel as 0xAARRGGBB, but JmeCursor documents its imagesData as RGBA bytes and the native cursor implementations consume the buffer in that order. This swaps the red and blue channels on little-endian platforms (and can produce incorrect colors); please pack/write the channels in the format expected by the cursor backends.

[mondogo24]

@jaime-jmebot in which documents is metion that imagesData must be in a RGBA format? I have seen that in AndroidMouseInput24 implementation: RGBA is assumed. But in LWJGL: ARGB in needed (as this documentation states: https://legacy.lwjgl.org/javadoc/org/lwjgl/input/Cursor.html). So I also ask if there is any format standard for jmonkeyengine.

[jaime-jmebot]

Thanks for tracing the backend differences and making the output format configurable. That is a reasonable way to avoid pretending the existing JmeCursor data has one consistent pixel convention, and the example's explicit LWJGL configuration makes its intended target clear. One important remaining mismatch is that Android's current setNativeCursor() reads four int channel values per pixel (inputImage.get(tmpPixel)), while this converter emits one packed int per pixel for either ARGB or RGBA. So selecting RGBA still won't make this converter output consumable by Android; the converter/backend contract needs to account for that distinct buffer shape (or the backend needs adapting). The row traversal and flip are configurable, but the vertical-flip option should also be considered alongside hotspot coordinates so the hotspot continues to refer to the same point after the pixels are flipped.

[jaime-jmebot]

Thanks for explaining the intended role of the flip option. I agree that it should not unexpectedly alter the caller's configured hotspot if the API defines it as a pixel-data-only transform. The trade-off is that a fixed hotspot after flipping points to a different pixel in the cursor artwork; callers who want the same semantic point (for example, the tip of an arrow) will need to adjust the hotspot themselves. It would help to state that explicitly in the converter's documentation, so the behavior is predictable.

On the Android build issue, I checked the current jme3-android-examples/build.gradle: it now declares the test-data dependency through libs.jme3.testdata and extracts it before preBuild. The exact failure can depend on the Gradle task and environment, so could you share the command and the relevant error output? That will make it possible to tell whether this is still a build-script issue or an SDK/emulator setup issue.

[jaime-jmebot]

Thanks for the follow-up. I checked the updated conversion path: RGBAI now writes four integer channel values per pixel, matching what AndroidMouseInput24 reads, and the animated buffer allocation scales for that format too. That resolves the buffer-shape mismatch I called out. The Android backend still uses only one frame, but the format conversion itself now matches its current static-cursor consumer.

[jaime-jmebot]

• The new cursor pixel packing does not match the documented/backend RGBA buffer layout, so cursor colors may be corrupted.
• Please adjust the channel ordering to match what JmeCursor backends consume.

[jaime-jmebot]

Hi @mondogo24, thanks for raising this. You're right that the LWJGL2 Cursor API expects packed ARGB integers, so the current 0xAARRGGBB packing is correct for the LWJGL backend; my earlier comment overstated the requirement as a single RGBA layout.

There isn't a single explicit pixel-format contract in JmeCursor itself—it only describes imagesData as a sequence of integers. In practice, the backends currently disagree: LWJGL passes the IntBuffer directly to its ARGB-based cursor constructor, while Android explicitly interprets each integer as RGBA bytes and reassembles it into ARGB. That means a universal packing convention needs to be established and applied consistently in the backend implementations (or converted at each backend boundary), rather than assuming one backend's format is the cross-platform standard.

Could you also check the converter's row order against the documented lower-left cursor coordinate convention? The ImageRaster traversal currently reads rows from y = 0 upward, so that detail should be kept consistent with how each native backend interprets the pixels.

[mondogo24]

Ok, first, sorry for this long pull request. Didn't expect that this deprecation would be such a mess in some details, even AI has been confused about some details like the format packaging since the beginning.

So, I have done some research and I could identify these three backends/classes that use JmeCursor for changing cursors: SdlMouseInput, LwjglMouseInput and AndroidMouseInput24. Apparently, each class expects a different image data format without being well documented and that caused some confusion. Standardizing a format will surely break some client code and I believe that an Issue should be raised and discussed. So, in order to create a more cross-platform tool, I adapted the CursorConverter class.

Now, CursorConverter is configurable from it' s constructor, so depending of the target backend, you can configure the class to return the JmeCursor object needed. Also, about the concern raised by @jaime-jmebot, couldn't get much from the Internet, but from Gemini I believe is safe to say that the SDL, LWJGL and Android libraries need a "row expanded" image data (from y=0 upward). So being able to configure the byte package format and vertical flips should be enough. Even, I an issue is raised about that a "column expasion" is needed, it should be easy to add that feature to CursorConverter.

One more thing, this pull request need the assets that I have summited in the jme-testdata repository as a pull request: jMonkeyEngine/jme-testdata#7. Otherwise, the test named "TestCursor" won't find the assets.

[jaime-jmebot]

Hi @mondogo24, thanks for taking another pass through the backend behavior and making the converter's pixel format and vertical flip configurable. That is a useful way to expose the existing backend differences without silently changing the legacy JmeCursor contract, and the updated example clearly shows how to configure a cursor for the LWJGL path.

I checked the converter and the cursor backend implementations. The remaining cross-platform gap is Android: its current cursor path consumes four channel values per pixel, whereas CursorConverter produces one packed integer per pixel in either ARGB or RGBA. As I noted in the thread, selecting the RGBA option therefore still does not produce the buffer shape Android expects. Please make sure this contract is addressed before presenting the converter as a cross-platform replacement. Also, the default hotspot and its bounds check currently allow a coordinate equal to the image dimension, which is outside the valid pixel index range; I left suggestions on the affected overloads and validation helper. When vertical flipping is enabled, it would also be worth keeping explicit hotspot coordinates aligned with the flipped pixels.

The core approach of converting ordinary Texture2D assets, and combining same-sized frames for animation, is a nice practical replacement for format-specific cursor loading. The example depends on the corresponding assets being available in jme-testdata, as you mention. Once the buffer contract and hotspot edge cases are resolved, this will be in good shape for another review. Thanks for working through the backend details carefully.

[mondogo24]

These should cover the issues raised by @jaime-jmebot. About this mention:

When vertical flipping is enabled, it would also be worth keeping explicit hotspot coordinates aligned with the flipped pixels.

My idea with vertical flipping is basically a way to deal with backends and image file formats problems that might require flipping the images. So, from my perspective, image flipping should only change the "visuals" and don't have secondary effects.

Also, I tried to run the Android example and got a build error in line 59 of the following build.gradle:

jmonkeyengine/jme3-android-examples/build.gradle

Lines 52 to 63 in 1bcd65e

	sourceSets {
	main {
	java {
	srcDir 'src/main/java'
	}
	assets {
	srcDir 'src/assets'
	srcDir layout.buildDirectory.dir('jme3-testdata')
	srcDir '../jme3-examples/src/main/resources'
	}
	}
	}

I don't have much experience in gradle but I suspect that the file hasn't been updated since the migration of 'jme3-testdata'. Just wanted to mention it in case it is not a problem of mine (like emulator not well configured or something).

[jaime-jmebot]

Hi @mondogo24, thanks for continuing to work through the cursor backend differences and for sharing the Android build concern. The converter's configurable output and row handling are a thoughtful way to avoid changing the existing cursor conventions unexpectedly, and keeping the desktop-only loader deprecated rather than removing it preserves compatibility.

I found one remaining buffer-size issue in the animated conversion path: the RGBAI output writes four integer channel values per pixel, while the allocated imagesData buffer is sized for one integer per pixel. That buffer will overflow when frames are written using this format, so the capacity needs to account for the selected format's channel count. I left an inline comment with a proposed adjustment.

On the vertical flip, I understand your intent that it changes pixel presentation only. In that case, leaving hotspot coordinates untouched is a reasonable API choice, but the hotspot will refer to a different point in the flipped artwork; documenting that behavior would help callers who need the hotspot to track a particular feature. For the Android examples build, I checked the current script and it already extracts the libs.jme3.testdata dependency before preBuild. If the issue persists, please share the exact Gradle command and error output so we can distinguish a script problem from local SDK or emulator setup.

Thanks for the careful backend investigation. Once the animated buffer capacity is corrected, I’d be glad to take another look.

[mondogo24]

@jaime-jmebot This should fix the discussed. I decided to create a new method to avoid image format dependent code be too far away from each other.

[jaime-jmebot]

One edge case remains for the animated overloads: when cursorFrames and frameDelays are both empty, the length check passes and these lists stay empty, so imageFrameHeights.get(0) throws an IndexOutOfBoundsException. Please reject an empty frame array explicitly so callers get a clear argument error instead of an implementation exception.

Suggested change

	.collect(Collectors.toList());
	if (imageFrameHeights.isEmpty()) {
	throw new IllegalArgumentException("At least one cursor frame is required");
	}
	int imageHeight = imageFrameHeights.get(0);

[jaime-jmebot]

This deep-copy path uses BufferUtils.clone(ByteBuffer), which calls rewind() on the supplied buffer and advances it to its limit while copying. Because originalBuffer belongs to the source Image, calling deepClone() therefore changes the source image's buffer position as a side effect (including the cached asset images used by the converter). Please clone a duplicate so the source buffer's position and limit remain untouched.

Suggested change

	}
	clone.add(BufferUtils.clone(originalBuffer.duplicate()));

[jaime-jmebot]

There is also a problem with the cloned buffer's state: BufferUtils.clone() leaves its returned buffer positioned at its limit. Since Image data is commonly consumed via the buffer's remaining elements, a deepClone() can consequently appear to contain no image data. Please reset the copied buffer for reading before storing it.

Suggested change

	}
	ByteBuffer clonedBuffer = BufferUtils.clone(originalBuffer.duplicate());
	clonedBuffer.flip();
	clone.add(clonedBuffer);

[jaime-jmebot]

Correction: I’m not confident in my previous note about the cloned buffer’s position without verifying the exact BufferUtils.clone() contract, so please disregard that suggestion. The source-buffer side-effect concern in my other comment still stands if the helper rewinds its input; cloning a duplicate protects the source buffer’s state.

[jaime-jmebot]

Hi @mondogo24, thanks for the update and for separating the format-specific conversion into its own method. I rechecked the Android path: the RGBAI output now writes four integer channel values per pixel, which matches what AndroidMouseInput24 reads, and the animated buffer allocation accounts for that larger representation. That resolves the buffer-shape issue I raised earlier.

I did spot two edge cases in the new image-copy and animated-frame paths. Image.deepClone() should copy from a duplicate of each source buffer so that cloning does not alter the original buffer's position or limit; that matters particularly when the source image is shared through the asset cache. Also, the animated conversion path needs to reject an empty frame list before indexing the first frame, otherwise an empty input fails with an IndexOutOfBoundsException rather than a useful argument error. I’ve left inline suggestions for both.

The converter and example now cover the intended configurable formats and animation use cases clearly. Once those defensive cases are handled, I think this will be ready for another look. Thanks for the careful follow-through!