Summary: | [HiDPI] Cap swt.autoScale at 200% by default | ||
---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> |
Component: | SWT | Assignee: | Markus Keller <markus.kell.r> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | arunkumar.thondapu, lshanmug, markus.kell.r, niraj.modi, sravankumarl |
Version: | 4.7 | ||
Target Milestone: | 4.7 M7 | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://git.eclipse.org/r/92021 https://git.eclipse.org/r/92151 https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=6cdabca59de9a57692b24da180f04cc98307f448 |
||
Whiteboard: | |||
Bug Depends on: | |||
Bug Blocks: | 511188, 510974 |
Description
Markus Keller
2017-02-28 10:26:55 EST
New Gerrit change created: https://git.eclipse.org/r/92021 The patch implements the suggestion using a new default "integer200" for the "swt.autoScale" property: b>integer200</b>: like <b>integer</b>, but the maximal zoom level is 200%. SWT/platform leads: Any opposition? (In reply to Markus Keller from comment #2) > The patch implements the suggestion using a new default "integer200" for the > "swt.autoScale" property: > > b>integer200</b>: like <b>integer</b>, but the maximal zoom level is 200%. > > SWT/platform leads: Any opposition? Hi Markus, Above problem is very specific to Eclipse at 300% zoom and it might effect applications which already have image bundles at zoom level higher than 200. We can think of two alternate ways to address this use-case in general way without affecting any of existing SWT/Eclipse clients(if any): 1. We can introduce a new system property as below e.g. swt.autoScale.maxZoomLevel=200 [or, alternately on lines with how we pass maximum RAM available to JVM] 2. We can overload the existing system property as below: swt.autoScale=200max swt.autoScale=100min (option for our use-case) Yes, this would require to add this new system property setting in eclipse.ini Let us know your thought on this. (In reply to Niraj Modi from comment #3) > (In reply to Markus Keller from comment #2) > > The patch implements the suggestion using a new default "integer200" for the > > "swt.autoScale" property: > > > > b>integer200</b>: like <b>integer</b>, but the maximal zoom level is 200%. > > > > SWT/platform leads: Any opposition? > > Hi Markus, > Above problem is very specific to Eclipse at 300% zoom and it might effect > applications which already have image bundles at zoom level higher than 200. > +1, we should provide a solution that is generic for SWT applications and not specific to this use-case alone. At least bug 510974 is not specific to Eclipse. It happens in all SWT apps that use the ToolBar widget, regardless of HiDPI icons. I think it's better to ship with defaults that work, rather than letting every user deal with known issues. Affecting existing SWT clients was the goal for this fix. (In reply to Markus Keller from comment #5) > At least bug 510974 is not specific to Eclipse. It happens in all SWT apps > that use the ToolBar widget, regardless of HiDPI icons. > > I think it's better to ship with defaults that work, rather than letting > every user deal with known issues. Affecting existing SWT clients was the > goal for this fix. Hi Markus, I agree we should set the default to what works. Setting the default cap to 200 would work best. The concern is "integer200" caps at 200% while capping at other zoom levels like 300% may be valid for some application. Since we are providing a way to cap the zoom level, may be we should let the cap value be configurable? New Gerrit change created: https://git.eclipse.org/r/92151 (In reply to Eclipse Genie from comment #7) > New Gerrit change created: https://git.eclipse.org/r/92151 Created an alternate patch, on lines with 2nd approach @ comment 3. This patch by default sets maximum supported zoom level to 200% for SWT/Eclipse based applications, yet the value is user configurable. For applications which want to go beyond 200% zoom level, they can configure it via below system property e.g. swt.autoScale=300:max Note: Since by default the maximum supported zoom would be 200%, so Eclipse wouldn't require to add a new system property setting in eclipse.ini for now. The current default "integer" is already a concession to the open problems mentioned in bug 493462. My proposed "integer200" would just extend the default to avoid bug 510974 and 511188 as well. (In reply to Niraj Modi from comment #8) > > New Gerrit change created: https://git.eclipse.org/r/92151 By default, that patch would behave the same as mine, but it has quite a few problems: - The patch affects all modes, with no way to disable capping for the other modes. A no-go. - The format is confusing. swt.autoScale=200:max would be equivalent to my integer200, but it doesn't tell that the "integer" mode applies. - Javadoc of SWT_AUTOSCALE is completely wrong. The literal <b>integer:max</b> doesn't make sense, and the rest of the Javadoc doesn't describe the implementation. - Do we have expected use cases for any capping other than 200? If not, then we shouldn't make this more complicated than necessary. We could add support for custom capping values for the other modes, but I don't think we should. The "false" and "<value>" modes are meant for situations where our DPI-detection somehow went wrong. The other modes are mainly to allow quick experiments by SWT developers. In both cases, capping is not really necessary, and a default capping would be confusing. Gerrit change https://git.eclipse.org/r/92021 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=6cdabca59de9a57692b24da180f04cc98307f448 I think I've explained all the tradeoffs and I don't see remaining open issues, so I think it's best to go forward and try the available solution in practice before the 4.7 endgame starts. Pushed my fix, but I'm still open for better proposals. |