Destroy unused handles #1477
Open
Destroy unused handles #1477
+139
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amartya4256
force-pushed
the
destroy_handles
branch
6 times, most recently
from
c0b5222
to
a81be93
Compare
fedejeanne
reviewed
Sep 25, 2024
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Device.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Resource.java
Show resolved
Hide resolved
amartya4256
force-pushed
the
destroy_handles
branch
from
a81be93
to
99f22f7
Compare
amartya4256
force-pushed
the
destroy_handles
branch
3 times, most recently
from
414cfe6
to
c639a59
Compare
This Contribution destroys those handles for the resources which were created for zoom levels which no monitor has anymore. The process is triggered on a DPI_CHANGED event. contributes to eclipse-platform#62 and eclipse-platform#127
amartya4256
force-pushed
the
destroy_handles
branch
from
c639a59
to
b728726
Compare
fedejeanne
reviewed
Oct 2, 2024
| @@ -947,4 +953,24 @@ public void setWarnings (boolean warnings) { | |||
| protected int getDeviceZoom () { | |||
| return DPIUtil.mapDPIToZoom ( _getDPIx ()); | |||
| } | |||
|
|
|||
| void registerResource(Resource resource) { | |||
There was a problem hiding this comment.
Suggested change
| void registerResource(Resource resource) { | |
| void registerResourceWithZoomSupport(Resource r) { |
| @@ -55,6 +58,7 @@ public abstract class Device implements Drawable { | |||
| String[] loadedFonts; | |||
|
|
|||
| volatile boolean disposed; | |||
| private Vector<Resource> resources = new Vector<>(); | |||
There was a problem hiding this comment.
Suggested change
| private Vector<Resource> resources = new Vector<>(); | |
| private Vector<Resource> resourcesWithZoomSupport = new Vector<>(); |
Also, why use Vector and not Collection / List? Do you need synchronization?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contributes to the cleanup of those handles of Resources which were created for the zoom levels which are not applicable for the existing zoom levels of all the monitor. i.e., if there were 2 monitors with 100% and 200% zoom levels and the seconds monitor is switched to 100% as well. On the DPI_CHANGED event, we trigger a cleanup of handles of the resources using the resource tracker to remove the handles which were created for 200% zoom level since there doesn't exist any monitor with a zoom level of 100% anymore.
With this PR, we also refactor the resource tracker to allow it to track all the resources for win32 all the time but rather using the tracking flag to report errors, since that is the only use case of the tracking flag as of now. By allowing the Resource Tracker to track the resources, we can trigger the cleanup of all the relevant handles from a single point.
Contributes to #62 and #127