Skip to content

Conversation

@akoch-yatta
Copy link
Contributor

@akoch-yatta akoch-yatta commented Apr 2, 2025

This PR unifies the extraction of the target zoom of a font to be
affected by the zoom context of the underlying Widget. Before there
were different strategies like getting it from the Shell.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2025

Test Results

   545 files  ±0     545 suites  ±0   27m 36s ⏱️ - 1m 11s
 4 373 tests ±0   4 355 ✅ ±0   18 💤 ±0  0 ❌ ±0 
16 634 runs  ±0  16 496 ✅ ±0  138 💤 ±0  0 ❌ ±0 

Results for commit a32a561. ± Comparison against base commit 62c2cb0.

♻️ This comment has been updated with latest results.

@akoch-yatta akoch-yatta marked this pull request as draft April 3, 2025 06:54
@akoch-yatta akoch-yatta force-pushed the always-use-fontprovider branch 2 times, most recently from 21bf53f to be9b308 Compare April 3, 2025 10:42
@akoch-yatta akoch-yatta changed the title [win32] Prefer SWTFontProvider over Font#win32_new [win32] Use consistent font zoom retrieval Apr 3, 2025
@akoch-yatta akoch-yatta marked this pull request as ready for review April 3, 2025 11:11
@akoch-yatta akoch-yatta linked an issue Apr 4, 2025 that may be closed by this pull request
Copy link
Contributor

@amartya4256 amartya4256 left a comment

Choose a reason for hiding this comment

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

Looks rather cleaner to me. Also using a getter makes it more abstract for the consumers since they don't need to worry about which component's nativeZoom they should use. Approved.

return GC.win32_new(hDC, data);
}

int getFontZoom() {
Copy link
Contributor

@amartya4256 amartya4256 Apr 23, 2025

Choose a reason for hiding this comment

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

Just a suggestion, can we not call it getNativeZoom? Since, I am not sure if getFontZoom would make sense for every kind of widget + the method name sounds more relevant to what it returns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, yes, makes sense. It is solely used for fonts right now, but that method would not be limited to this purpose. I will rename it.

@akoch-yatta akoch-yatta force-pushed the always-use-fontprovider branch from be9b308 to 0f4b54f Compare April 23, 2025 13:24
This commit unifies the extraction of the target zoom of a font to be
affected by the zoom context of the underlying Widget. Before there
were different strategies like getting it from the Shell.
@HeikoKlare HeikoKlare force-pushed the always-use-fontprovider branch from 0f4b54f to a32a561 Compare April 24, 2025 15:32
@HeikoKlare
Copy link
Contributor

Version increment check is failing for infrastructure reasons and no further version bump is required here.

@HeikoKlare HeikoKlare merged commit 033d733 into eclipse-platform:master Apr 24, 2025
9 of 10 checks passed
@HeikoKlare HeikoKlare deleted the always-use-fontprovider branch April 24, 2025 15:51
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.

Use consistent font zoom retrieval

3 participants