Skip to content

Conversation

@ShahzaibIbrahim
Copy link
Contributor

@ShahzaibIbrahim ShahzaibIbrahim commented Dec 10, 2025

This is a preparatory commit to remove all possible usages of getDPI from gtk. Since getDPI is set to be deprecated.

PR for Deprecation: #2872

@ShahzaibIbrahim ShahzaibIbrahim linked an issue Dec 10, 2025 that may be closed by this pull request
1 task
@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

Test Results (linux)

   88 files     88 suites   14m 24s ⏱️
4 557 tests 4 337 ✅ 220 💤 0 ❌
  211 runs    208 ✅   3 💤 0 ❌

Results for commit 49536d1.

♻️ This comment has been updated with latest results.

@jonahgraham
Copy link
Contributor

The GTK3 tests hung after 360m - I don't think that is because of your change and I have raised #2876 to track that

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Having the scale factor being based on the screen DPI leads to unexpected result e.g. Image too big/small. Having a screen dpi independent factor leads to consistent results.

I do not understand how the commit/PR message relates to the actual change. What unexpected results to you refer to? Can you share an example?

And can you explain why it is correct to use a fixed DPI value of 96 in Display, even though the current getDPI() implementation has more sophisticated logic to determine the actual DPI? Should the implementation of getDPI() be changed then, in case the value is wrong?

@ShahzaibIbrahim ShahzaibIbrahim force-pushed the master-224-gtk branch 3 times, most recently from 04f5613 to 7723c4f Compare December 30, 2025 13:04
@ShahzaibIbrahim
Copy link
Contributor Author

Having the scale factor being based on the screen DPI leads to unexpected result e.g. Image too big/small. Having a screen dpi independent factor leads to consistent results.

I do not understand how the commit/PR message relates to the actual change. What unexpected results to you refer to? Can you share an example?

And can you explain why it is correct to use a fixed DPI value of 96 in Display, even though the current getDPI() implementation has more sophisticated logic to determine the actual DPI? Should the implementation of getDPI() be changed then, in case the value is wrong?

After our discussion. I have changed the commit message and its content. Now we only get rid of getDPI usage only where its possible. For Display getDPI could be used for both Device and Printer both and because of this some places the value currently cannot be replaced.

For later: we can deprecate the getDPI method now, but later we can keep the usage to internal by making getDPI package protected.

This is a preparatory commit to remove all **possible** usages of getDPI
from gtk. Since getDPI is set to be deprecated.
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Reusing the already stored DPI value instead of re-retrieving it is fine to avoid unnecessary multiple calls to getDPI().

@HeikoKlare HeikoKlare merged commit 383ed2a into eclipse-platform:master Dec 30, 2025
19 checks passed
@HeikoKlare HeikoKlare deleted the master-224-gtk branch December 30, 2025 15:33
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.

Mark Device::getDPI() method as deprecated

3 participants