Skip to content

Conversation

@HeikoKlare
Copy link
Contributor

Currently, DPI change handling is implemented in Control. However, according OS events are only created and reasonable at Shell level. This change thus moves the according implementations to the Shell class. It also limits the last queued DPI event storage for cancellation on subsequent events to the Shell class.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

Test Results

  115 files  ±0    115 suites  ±0   11m 38s ⏱️ +42s
4 554 tests ±0  4 538 ✅ ±0  16 💤 ±0  0 ❌ ±0 
  311 runs  ±0    308 ✅ ±0   3 💤 ±0  0 ❌ ±0 

Results for commit dba7865. ± Comparison against base commit 1f8df87.

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare marked this pull request as ready for review October 16, 2025 17:35
Currently, DPI change handling is implemented in Control. However,
according OS events are only created and reasonable at Shell level.
This change thus moves the according implementations to the Shell class.
It also limits the last queued DPI event storage for cancellation on
subsequent events to the Shell class.
Copy link
Member

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I tested around and I could find any place that would reach the now empty methods in Control. Approved ✔️

@HeikoKlare would it be too bold or wrong to do this in Control?

LRESULT WM_DPICHANGED(long wParam, long lParam) {
	throw new RuntimeException("This method should not be reached");
}

LRESULT WM_DISPLAYCHANGE(long wParam, long lParam) {
	throw new RuntimeException("This method should not be reached");
}

@HeikoKlare
Copy link
Contributor Author

would it be too bold or wrong to do this in Control?

Probaby those events should only be sent for shells, but I am not sure if we can or should safely assume that. For example, embedded external window handles could be a case where something else happens.
In any case, I don't think we get any benefit when throwing an exception here, do we? Even if such an event was sent for a control that is not a shell, we wouldn't want to handle that event anway, so we should not throw an exception. If we wanted to avoid returning null here, it would probably be better then to move the methods and their calls to Shell, but since for other OS events the same is done (returning null in Control and doing the relevant work in subclasses) and since it would require duplication of the windowProc logic, I just adopted the existing pattern.

@fedejeanne fedejeanne merged commit c422be8 into eclipse-platform:master Oct 21, 2025
17 checks passed
@fedejeanne fedejeanne deleted the dpichangehandling-shell branch October 21, 2025 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move DPI change handling from Control to Shell

2 participants