Skip to content

feat: support dip <-> screen conversion on Linux X11#46211

Merged
codebytere merged 1 commit intomainfrom
support-dip-screen-conversion-linux
May 2, 2025
Merged

feat: support dip <-> screen conversion on Linux X11#46211
codebytere merged 1 commit intomainfrom
support-dip-screen-conversion-linux

Conversation

@codebytere
Copy link
Copy Markdown
Member

@codebytere codebytere commented Mar 24, 2025 •
edited
Loading

Description of Change

Refs #45848.

Adds support for screen.dipToScreenPoint(point) and screen.screenToDipPoint(point) on Linux X11. There's currently no support for Wayland as the coordinate system is notably more difficult to reconcile with this.

Checklist

Release Notes

Notes: Added support for screen.dipToScreenPoint(point) and screen.screenToDipPoint(point) on Linux X11.

@codebytere codebytere added semver/minor backwards-compatible functionality target/34-x-y PR should also be added to the "34-x-y" branch. target/35-x-y PR should also be added to the "35-x-y" branch. target/36-x-y PR should also be added to the "36-x-y" branch. labels Mar 24, 2025
@codebytere codebytere requested review from ckerr and deepak1556 March 24, 2025 09:06
@codebytere codebytere force-pushed the support-dip-screen-conversion-linux branch from c6d9243 to e75c613 Compare March 24, 2025 10:55
deepak1556
deepak1556 reviewed Mar 24, 2025
View reviewed changes
@ckerr ckerr changed the title feat: support dip <-> screen conversion on Linux feat: support dip <-> screen conversion on Linux X11 Mar 24, 2025
@codebytere codebytere force-pushed the support-dip-screen-conversion-linux branch 4 times, most recently from 81f75e4 to b185eda Compare March 24, 2025 14:38
@codebytere codebytere force-pushed the support-dip-screen-conversion-linux branch 2 times, most recently from d6ef336 to ecfb1ef Compare March 24, 2025 14:41
@codebytere codebytere requested a review from deepak1556 March 24, 2025 14:48
deepak1556
deepak1556 reviewed Mar 24, 2025
View reviewed changes
deepak1556
deepak1556 approved these changes Mar 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

LGTM on the implementation

@codebytere codebytere force-pushed the support-dip-screen-conversion-linux branch from ecfb1ef to cfec681 Compare March 24, 2025 18:32
@ckerr
Copy link
Copy Markdown
Member

ckerr commented Mar 25, 2025

In linux-x64 CI:

2025-03-24T23:02:48.6851013Z ../../electron/shell/browser/api/electron_api_screen.cc:145:38: error: invalid operands to binary expression ('const Point' and 'gfx::Vector2dF')
2025-03-24T23:02:48.6851678Z 145 | return display.bounds().origin() + delta_dip;
2025-03-24T23:02:48.6851987Z | ~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~

@codebytere codebytere force-pushed the support-dip-screen-conversion-linux branch from cfec681 to 16728a9 Compare March 25, 2025 22:09
@ckerr
Copy link
Copy Markdown
Member

ckerr commented Mar 26, 2025 •
edited
Loading

This old build error again 😸

2025-03-25T22:32:34.7353259Z ld.lld: error: duplicate symbol: x11_util::IsX11()
2025-03-25T22:32:34.7353686Z >>> defined at x11_util.h:12 (../../electron/shell/browser/linux/x11_util.h:12)
2025-03-25T22:32:34.7354140Z >>> obj/electron/electron_lib/electron_api_screen.o:(x11_util::IsX11())
2025-03-25T22:32:34.7354567Z >>> defined at x11_util.h:12 (../../electron/shell/browser/linux/x11_util.h:12)
2025-03-25T22:32:34.7354977Z >>> obj/electron/electron_lib/native_window_views.o:(.text+0x0)

Even though it's a simple function, probably best to move the impl into a .cc file.

If you do go that route, x11_util.h won't need to #include anything either

samuelmaddock
samuelmaddock reviewed Mar 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@samuelmaddock samuelmaddock left a comment

Choose a reason for hiding this comment

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

API LGTM

Minor request about clarifying behavior on Wayland.

itsananderson
itsananderson reviewed Mar 27, 2025
View reviewed changes
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Mar 31, 2025
@codebytere codebytere force-pushed the support-dip-screen-conversion-linux branch from 5b4f975 to a0f413a Compare April 22, 2025 09:04
@release-clerk
Copy link
Copy Markdown

release-clerk bot commented May 2, 2025

Release Notes Persisted

Added support for screen.dipToScreenPoint(point) and screen.screenToDipPoint(point) on Linux X11.

@trop
Copy link
Copy Markdown
Contributor

trop bot commented May 2, 2025

I was unable to backport this PR to "34-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Copy Markdown
Contributor

trop bot commented May 2, 2025

I was unable to backport this PR to "35-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot removed the target/34-x-y PR should also be added to the "34-x-y" branch. label May 2, 2025
@trop
Copy link
Copy Markdown
Contributor

trop bot commented May 2, 2025

I was unable to backport this PR to "36-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot added needs-manual-bp/34-x-y and removed target/35-x-y PR should also be added to the "35-x-y" branch. labels May 2, 2025
@trop trop bot mentioned this pull request May 2, 2025
Merged
@trop trop bot added needs-manual-bp/35-x-y needs-manual-bp/36-x-y and removed target/36-x-y PR should also be added to the "36-x-y" branch. labels May 2, 2025
@trop
Copy link
Copy Markdown
Contributor

trop bot commented May 2, 2025

I have automatically backported this PR to "37-x-y", please check out #46895

@trop trop bot added in-flight/37-x-y merged/37-x-y PR was merged to the "37-x-y" branch. and removed target/37-x-y PR should also be added to the "37-x-y" branch. in-flight/37-x-y labels May 2, 2025
@ckerr ckerr mentioned this pull request May 13, 2025
Merged
3 tasks
@deepak1556 deepak1556 mentioned this pull request May 16, 2025
Merged
@trop
Copy link
Copy Markdown
Contributor

trop bot commented May 16, 2025

@deepak1556 has manually backported this PR to "35-x-y", please check out #47124

@deepak1556 deepak1556 mentioned this pull request May 16, 2025
Merged
@trop
Copy link
Copy Markdown
Contributor

trop bot commented May 16, 2025

@deepak1556 has manually backported this PR to "36-x-y", please check out #47125

@trop trop bot added in-flight/36-x-y merged/36-x-y PR was merged to the "36-x-y" branch. merged/35-x-y PR was merged to the "35-x-y" branch. and removed needs-manual-bp/36-x-y in-flight/36-x-y in-flight/35-x-y labels May 16, 2025
kigh-ota pushed a commit to kigh-ota/electron that referenced this pull request Sep 30, 2025
@codebytere @kigh-ota
feat: support dip <-> screen conversion on Linux X11 (electron#46211)
7169acf 
feat: support dip <-> screen conversion on Linux
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkleinsc jkleinsc jkleinsc left review comments

@ckerr ckerr ckerr approved these changes

@deepak1556 deepak1556 deepak1556 approved these changes

@samuelmaddock samuelmaddock Awaiting requested review from samuelmaddock

@itsananderson itsananderson Awaiting requested review from itsananderson

Assignees

No one assigned

Labels

api-review/approved ✅ merged/35-x-y PR was merged to the "35-x-y" branch. merged/36-x-y PR was merged to the "36-x-y" branch. merged/37-x-y PR was merged to the "37-x-y" branch. needs-manual-bp/34-x-y semver/minor backwards-compatible functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@codebytere @ckerr @itsananderson @jkleinsc @deepak1556 @samuelmaddock