Skip to content

Conversation

@sratz
Copy link
Member

@sratz sratz commented Nov 12, 2024

Resolves #373.

I came up with this solution with some google research. It seems to work well, but I'm quite clueless about those Win32 APIs, so would be great if a Win32 expert could help out here.

@sratz sratz marked this pull request as draft November 12, 2024 15:45
@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2024

Test Results

   502 files     502 suites   8m 7s ⏱️
 4 334 tests  4 320 ✅  14 💤 0 ❌
16 575 runs  16 466 ✅ 109 💤 0 ❌

Results for commit 1816af4.

♻️ This comment has been updated with latest results.

@DenisUngemach
Copy link
Contributor

Hi, the undocumented flag PW_RENDERFULLCONTENT seems to be necessary for printWindow.
Also chromium does not really know why:
https://chromium.googlesource.com/chromium/src.git/+/62.0.3178.1/ui/snapshot/snapshot_win.cc

Without that flag PrintWindow may not correctly capture what's actually onscreen.

@sratz sratz requested a review from HeikoKlare December 11, 2024 15:29
@sratz sratz force-pushed the win32-printwindow branch from d3772be to ef3c45a Compare December 11, 2024 15:30
@sratz
Copy link
Member Author

sratz commented Dec 11, 2024

This appears to be working well.

Nobody in Platform is actually calling Control.print (GC), so it's hard to test that more intensively.

My proposal would be to get that in in M1 and hope for (no negative) feedback from downstream consumers elsewhere.

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.

The change sounds reasonable, in particular also considering the Chromium implementation, but I am no expert on this either. We also do not make any use of the print method in our product, so I cannot assist with any testing.

My proposal would be to get that in in M1 and hope for (no negative) feedback from downstream consumers elsewhere.

I agree. Let's try to fix this as early as possible in a release cycle. What about explicitly pinging the people involved in #373 to test the fix and give feedback? Otherwise I think we might not get any (negative) feedback simply because of the lack of testing.

@sratz sratz added the edge Edge Browser label Feb 25, 2025
@sratz sratz force-pushed the win32-printwindow branch from ef3c45a to ba39253 Compare March 5, 2025 07:52
@sratz sratz marked this pull request as ready for review March 5, 2025 07:52
@sratz sratz added this to the 4.36 M1 milestone Mar 5, 2025
@sratz sratz force-pushed the win32-printwindow branch from ba39253 to 1816af4 Compare March 5, 2025 11:43
@sratz sratz requested a review from HeikoKlare March 5, 2025 15:10
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.

Let's give it a try.

@sratz sratz merged commit d42bca6 into eclipse-platform:master Mar 5, 2025
14 checks passed
@sratz sratz deleted the win32-printwindow branch March 5, 2025 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

edge Edge Browser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Edge] Browser with SWT.EDGE returns blank image when using print()

3 participants