Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minibrowser: implement HiDPI support #30343

Merged
merged 1 commit into from Sep 14, 2023
Merged

Conversation

delan
Copy link
Member

@delan delan commented Sep 12, 2023

This patch makes the minibrowser behave correctly under HiDPI and mixed-dpi environments, by plumbing the servoshell Window’s scale factor into egui during context creation and on ScaleFactorChanged events.

  • 694349f exposes our calculated scale factor as Window­Ports­Methods­::hidpi­_factor
  • bc5ff50 plumbs that into egui during context creation
  • a9e9c9c multiplies that by the scale factor in Window­::get­_coordinates, which affects where Servo and WebRender will put the viewport, while marking the toolbar_height fields as DeviceIndependentPixel to prevent future regressions
  • 69138c0 multiplies toolbar_height by the scale factor in the CursorMoved handler, which affects the relative coordinates of input events, while using euclid’s Size2D and Length types to prevent future regressions
  • b90e315 intercepts Scale­Factor­Changed events so we can set egui’s scale factor manually, because we don’t always want to use the device scale factor

To test this manually:

  1. Run Servo on a HiDPI monitor with RUST_LOG=warn,servo::app=info
  2. Go to data:text/html,<body style=margin:0>BODY and check that:
    1. text in the toolbar is at a reasonable size and not too small
    2. text in the page is at the size you would expect for unstyled text
    3. none of the word “BODY” is cut off by the toolbar
  3. If you have a mixed-dpi setup, move the window to your other monitor and:
    1. check the same things as above
    2. check that the logs contain [INFO] window scale factor changed to <new dpi>, setting scale factor to <new dpi>
  4. Repeat with --device-pixel-ratio 1 and check that:
    1. text in the toolbar and page is too small on your HiDPI monitor
    2. those log messages always end with “setting scale factor to 1”

Test results:

  • ☑️ X11 with Xft.dpi: {96,192} (note that X11 doesn’t seem to support mixed-dpi)
  • ☑️ Wayland with wlr-randr --output DP-2 --scale 1 --output DP-3 --scale 2
  • macOS not yet tested, please help!
  • ☑️ Windows build 19045 with one 100% monitor and one 200% monitor

  • There are tests for these changes OR
  • These changes do not require tests because the minibrowser is not currently testable, but euclid’s type safety is now used where possible

@delan delan mentioned this pull request Sep 12, 2023
27 tasks
@delan delan force-pushed the servoshell-clarify-dpr-override branch from 1e9f3f6 to e32ce0e Compare September 12, 2023 09:04
@delan delan changed the base branch from servoshell-clarify-dpr-override to servoshell-dip-n-dp September 12, 2023 09:20
@delan delan force-pushed the minibrowser-hidpi-support branch 2 times, most recently from f546afd to b90e315 Compare September 13, 2023 05:17
@delan delan marked this pull request as ready for review September 13, 2023 07:14
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

This looks great. Just a couple comments below.

ports/servoshell/app.rs Outdated Show resolved Hide resolved
ports/servoshell/app.rs Outdated Show resolved Hide resolved
ports/servoshell/headless_window.rs Outdated Show resolved Hide resolved
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Thanks!

@sagudev
Copy link
Member

sagudev commented Sep 13, 2023

let's run try build so I do not need to build it myself to test windows

@bors-servo try=all

@github-actions
Copy link

🤔 Unknown try string 'try=all'

@jdm
Copy link
Member

jdm commented Sep 13, 2023

@bors-servo try

@github-actions
Copy link

🔨 Triggering try run (#6174746797) with platform=all and layout=all

@github-actions
Copy link

Test results for linux-wpt-layout-2013 from try job (#6174746797):

Flaky unexpected result (21)
  • TIMEOUT [expected ERROR] /_webgl/conformance2/textures/image/tex-3d-rgb9_e5-rgb-half_float.html
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/CSS2/linebox/inline-negative-margin-001.html (#23862)
    • FAIL [expected PASS] subtest: [data-expected-height] 1 assert_equals:
      <inline-block data-expected-height="10">123 <span style="margin-left: -8ch">1234 </span></inline-block>
      height expected 10 but got 20
    • FAIL [expected PASS] subtest: [data-expected-height] 2 assert_equals:
      <inline-block data-expected-height="10">123 <span style="margin-left: -8ch">123 </span></inline-block>
      height expected 10 but got 20
    • PASS [expected FAIL] subtest: [data-expected-height] 4
  • OK /css/css-flexbox/text-as-flexitem-size-001.html (#28726)
    • FAIL [expected PASS] subtest: .flexbox > div 1 assert_equals:
      <div data-expected-width="70" data-expected-height="35">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 2 assert_equals:
      <div data-expected-width="50" data-expected-height="45">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 50 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 5 assert_equals:
      <div style="height: 30px" data-expected-width="70" data-expected-height="30">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 8 assert_equals:
      <div style="min-height: 40px" data-expected-width="70" data-expected-height="40">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
    • FAIL [expected PASS] subtest: .flexbox > div 11 assert_equals:
      <div style="max-height: 20px" data-expected-width="70" data-expected-height="20">
      <p>xx xxx</p>
      <p>xx</p>
      </div>
      width expected 70 but got 46
  • TIMEOUT [expected FAIL] /css/css-text/white-space/white-space-pre-051.html
  • TIMEOUT [expected PASS] /css/css-variables/variable-reference-visited.html
  • OK /css/cssom-view/offsetTopLeft-border-box.html (#24237)
    • FAIL [expected PASS] subtest: container: 0 assert_equals: offsetTop expected 2 but got 0
    • FAIL [expected PASS] subtest: container: 1 assert_equals: offsetTop expected 2 but got 0
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
    • PASS [expected FAIL] subtest: Navigating to a different document with link click
    • TIMEOUT [expected FAIL] subtest: Navigating to a different document with form submission Test timed out
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
    • PASS [expected FAIL] subtest: load event does not fire on window.open('about:blank')
  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-referrer.window.html (#29081)
    • PASS [expected TIMEOUT] subtest: no-referrer referrer policy used to create the starting page
  • TIMEOUT [expected OK] /html/browsers/origin/cross-origin-objects/cross-origin-objects.html (#28569)
  • OK /html/browsers/the-window-object/open-close/creating_browsing_context_test_01.html (#29046)
  • CRASH [expected FAIL] /html/canvas/element/manual/text/canvas.2d.disconnected.html (#30063)
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • TIMEOUT [expected OK] /html/semantics/forms/form-submission-0/reparent-form-during-planned-navigation-task.html (#29724)
    • TIMEOUT [expected PASS] subtest: reparent-form-during-planned-navigation-task Test timed out
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: Basic test (formdata event)
  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-delayed.html (#27659)
    • FAIL [expected PASS] subtest: async document.write in a module assert_true: onload must be called expected true got false
  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-static-import-delayed.html (#26243)
    • PASS [expected FAIL] subtest: document.write in an imported module
  • TIMEOUT [expected OK] /html/webappapis/scripting/processing-model-2/unhandled-promise-rejections/promise-rejection-events.html (#26371)
    • TIMEOUT [expected FAIL] subtest: delayed handling: delaying handling rejected promise created from createImageBitmap will cause both events to fire Test timed out
  • TIMEOUT [expected OK] /webaudio/the-audio-api/the-audiocontext-interface/audiocontext-not-fully-active.html (#27664)
  • TIMEOUT [expected OK] /webmessaging/with-ports/017.html (#24486)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank Test timed out
  • OK [expected TIMEOUT] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.tentative.html (#29053)
    • PASS [expected TIMEOUT] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe
Stable unexpected results that are known to be intermittent (16)
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/css-fonts/variations/font-weight-matching.html (#20686)
    • PASS [expected FAIL] subtest: Test @font-face matching for weight 420
  • OK /css/cssom-view/elementFromPoint-list-001.html (#23915)
    • PASS [expected FAIL] subtest: <li>Outside 1</li>
  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • PASS [expected TIMEOUT] subtest: background-image sec-fetch-dest - Not sent to non-trustworthy same-site destination
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src=''
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='about:blank'
  • OK /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling-dynamic.html (#28066)
    • PASS [expected FAIL] subtest: 0080 00FF set in href="" targeting a frame and clicked
  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted assert_array_equals: Pages opened during history navigation expected property 1 to be 5 but got 3 (expected array [6, 5] got [6, 3])
  • OK /html/browsers/history/the-location-interface/location-protocol-setter.html (#20839)
    • PASS [expected FAIL] subtest: Equivalent tests for data URL and srcdoc <iframe>s
  • OK [expected CRASH] /html/canvas/offscreen/the-offscreen-canvas/size.attributes.parse.minus.worker.html (#30164)
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • OK /html/semantics/embedded-content/the-img-element/non-active-document.html (#21544)
    • PASS [expected FAIL] subtest: createHTMLDocument
    • PASS [expected FAIL] subtest: <template>
  • OK /html/syntax/parsing/DOMContentLoaded-defer.html (#21550)
    • PASS [expected FAIL] subtest: The end: DOMContentLoaded and defer scripts
  • PASS [expected CRASH] /streams/readable-streams/crashtests/strategy-worker-terminate.html (#30124)
  • OK [expected TIMEOUT] /webmessaging/with-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
  • TIMEOUT [expected OK] /webmessaging/without-ports/017.html (#24486)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank Test timed out
  • ERROR [expected OK] /workers/constructors/Worker/Worker-constructor.html (#22991)

@github-actions
Copy link

Test results for linux-wpt-layout-2020 from try job (#6174746797):

Flaky unexpected result (18)
  • OK /_mozilla/mozilla/task_queue_throttling.any.html (#22519)
    • FAIL [expected PASS] subtest: Throttling the performance timeline task queue. assert_true: expected true got false
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-with-non-reserved-words.html (#16216)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/CSS2/linebox/inline-negative-margin-001.html (#23862)
    • FAIL [expected PASS] subtest: [data-expected-height] 1 assert_equals:
      <inline-block data-expected-height="10">123 <span style="margin-left: -8ch">1234 </span></inline-block>
      height expected 10 but got 20
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • PASS [expected FAIL] subtest: sec-fetch-site - Not sent to non-trustworthy same-origin destination, no attributes
  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src=''
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='about:blank'
  • TIMEOUT [expected FAIL] /html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-separate.html
  • OK /html/syntax/parsing/DOMContentLoaded-defer.html (#21550)
    • FAIL [expected PASS] subtest: The end: DOMContentLoaded and defer scripts assert_false: DOMContentLoaded should not have fired before executing a task queued from a defer script expected false got true
  • TIMEOUT /html/webappapis/scripting/events/compile-event-handler-settings-objects.html (#24246)
    • PASS [expected FAIL] subtest: The entry settings object while executing the compiled callback via Web IDL's invoke must be that of the node document
  • TIMEOUT [expected OK] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry.html (#25805)
    • TIMEOUT [expected FAIL] subtest: Fulfillment handler on pending-then-fulfilled promise Test timed out
    • TIMEOUT [expected FAIL] subtest: Rejection handler on pending-then-rejected promise Test timed out
  • ERROR /resource-timing/content-type-parsing.html (#29131)
    • FAIL [expected TIMEOUT] subtest: mime-type 16 : text/html;charset=�gbk assert_equals: expected (string) "text/html" but got (undefined) undefined
    • TIMEOUT [expected NOTRUN] subtest: mime-type 17 : text/html;charset= gbk Test timed out
  • TIMEOUT /resource-timing/nested-context-navigations-iframe.html (#24311)
    • TIMEOUT [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent, even after history navigations by the parent Test timed out
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent, even after history navigations by the parent
    • NOTRUN [expected PASS] subtest: Test that iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent
    • NOTRUN [expected TIMEOUT] subtest: Test that iframe refreshes are not observable by the parent
  • OK /resource-timing/status-codes-create-entry.html (#28675)
    • PASS [expected FAIL] subtest: Make sure all status codes are reported
  • TIMEOUT [expected OK] /wasm/jsapi/functions/entry.html (#27087)
    • TIMEOUT [expected FAIL] subtest: Start function Test timed out
  • OK [expected TIMEOUT] /webmessaging/without-ports/017.html (#24486)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, about:blank
  • ERROR [expected OK] /workers/semantics/run-a-worker/003.html (#22765)
Stable unexpected results that are known to be intermittent (15)
  • OK [expected TIMEOUT] /css/css-flexbox/abspos/position-absolute-013.html (#28405)
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • PASS [expected FAIL] subtest: Matching font-weight: '430' should prefer '450 460' over '500'
    • FAIL [expected PASS] subtest: Matching font-weight: '430' should prefer '400 425' over '350 399' assert_equals: Unexpected font on test element expected 487 but got 532
    • PASS [expected FAIL] subtest: Matching font-weight: '430' should prefer '340 398' over '501 550'
    • FAIL [expected PASS] subtest: Matching font-weight: '500' should prefer '500' over '450 460' assert_equals: Unexpected font on test element expected 487 but got 532
    • FAIL [expected PASS] subtest: Matching font-weight: '501' should prefer '501' over '502 510' assert_equals: Unexpected font on test element expected 487 but got 532
    • FAIL [expected PASS] subtest: Matching font-weight: '501' should prefer '450 460' over '390 410' assert_equals: Unexpected font on test element expected 487 but got 532
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '350 399' over '340 360'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '200 300' over '400'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '450 460' over '500 501'
    • PASS [expected FAIL] subtest: Matching font-stretch: '110%' should prefer '110% 120%' over '115% 116%'
    • And 16 more unexpected results...
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-load-as-html.xhtml (#29070)
    • TIMEOUT [expected PASS] subtest: javascript: URL navigation to a string must create a HTML document using the correct properties Test timed out
  • OK /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling-dynamic.html (#28066)
    • FAIL [expected PASS] subtest: D83D DE0D set in href="" targeting a frame and clicked assert_equals: expected "😍" but got ""
    • FAIL [expected PASS] subtest: DE0D 0041 set in href="" targeting a frame and clicked assert_equals: expected "\ufffdA" but got ""
  • OK /html/browsers/history/the-location-interface/location-protocol-setter.html (#20839)
    • PASS [expected FAIL] subtest: Equivalent tests for data URL and srcdoc <iframe>s
  • OK [expected TIMEOUT] /html/browsers/origin/cross-origin-objects/cross-origin-objects.html (#28569)
  • OK /html/browsers/the-window-object/open-close/creating_browsing_context_test_01.html (#29046)
  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/document-with-fragment-top.html (#28259)
    • FAIL [expected TIMEOUT] subtest: Autofocus elements in top-level browsing context's documents with "top" fragments should work. assert_not_equals: got disallowed value Element node <body></body>
  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/skip-another-top-level-browsing-context.html (#24161)
    • TIMEOUT [expected PASS] subtest: Autofocus elements queued in another top-level browsing context's documents should be skipped. Test timed out
  • OK /html/semantics/forms/form-submission-0/form-submit-iframe-then-location-navigate.html (#29634)
    • PASS [expected FAIL] subtest: Verifies that location navigations take precedence when following form submissions.
  • OK /html/semantics/forms/historical.html (#28568)
    • PASS [expected FAIL] subtest: <input name=isindex> should not be supported
  • OK /html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html (#23205)
    • FAIL [expected PASS] subtest: Check that rel=noopener with target=_self does a normal load this.openedWindow.findLink is not a function
  • OK [expected TIMEOUT] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry-different-function-realm.html (#25805)
  • PASS [expected CRASH] /streams/readable-streams/crashtests/strategy-worker-terminate.html (#30124)
  • TIMEOUT [expected OK] /webmessaging/without-ports/018.html (#24485)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, javascript: Test timed out

@github-actions
Copy link

✨ Try run (#6174746797) succeeded.

@sagudev
Copy link
Member

sagudev commented Sep 13, 2023

I tested on my windows and something is working, but there is no logging and on mixed-dpi setup there is no change (blurred on hi-dpi monitor).

@mrobinson mrobinson mentioned this pull request Sep 14, 2023
5 tasks
@delan
Copy link
Member Author

delan commented Sep 14, 2023

I tested on my windows and something is working, but there is no logging and on mixed-dpi setup there is no change (blurred on hi-dpi monitor).

Thanks for testing this! It looks like there’s no logging in the Windows try artifacts, because our Windows release builds use the Windows subsystem. With a debug build and RUST_LOG=warn,servo::app and moving the window from a 1x monitor to a 2x monitor, it looks like winit isn’t giving us a ScaleFactorChanged event for some reason, only Moved and Resized:

[2023-09-14T10:02:42Z TRACE servo::app] @12.6279403s (+28.6µs) WindowEvent { window_id: WindowId(WindowId(2689246)), event: ModifiersChanged(SHIFT) }
[2023-09-14T10:02:42Z TRACE servo::app] @12.6281527s (+212.4µs) WindowEvent { window_id: WindowId(WindowId(2689246)), event: KeyboardInput { device_id: DeviceId(DeviceId(0)), input: KeyboardInput { scancode: 42, state: Pressed, virtual_keycode: Some(LShift), modifiers: SHIFT }, is_synthetic: false } }
[2023-09-14T10:02:42Z TRACE servo::app] @12.6288537s (+7.6µs) RedrawRequested(WindowId(WindowId(2689246)))
[2023-09-14T10:02:42Z TRACE servo::app] RedrawRequested
[2023-09-14T10:02:42Z TRACE servo::app] need_recomposite
[2023-09-14T10:02:42Z ERROR egui_glow] GL error, at C:\Users\delan\.cargo\registry\src\github.com-1ecc6299db9ec823\egui_glow-0.22.0\src\painter.rs:284 (FRAMEBUFFER_SRGB): GL_INVALID_ENUM (0x500). Please file a bug at https://github.com/emilk/egui/issues
[2023-09-14T10:02:42Z TRACE servo::app] PumpResult::ReadyToPresent
[2023-09-14T10:02:42Z TRACE servo::app] @12.6356575s (+23.2µs) WindowEvent { window_id: WindowId(WindowId(2689246)), event: ModifiersChanged(SHIFT | LOGO) }
[2023-09-14T10:02:42Z TRACE servo::app] @12.6357864s (+128.9µs) WindowEvent { window_id: WindowId(WindowId(2689246)), event: KeyboardInput { device_id: DeviceId(DeviceId(0)), input: KeyboardInput { scancode: 57435, state: Pressed, virtual_keycode: Some(LWin), modifiers: SHIFT | LOGO }, is_synthetic: false } }
[2023-09-14T10:02:42Z TRACE servo::app] @12.6359375s (+4.2µs) RedrawRequested(WindowId(WindowId(2689246)))
[2023-09-14T10:02:42Z TRACE servo::app] RedrawRequested
[2023-09-14T10:02:42Z TRACE servo::app] need_recomposite
[2023-09-14T10:02:42Z ERROR egui_glow] GL error, at C:\Users\delan\.cargo\registry\src\github.com-1ecc6299db9ec823\egui_glow-0.22.0\src\painter.rs:284 (FRAMEBUFFER_SRGB): GL_INVALID_ENUM (0x500). Please file a bug at https://github.com/emilk/egui/issues
[2023-09-14T10:02:42Z TRACE servo::app] PumpResult::ReadyToPresent
[2023-09-14T10:02:43Z TRACE servo::app] @13.0215462s (+13.7µs) WindowEvent { window_id: WindowId(WindowId(2689246)), event: Moved(PhysicalPosition { x: 2567, y: 28 }) }
[2023-09-14T10:02:43Z TRACE servo::app] @13.0216566s (+110.4µs) WindowEvent { window_id: WindowId(WindowId(2689246)), event: Resized(PhysicalSize { width: 277, height: 740 }) }
[2023-09-14T10:02:43Z TRACE servo::app] PumpResult::Resize
[2023-09-14T10:02:43Z ERROR egui_glow] GL error, at C:\Users\delan\.cargo\registry\src\github.com-1ecc6299db9ec823\egui_glow-0.22.0\src\painter.rs:284 (FRAMEBUFFER_SRGB): GL_INVALID_ENUM (0x500). Please file a bug at https://github.com/emilk/egui/issues
[2023-09-14T10:02:43Z TRACE servo::app] PumpResult::ReadyToPresent
[2023-09-14T10:02:43Z TRACE servo::app] @13.0684782s (+2.7606ms) RedrawRequested(WindowId(WindowId(2689246)))
[2023-09-14T10:02:43Z TRACE servo::app] RedrawRequested
[2023-09-14T10:02:43Z TRACE servo::app] need_recomposite
[2023-09-14T10:02:43Z ERROR egui_glow] GL error, at C:\Users\delan\.cargo\registry\src\github.com-1ecc6299db9ec823\egui_glow-0.22.0\src\painter.rs:284 (FRAMEBUFFER_SRGB): GL_INVALID_ENUM (0x500). Please file a bug at https://github.com/emilk/egui/issues

@delan
Copy link
Member Author

delan commented Sep 14, 2023

winit’s source suggests that it delivers ScaleFactorChanged in response to WM_DPICHANGED, but even with a modified version of Raymond Chen’s scratch program, it looks like Windows isn’t giving us WM_DPICHANGED when I move the window between monitors with different dpi, because I don’t get a MessageBox:

// build with: cl scratch.cc

// https://stackoverflow.com/q/27613278
#pragma comment (lib, "user32.lib")
#pragma comment (lib, "ole32.lib")
#pragma comment (lib, "comctl32.lib")

// https://gist.github.com/nanoant/dde75805132561140ed5e38f4048f5c1#file-win32directwrite-cpp-L146
#define HANDLE_WM_DPICHANGED(hwnd, wParam, lParam, fn) ((fn)((hwnd), LOWORD(wParam), HIWORD(wParam), *((RECT*)(lParam))), 0L)

#define STRICT
#include <windows.h>
#include <windowsx.h>
#include <ole2.h>
#include <commctrl.h>
#include <shlwapi.h>
HINSTANCE g_hinst;
BOOL
OnCreate(HWND hwnd, LPCREATESTRUCT lpcs)
{
    return TRUE;
}
void
OnDestroy(HWND hwnd)
{
    PostQuitMessage(0);
}
void
PaintContent(HWND hwnd, PAINTSTRUCT *pps)
{
}
void
OnPaint(HWND hwnd)
{
    PAINTSTRUCT ps;
    BeginPaint(hwnd, &ps);
    PaintContent(hwnd, &ps);
    EndPaint(hwnd, &ps);
}
void
OnPrintClient(HWND hwnd, HDC hdc)
{
    PAINTSTRUCT ps;
    ps.hdc = hdc;
    GetClientRect(hwnd, &ps.rcPaint);
    PaintContent(hwnd, &ps);
}
void
OnDpiChanged(HWND hwnd, WORD x, WORD y, RECT suggested)
{
    MessageBoxW(NULL, L"", L"WM_DPICHANGED", 0);
}
/*
 *  Window procedure
 */
LRESULT CALLBACK
WndProc(HWND hwnd, UINT uiMsg, WPARAM wParam, LPARAM lParam)
{
    switch (uiMsg) {
    HANDLE_MSG(hwnd, WM_CREATE, OnCreate);
    HANDLE_MSG(hwnd, WM_SIZE, OnSize);
    HANDLE_MSG(hwnd, WM_DESTROY, OnDestroy);
    HANDLE_MSG(hwnd, WM_PAINT, OnPaint);
    HANDLE_MSG(hwnd, WM_DPICHANGED, OnDpiChanged);
    case WM_PRINTCLIENT: OnPrintClient(hwnd, (HDC)wParam); return 0;
    }
    return DefWindowProc(hwnd, uiMsg, wParam, lParam);
}
BOOL
InitApp(void)
{
    WNDCLASS wc;
    wc.style = 0;
    wc.lpfnWndProc = WndProc;
    wc.cbClsExtra = 0;
    wc.cbWndExtra = 0;
    wc.hInstance = g_hinst;
    wc.hIcon = NULL;
    wc.hCursor = LoadCursor(NULL, IDC_ARROW);
    wc.hbrBackground = (HBRUSH)(COLOR_WINDOW + 1);
    wc.lpszMenuName = NULL;
    wc.lpszClassName = TEXT("Scratch");
    if (!RegisterClass(&wc)) return FALSE;
    InitCommonControls();               /* In case we use a common control */
    return TRUE;
}
int WINAPI WinMain(HINSTANCE hinst, HINSTANCE hinstPrev,
                   LPSTR lpCmdLine, int nShowCmd)
{
    MSG msg;
    HWND hwnd;
    g_hinst = hinst;
    if (!InitApp()) return 0;
    if (SUCCEEDED(CoInitialize(NULL))) {/* In case we use COM */
        hwnd = CreateWindow(
            TEXT("Scratch"),                /* Class Name */
            TEXT("Scratch"),                /* Title */
            WS_OVERLAPPEDWINDOW,            /* Style */
            CW_USEDEFAULT, CW_USEDEFAULT,   /* Position */
            CW_USEDEFAULT, CW_USEDEFAULT,   /* Size */
            NULL,                           /* Parent */
            NULL,                           /* No menu */
            hinst,                          /* Instance */
            0);                             /* No special parameters */
        ShowWindow(hwnd, nShowCmd);
        while (GetMessage(&msg, NULL, 0, 0)) {
            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }
        CoUninitialize();
    }
    return 0;
}

@sagudev
Copy link
Member

sagudev commented Sep 14, 2023

Maybe we need to tell windows that we are DPI aware application.
https://learn.microsoft.com/en-us/windows/win32/hidpi/wm-dpichanged#remarks

@delan
Copy link
Member Author

delan commented Sep 14, 2023

That did the trick! With d621a90:

[2023-09-14T10:51:50Z INFO  servo::app] window scale factor changed to 2, setting scale factor to 2
[2023-09-14T10:51:52Z INFO  servo::app] window scale factor changed to 1, setting scale factor to 1

Base automatically changed from servoshell-dip-n-dp to master September 14, 2023 11:56
@delan
Copy link
Member Author

delan commented Sep 14, 2023

(force push to rebase over #30344)

@delan delan added this pull request to the merge queue Sep 14, 2023
Merged via the queue into master with commit bb1a6c2 Sep 14, 2023
10 checks passed
@delan delan deleted the minibrowser-hidpi-support branch September 14, 2023 15:04
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.

minibrowser: HiDPI support
4 participants