[Win32] Properly encapsulate cursor handle creation and disposal #2403
Conversation
Test Results 546 files 546 suites 33m 44s ⏱️ Results for commit 6cd670a. ♻️ This comment has been updated with latest results. |
| @@ -120,7 +119,7 @@ public final class Cursor extends Resource { | |||
| public Cursor(Device device, int style) { | |||
| super(device); | |||
| this.cursorHandleProvider = new StyleCursorHandleProvider(style); | |||
| this.handle = this.cursorHandleProvider.createHandle(this, DEFAULT_ZOOM); | |||
| this.handle = this.cursorHandleProvider.createHandle(device, DEFAULT_ZOOM).getHandle(); | |||
There was a problem hiding this comment.
just a Nit: The handle field still stores the raw long, while zoomLevelToHandle stores the CursorHandle. This introduces a risk of the two becoming out-of-sync if the logic changes in the future. IMO It might be cleaner to store the CursorHandle directly in handle and call this.handle.getHandle() (or rename the method to getNativeHandle() for clarity) wherever the long handle is needed @HeikoKlare
There was a problem hiding this comment.
It's true that the two values may get out of sync but that's actually not different from the current state. CursorHandle is immutable, so it will be as consistent to the current handle value as it was before. We need to get rid of the handle, but that's something that will be done with the lazy generation of handles implemented by Shahzaib.
|
I have tried with different zoom levels (100/150/175) and changes looks good to me |
Currently, the distinction of how to create a cursor handle inside image-data-based providers depends on state of a given cursor instance and requires dummy values to be passed (such as `mask==null`). This change extracts the according behaviors into separate classes that encapsulate the required data and functionality.
Currently, cursor handles are destroyed in different ways depending on whether the `isIcon` field is set. This field is set in cases when the handle is created for an OS icon handle. This contract has to be ensured implicitly and can easily be broken. With this change, the cursor handle creation creates an according CursorHandle type that is capable of executing the associated disposal logic.
HeikoKlare
force-pushed
the
cursor-disposal-CursorHandle
branch
from
7500742 to
6cd670a
Compare
Currently, the
Cursorimplementation has a design flaw related to theisIconfield:This change gets rid of the
isIconfield by two means:CursorHandleclass that encapsulates the destroy logic for a handle and assign it upon creation of the handle, such that destroy logic fits to how the handle was created.ImageDataWithMaskHandleProviderthat properly handles the case that a Cursor was created with a mask which was yet only identified by theisIconfield of a cursor.It is split up into to commits addressing each of the above concerns but with the common goal of getting rid of the
isIconfield.