Skip to content

Conversation

@HeikoKlare
Copy link
Contributor

@HeikoKlare HeikoKlare commented Jan 16, 2025

Monitor-specific scaling needs to be activated before the display that is supposed to run in that mode is created. Currently, the scaling is activated in the Workbench based on an according preference after the Display has already been created.

This change moves the evaluation of the experimental preference and the according initialization of monitor-specific scaling to before the Display is created inside the Workbench. Since the preference is now accessed at a point in time where no workspace may already be available, the preference was moved to the ConfigurationScope so that it is stored independent from the actual workspace.

The drawback with this approach is that for an Eclipse application started from a workspace, where the configuration area will usually be cleared before every application start, the preference will be reset on every restart. Thus, for that scenario, the system property would currently have to be set explicitly via the VM arguments.

Fixes #2712

Closes #2713

Monitor-specific scaling needs to be activated before the display that
is supposed to run in that mode is created. Currently, the scaling is
activated in the Workbench based on an according preference after the
Display has already been created.

This change moves the evaluation of the experimental preference and the
according initialization of monitor-specific scaling to before the
Display is created inside the Workbench. Since the preference is now
accessed at a point in time where no workspace may already be available,
the preference was moved to the ConfigurationScope so that it is stored
independent from the actual workspace.
@HeikoKlare
Copy link
Contributor Author

@laeubi may I ask you to have a look at this? I had not so much to do with preferences yet, so there is probably a better / more "correct" way to manage preferences in the configuration scope.
This may supercede or be a follow-up to #2713

@laeubi
Copy link
Contributor

laeubi commented Jan 16, 2025

The drawback with this approach is that for an Eclipse application started from a workspace, where the configuration area will usually be cleared before every application start, the preference will be reset on every restart

I already mentioned this here:

I think Equniox is sadly doing "too much" maybe here...

boolean rescaleAtRuntime = PrefUtil.getAPIPreferenceStore()
.getBoolean(IWorkbenchPreferenceConstants.RESCALING_AT_RUNTIME);
boolean rescaleAtRuntime = ConfigurationScope.INSTANCE.getNode(WorkbenchPlugin.PI_WORKBENCH)
.getBoolean(IWorkbenchPreferenceConstants.RESCALING_AT_RUNTIME, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

should this possibly take a system property into account for the default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question would be which one to take into account. What the preference currently does is to set a system property which configures the HiDPI behavior of SWT. If you specify that system property manually (via INI/VM argument), this will be used anyway (in the if-block above) and the preference will be ignored.
So if we want to take a preference into account, we should probably just remove the if/else and integrate the system property value into this preference evaluation. The reason for not doing it that way was that we wanted to log a warning, as the interaction between system property and preference may be hard to understand, so setting a VM argument or INI flag to define it should be avoided in favor of simply using the preference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I would keep as is for now (it conforms to the existing behavior also defaulting to "false"). Additionally, this is currently only an experimental preference (which is reasonably set to "false" by default) to be replaced by a "proper" one once the feature is ready to be used productively and potentially activated by default.

@HeikoKlare
Copy link
Contributor Author

I already mentioned this here:

I think Equniox is sadly doing "too much" maybe here...

Interesting. Thank you for the pointer! I did not even question the behavior, I just wanted to mention that it will have some impact on configuring (and in particular testing) the monitor-specific scaling feature.

@HeikoKlare HeikoKlare marked this pull request as ready for review January 16, 2025 10:10
@github-actions
Copy link
Contributor

Test Results

 1 818 files  ±0   1 818 suites  ±0   1h 30m 56s ⏱️ + 1m 3s
 7 733 tests ±0   7 504 ✅ ±0  228 💤 ±0  1 ❌ ±0 
24 360 runs  ±0  23 610 ✅ ±0  749 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 7e6708a. ± Comparison against base commit de8c667.

@HeikoKlare
Copy link
Contributor Author

Failing test is unrelated and documented: #294

@HeikoKlare HeikoKlare merged commit 05b9f73 into eclipse-platform:master Jan 16, 2025
15 of 17 checks passed
@HeikoKlare HeikoKlare deleted the monitor-specific-scaling-preference branch January 16, 2025 10:50
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.

IDE can't start anymore without preselected workspace

2 participants