-
Notifications
You must be signed in to change notification settings - Fork 137
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
Refactor Image for storing ImageMetadata #1496
base: master
Are you sure you want to change the base?
Conversation
Test Results 483 files ±0 483 suites ±0 8m 30s ⏱️ +11s For more details on these failures, see this check. Results for commit 8f0c350. ± Comparison against base commit 5d67ce6. ♻️ This comment has been updated with latest results. |
68e8193
to
597c46d
Compare
597c46d
to
9acf71a
Compare
3832a24
to
e353b1a
Compare
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
@@ -2312,6 +1979,7 @@ public void setBackground(Color color) { | |||
|
|||
/* Get the HDC for the device */ | |||
long hDC = device.internal_new_GC(null); | |||
long handle = win32_getHandle(this, getZoom()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, why using this handle? Shouldn't that be applied to all handles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Earlier, it was just done by the handle. But you are right. It means it was a wrong implementation already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume your comment has been addressed, @akoch-yatta ?
The current implementation calls setBackground(Color color, long handle)
for each handle in zoomLevelToImageHandle
:
public void setBackground(Color color) {
...
zoomLevelToImageHandle.values().forEach(imageHandle -> setBackground(color, imageHandle.handle));
}
private void setBackground(Color color, long handle) {
...
/* Get the HDC for the device */
long hDC = device.internal_new_GC(null);
/* Change the background color in the image */
BITMAP bm = new BITMAP();
OS.GetObject(handle, BITMAP.sizeof, bm);
long hdcMem = OS.CreateCompatibleDC(hDC);
OS.SelectObject(hdcMem, handle);
...
}
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
8067b98
to
6225eae
Compare
This contribution encapsulates the metadata of the image in an innerclass ImageHandle which is used to create a hashmap of zoom level to imageHandle object inside an image object, making it straight forward to obtain any metadata information from an image for a zoom level. contributes to eclipse-platform#62 and eclipse-platform#127
6225eae
to
8f0c350
Compare
Converted it back to Draft, this one should be merged in 4.35 M1 |
This PR contributes to the refactoring of
Image
class for win32.The goal is to store more than just the
handle
of the image for each zoom level. To do that, a new private class is created (it's calledImageHandle
) and an image contains one instance of this new class for each zoom level. The class contains the following fields:handle
height
width
contributes to #62 and #127