Skip to content

Conversation

@arunjose696
Copy link
Contributor

@arunjose696 arunjose696 commented Jul 10, 2025

Previously, stringExtent calculated the width of a string in pixels using GC.getZoom(), which could lead to inconsistencies if the provided font had a different zoom level. This change ensures that the pixels-to-points conversion uses the zoom of the specified font.

Fixes #2311

Reproduction Steps
On a 100% monitor, set these parameters:

-Dswt.enable.autoScale=true
-Dswt.autoScale=150
-Dswt.autoScale.method=nearest

Screenshots


Current Behaviour

With This Change

Have tested the changes on zooms 100,150,200&225 with -Dswt.autoScale=150 ,100 &200,

Previously, stringExtent calculated the width of a string in pixels using GC.getZoom(), which could lead to inconsistencies if the provided font had a different zoom level. This change ensures that the pixels-to-points conversion uses the zoom of the specified font.
@github-actions
Copy link
Contributor

Test Results

   545 files  ±0     545 suites  ±0   27m 13s ⏱️ - 2m 37s
 4 406 tests ±0   4 389 ✅ ±0   17 💤 ±0  0 ❌ ±0 
16 745 runs  ±0  16 607 ✅ ±0  138 💤 ±0  0 ❌ ±0 

Results for commit ad550fe. ± Comparison against base commit 648a444.

Copy link
Member

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution, @arunjose696 !

In general it looks good and I couldn't find any regressions with this PR. I just have a question regarding the necessity of one change.

Also, have you by any chance tried to make the font in the line number ruler smaller so that it has the same size of the code? I assume this would mean that the height of lines in the code would also change, making the code look more compact (which I like) but this is just a thought.

Copy link
Contributor

@ShahzaibIbrahim ShahzaibIbrahim left a comment

Choose a reason for hiding this comment

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

The changes looks good to me. I also tested it with quarter autoScale (175% -> 150%). Functionality is as expected.

@arunjose696
Copy link
Contributor Author

arunjose696 commented Jul 11, 2025

Also, have you by any chance tried to make the font in the line number ruler smaller so that it has the same size of the code? I assume this would mean that the height of lines in the code would also change, making the code look more compact (which I like) but this is just a thought.

I tried this, but it turned out to be a bit tricky. The GC used to draw the line number ruler is created from a BufferedImage(which is a Image type), and it inherits the font zoom level from the last requested zoom for the image. This zoom is used by the GC for drawing. I haven’t found a straightforward way to reset the nativeZoom value of a GC that’s created from an image.

For experimentation, I was able to make this work on the first draw of the line number ruler by explicitly calling BufferedImage.getImageData(LOWER_ZOOM) just before the GC was created, which as a side effect sets the currentZoom of the image to LOWER_ZOOM. So when the GC is created from that image, it uses the desired zoom. However, later when switching monitors, SWT will request a new image using the zoom specified by swt.autoScale.

I feel this route would be much more complicated compared to current approach.

@arunjose696 arunjose696 removed their assignment Jul 11, 2025
@fedejeanne fedejeanne merged commit 756e8d8 into eclipse-platform:master Jul 14, 2025
17 checks passed
@fedejeanne fedejeanne deleted the arunjose696/3061/fontsStringExtent branch July 14, 2025 07:00
@fedejeanne
Copy link
Member

I see. Thank you for the fix and the thorough response, @arunjose696 !

If the size of the font in the line number ruler becomes an issue then we can circle back to the analysis. But I have no strong opinion about it and there are other things in more dire need of attention right now. I accepted your fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants