Skip to content

Commit

Permalink
ShellView>>createAt:extent: may incorrectly position and scale window
Browse files Browse the repository at this point in the history
The position and extent passed to createAt:extent: are specified in 96-dpi
co-ordinates. However, the scaling code treats them as system dpi
co-ordinates in determining the monitor on which they are created. This
may cause the window to be incorrectly placed.
  • Loading branch information
blairmcg committed Sep 13, 2024
1 parent 1b4c782 commit 6cd3a22
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 11 deletions.
2 changes: 1 addition & 1 deletion Core/Object Arts/Dolphin/MVP/Base/UI.CreateWindow.cls
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ resolveShellGeometry: aShellView
ifFalse: [monitor nextWindowPosition]]
ifFalse:
["Position was specified. Determine the monitor from that."
monitor := DisplayMonitor nearestPoint: position.
monitor := DisplayMonitor nearestPoint: position * SystemMetrics current dpi // dpi.
targetDpi := monitor effectiveDpi.
"Now we can calculate the extent as above"
extent := extent = UseDefaultGeometry
Expand Down
22 changes: 14 additions & 8 deletions Core/Object Arts/Dolphin/MVP/Base/UI.DisplayMonitor.cls
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,12 @@ adjustWindowRect: aRectangle
yourself!

cacheInfo
| buf |
buf := MONITORINFOEXW newBuffer.
(UserLibrary dpiAwareness inContextDo: [UserLibrary default getMonitorInfo: handle lpmi: buf])
ifFalse: [Win32Error signal].
workArea := buf workArea.
rectangle := buf rectangle.
deviceName := buf szDevice.
isPrimary := buf isPrimary.!
| monitorInfo |
monitorInfo := self infoWithDpiAwareness: UserLibrary dpiAwareness.
workArea := monitorInfo workArea.
rectangle := monitorInfo rectangle.
deviceName := monitorInfo szDevice.
isPrimary := monitorInfo isPrimary!

cascadeOffset
"Private - Answer the offset of a new default window position from the last. This should neatly cascade the windows in a diagonal down from a starting point inset from the top corner by one such offset.
Expand Down Expand Up @@ -159,6 +157,13 @@ info
(User32 getMonitorInfo: handle lpmi: info) ifFalse: [Win32Error signal].
^info!

infoWithDpiAwareness: aDpiAwareness
| buf |
buf := MONITORINFOEXW newBuffer.
(aDpiAwareness inContextDo: [UserLibrary default getMonitorInfo: handle lpmi: buf])
ifFalse: [Win32Error signal].
^buf!

isAttachedToDesktop
"Answer whether the receiver has any displays attached to the desktop."

Expand Down Expand Up @@ -259,6 +264,7 @@ handle!accessing!private! !
handle:!accessing!private! !
hash!comparing!public! !
info!accessing!private! !
infoWithDpiAwareness:!enquiries!private! !
isAttachedToDesktop!public!testing! !
isPrimary!public!testing! !
metrics!accessing!public! !
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ testCreatedNearOwnerModeless

testInplaceNotRepositioned
| desktopMonitors position dialog leftMonitor ownerRect |
desktopMonitors := DisplayMonitor desktopMonitors
asSortedCollection: [:a :b | a workArea topLeft < b workArea topLeft].
desktopMonitors := self desktopMonitors.
leftMonitor := desktopMonitors first.
"This test requires multiple monitors arranged in a typical horizontal configuration. It could be made to work in a vertical configuration, but there isn't much value."
self skipUnless:
Expand Down
4 changes: 4 additions & 0 deletions Core/Object Arts/Dolphin/MVP/Tests/UI.Tests.PresenterTest.cls
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ UI.Tests.PresenterTest comment: ''!
classToTest
^self subclassResponsibility!

desktopMonitors
^DisplayMonitor desktopMonitors asSortedArray: [:a :b | a origin < b origin]!

destroyPresenter
| shell |
presenter ifNil: [^self].
Expand Down Expand Up @@ -203,6 +206,7 @@ waitForInputIdle

!UI.Tests.PresenterTest categoriesForMethods!
classToTest!constants!private! !
desktopMonitors!helpers!private! !
destroyPresenter!public!Running! !
getTextExtent:!helpers!private! !
hasTestStbViewResources!private!testing! !
Expand Down
13 changes: 13 additions & 0 deletions Core/Object Arts/Dolphin/MVP/Tests/UI.Tests.ShellViewTest.cls
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ testOpensOnForegroundMonitor
shell3 := self createShell: self printString , ': shell3' centered: false.
self assert: shell3 displayMonitor equals: primary!

testPositionAtOriginOfLastMonitor
| subject monitor designPosition designExtent |
monitor := self desktopMonitors last.
designPosition := (monitor infoWithDpiAwareness: DpiAwareness unaware) rcWork origin.
designExtent := 200 @ 100.
subject := shells add: ShellView new.
subject
createAt: designPosition extent: designExtent;
show.
self assert: subject position equals: monitor origin.
self assert: subject extent equals: designExtent * monitor dpi // USER_DEFAULT_SCREEN_DPI!

verifyUpgradedView: anInteger identifier: aResourceIdentifier
| view |
super verifyUpgradedView: anInteger identifier: aResourceIdentifier.
Expand All @@ -88,6 +100,7 @@ classToTest!helpers!private! !
createShell:centered:!helpers!private! !
testDefaultPositioning!public!unit tests! !
testOpensOnForegroundMonitor!public!unit tests! !
testPositionAtOriginOfLastMonitor!public!unit tests! !
verifyUpgradedView:identifier:!helpers!private! !
!

Expand Down

0 comments on commit 6cd3a22

Please sign in to comment.