More changes trying to get DpiAware working (BL-16269)#7919
More changes trying to get DpiAware working (BL-16269)#7919JohnThomson wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the effort to make Bloom’s WinForms/React-based dialogs behave correctly in DPI-aware (PerMonitorV2) scenarios by switching several dialogs from fixed pixel sizing to “logical” sizing that is scaled to the monitor DPI at runtime, and by tightening the scope of legacy DPI context during startup.
Changes:
- Added
ReactDialog.SetScaledSize()and updated manyReactDialogusages to size dialogs using scaled logical dimensions instead of rawWidth/Height. - Adjusted
Program.ChooseACollection()to ensure the legacy DPI scope only applies to the collection chooser dialog, while opening the main shell happens in normal PerMonitorV2 context. - Updated ownership/positioning patterns for a few dialogs (e.g., problem report and team collection progress) to better align sizing with the intended owner window.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BloomExe/Workspace/WorkspaceView.cs | Auto-update dialog now uses scaled logical sizing. |
| src/BloomExe/web/controllers/ProblemReportApi.cs | Problem report dialog now scales size and reuses a computed owner. |
| src/BloomExe/TeamCollection/TeamCollection.cs | Team collection progress dialog now uses scaled sizing and passes an explicit owner to the progress-dialog runner. |
| src/BloomExe/TeamCollection/FolderTeamCollection.cs | Join team collection dialog now uses scaled logical sizing. |
| src/BloomExe/Program.cs | Limits legacy DPI scope to chooser dialog; opens collection afterward in normal DPI context. |
| src/BloomExe/MiscUI/ReactDialog.cs | Introduces SetScaledSize() and applies scaling in OnLoad. |
| src/BloomExe/MiscUI/BloomMessageBox.cs | Message box dialog now uses scaled logical sizing. |
| src/BloomExe/ErrorReporter/HtmlErrorReporter.cs | Error reporter dialog now uses scaled sizing when it is a ReactDialog, with fallback sizing otherwise. |
| src/BloomExe/Edit/EditingView.cs | Copyright/license dialog now uses scaled logical sizing. |
| src/BloomExe/Edit/EditingModel.cs | Duplicate-many-pages dialog now uses scaled logical sizing. |
| src/BloomExe/Collection/CollectionSettingsDialog.cs | Language chooser dialog now uses scaled logical sizing and reuses a computed owner. |
| src/BloomExe/BloomIntegrityChecker.cs | Integrity-check failure dialog now manually scales size based on DeviceDpi. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // on its actual target monitor | ||
| _desiredLogicalWidth = logicalWidth; | ||
| _desiredLogicalHeight = logicalHeight; | ||
| } | ||
|
|
||
| private int _desiredLogicalWidth = 0; | ||
| private int _desiredLogicalHeight = 0; | ||
|
|
||
| protected override void OnLoad(EventArgs e) | ||
| { | ||
| base.OnLoad(e); | ||
|
|
||
| // Apply the desired size during load, after positioning on the target monitor. | ||
| // Scale the logical dimensions based on the actual DPI of the monitor we're on. | ||
| if (_desiredLogicalWidth > 0 || _desiredLogicalHeight > 0) | ||
| { | ||
| int scaledWidth = (int)Math.Round(_desiredLogicalWidth * DeviceDpi / 96.0); | ||
| int scaledHeight = (int)Math.Round(_desiredLogicalHeight * DeviceDpi / 96.0); |
| dlg.Width = 750; | ||
| // Use logical dialog dimensions and scale them to current monitor DPI. | ||
| // Force handle creation first so DeviceDpi reflects the target display context. | ||
| var unusedHandle = dlg.Handle; |
| @@ -2560,14 +2562,13 @@ protected void ShowProgressDialog( | |||
| showReportButton = "never", | |||
| }, | |||
| "Sync Team Collection" | |||
| ) | |||
| // winforms dialog properties | |||
| { | |||
| Width = 620, | |||
| Height = 550, | |||
| }, | |||
| ); | |||
| dlg.SetScaledSize(620, 550, owner); | |||
| return dlg; | |||
| }, | |||
| doWhat, | |||
| doWhenMainActionFalse | |||
| doWhenMainActionFalse, | |||
| Shell.GetShellOrOtherOpenForm() | |||
There was a problem hiding this comment.
No longer passing to SetScaledSize
| // Normally, I'd want to fail fast and throw an exception if it isn't, | ||
| // but here we're in the middle of reporting an error, so I'd rather we just do our | ||
| // best to show the original error. So we'll go with prior code (and maybe get a poorly sized | ||
| // dialog if scale is not 100%).) |
1167442 to
444ff7f
Compare
444ff7f to
7b2bd9e
Compare
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson made 4 comments and resolved 2 discussions.
Reviewable status: 0 of 12 files reviewed, 2 unresolved discussions.
| dlg.Width = 750; | ||
| // Use logical dialog dimensions and scale them to current monitor DPI. | ||
| // Force handle creation first so DeviceDpi reflects the target display context. | ||
| var unusedHandle = dlg.Handle; |
| // Normally, I'd want to fail fast and throw an exception if it isn't, | ||
| // but here we're in the middle of reporting an error, so I'd rather we just do our | ||
| // best to show the original error. So we'll go with prior code (and maybe get a poorly sized | ||
| // dialog if scale is not 100%).) |
| // on its actual target monitor | ||
| _desiredLogicalWidth = logicalWidth; | ||
| _desiredLogicalHeight = logicalHeight; | ||
| } | ||
|
|
||
| private int _desiredLogicalWidth = 0; | ||
| private int _desiredLogicalHeight = 0; | ||
|
|
||
| protected override void OnLoad(EventArgs e) | ||
| { | ||
| base.OnLoad(e); | ||
|
|
||
| // Apply the desired size during load, after positioning on the target monitor. | ||
| // Scale the logical dimensions based on the actual DPI of the monitor we're on. | ||
| if (_desiredLogicalWidth > 0 || _desiredLogicalHeight > 0) | ||
| { | ||
| int scaledWidth = (int)Math.Round(_desiredLogicalWidth * DeviceDpi / 96.0); | ||
| int scaledHeight = (int)Math.Round(_desiredLogicalHeight * DeviceDpi / 96.0); |
| @@ -2560,14 +2562,13 @@ protected void ShowProgressDialog( | |||
| showReportButton = "never", | |||
| }, | |||
| "Sync Team Collection" | |||
| ) | |||
| // winforms dialog properties | |||
| { | |||
| Width = 620, | |||
| Height = 550, | |||
| }, | |||
| ); | |||
| dlg.SetScaledSize(620, 550, owner); | |||
| return dlg; | |||
| }, | |||
| doWhat, | |||
| doWhenMainActionFalse | |||
| doWhenMainActionFalse, | |||
| Shell.GetShellOrOtherOpenForm() | |||
There was a problem hiding this comment.
No longer passing to SetScaledSize
|
devin says src/BloomExe/MiscUI/ReactDialog.cs:R44-50 RegistrationManager.cs still uses direct Width/Height on ReactDialog This PR consistently converts all |
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson made 1 comment.
Reviewable status: 0 of 12 files reviewed, 3 unresolved discussions (waiting on andrew-polk).
src/BloomExe/MiscUI/ReactDialog.cs line 44 at r2 (raw file):
Previously, andrew-polk wrote…
devin says
src/BloomExe/MiscUI/ReactDialog.cs:R44-50
RegistrationManager.cs still uses direct Width/Height on ReactDialog
This PR consistently converts all
ReactDialogwidth/height assignments to useSetScaledSize, butRegistrationManager.ShowRegistrationDialog(src/BloomExe/Registration/RegistrationManager.cs:27-28) still setsregistrationDialog.WidthandregistrationDialog.Heightdirectly without DPI scaling. This means the registration dialog won't scale correctly on high-DPI monitors. It appears this file was simply missed during the transformation. Consider converting it to useSetScaledSizefor consistency.
Done
This change is