Bug 569233 - [GTK][HiDpi] Remove non cairo scale path
Summary: [GTK][HiDpi] Remove non cairo scale path
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.18   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-SWT-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-27 02:18 EST by Sravan Kumar Lakkimsetti CLA
Modified: 2021-01-11 13:35 EST (History)
2 users (show)

See Also:


Attachments
Screenshots with different desktop environments (1.59 MB, application/x-zip-compressed)
2020-11-27 10:40 EST, Sravan Kumar Lakkimsetti CLA
no flags Details
Modified snippet used for performance test (8.10 KB, application/octet-stream)
2020-12-10 02:39 EST, Sravan Kumar Lakkimsetti CLA
no flags Details
Fixed snippet for perf testing (9.04 KB, text/plain)
2020-12-10 20:31 EST, Alexandr Miloslavskiy CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sravan Kumar Lakkimsetti CLA 2020-11-27 02:18:41 EST
Remove auto scaling of coordinates and sizes in the non cairo scaling path.

Currently we are using cairo for image operations we don't use non cairo scale path now. 

Its better to remove that path
Comment 1 Sravan Kumar Lakkimsetti CLA 2020-11-27 10:40:18 EST
Created attachment 284917 [details]
Screenshots with different desktop environments
Comment 2 Eclipse Genie CLA 2020-11-27 10:43:33 EST
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172943
Comment 3 Sravan Kumar Lakkimsetti CLA 2020-12-10 02:39:18 EST
Created attachment 285012 [details]
Modified snippet used for performance test
Comment 4 Alexandr Miloslavskiy CLA 2020-12-10 20:31:19 EST
Created attachment 285019 [details]
Fixed snippet for perf testing

I have updated the snippet to do proper testing:
1) Added warmup iterations
2) Multiple iterations are measured and averaged
3) SWT.TOOL style is added to avoid focus switches between applications
4) Things are properly cleaned between iterations

With these changes, also running on top of Bug 569647, I see 0.10 sec both with and without patch. I conclude that performance difference, if any, is hard to measure.
Comment 5 Sravan Kumar Lakkimsetti CLA 2020-12-11 03:32:10 EST
(In reply to Alexandr Miloslavskiy from comment #4)
> Created attachment 285019 [details]
> Fixed snippet for perf testing
> 
> I have updated the snippet to do proper testing:
> 1) Added warmup iterations
> 2) Multiple iterations are measured and averaged
> 3) SWT.TOOL style is added to avoid focus switches between applications
> 4) Things are properly cleaned between iterations
> 
> With these changes, also running on top of Bug 569647, I see 0.10 sec both
> with and without patch. I conclude that performance difference, if any, is
> hard to measure.

If there is not performance improvement this this patch will become cleanup patch. lets target this in 4.24 when ubuntu 18.04 will become nearly obsolete.

Thank you for your review
Comment 6 Alexandr Miloslavskiy CLA 2020-12-14 14:50:41 EST
OK, this sounds reasonable.

I wonder if the problem on 18.04 can be fixed without much pain?
The simplest change could be to use this formula for DPI:
  gdk_monitor_get_scale_factor() * gdk_screen_get_resolution() / 96.0

This may already be enough for most cases.
Comment 7 Sravan Kumar Lakkimsetti CLA 2020-12-18 00:25:56 EST
Can you create a patch. I will do the requisite testing.
Comment 8 Sravan Kumar Lakkimsetti CLA 2020-12-18 02:39:40 EST
https://git.eclipse.org/c/platform/eclipse.platform.swt.git/tree/bundles/org.eclipse.swt/Eclipse%20SWT/gtk/org/eclipse/swt/graphics/Device.java#n688 is the location where we determine the dpi and select whether to scale all method or use cairo scaling method.

Can you try passing gdk_monitor_get_scale_factor() * gdk_screen_get_resolution() * 100/ 96.0 to DPIUtil.setDeviceZoom method?
Comment 9 Alexandr Miloslavskiy CLA 2021-01-06 09:36:30 EST
Was on vacation, will hopefully try it this or next week.
Comment 10 Alexandr Miloslavskiy CLA 2021-01-11 13:35:06 EST
I found that the old branch in 'Device.getDeviceZoom()' already does exactly that. This explains why things briefly worked fine on KUbuntu 18.04 before Bug 539392.

However, since then, it was broken again in a series of various hacks.

What needs to be done on systems like KUbuntu 18.04 is:
1) Don't scale Controls and Fonts
   This is kind of similar to 'DPIUtil.setUseCairoAutoScale(true)'.
   Also, Controls should observe that their DPI matches DPI of screen.
2) Scale Images.

It seems that existing SWT hacks can't cover this combination.