Implement hidpi for web platform #1233

tangmi · 2019-10-20T08:10:36Z

I believe both backends are implemented, but I'm not sure if just the output I'm seeing from doing cargo web start ... indicates it's working.

This change set it based on the MDN docs for Window.devicePixelRatio and this JSFiddle.

Some points:

Use Window.devicePixelRatio to get the hidpi_factor
Set canvas element to the physical size and the canvas' css size to the logical size
(not implemented yet) register an event for hidpi_factor changes using matchMedia (JSFiddle prototype here https://jsfiddle.net/b6zcg24u/)
- Note: Changing the browser's zoom level affects the devicePixelRatio
Note: winit needs an example that outputs graphics to test this?
TODO: update dpi mod docs!

Tested on all platforms changed
Compilation warnings were addressed
cargo fmt has been run on this branch
Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
Created or updated an example program if it would help users understand this functionality
Updated feature matrix, if new features were added or implemented

tangmi · 2019-10-20T08:20:40Z

src/platform_impl/web/web_sys/mod.rs

-    match document.fullscreen_element() {
-        Some(elem) => {
-            let raw: Element = canvas.clone().into();
-            raw == elem


This line was causing a build error (that Element does not implement PartialEq).

Huh, that's odd. I don't seem to have the same error?

I'll get back to you with my specific versions when I can, but at the time of writing, I had a "clean" lockfile.

Osspial · 2019-10-24T00:26:18Z

Hi, and thanks for working on this. Congratulations on the first PR!

Could you rebase this against the dpi-overhaul branch? We're just about ready to push a major revamp of the DPI API to master, and WASM is the only platform that it's not implemented on. Since this PR implements HiDPI support, it might as well go the extra mile and implement it with the improved API.

tangmi · 2019-10-24T03:09:22Z

@Osspial I sure can. Please don't block merging the revamp on this PR--I'm happy to keep this changelist up to date wherever it needs to be, but free time seems to elude me 😕.

I'd like to also implement the hidpifactorchanged event as a part of this, as well. I'm also planning on at least hacking together some end-to-end tests that use gfx to do some rasterization.

ryanisaacg

Thanks for the PR! If you don't have time to rebase against dpi-overhaul, I'll give it a shot if I get a chance.

ryanisaacg · 2019-10-24T03:23:11Z

src/platform_impl/web/web_sys/mod.rs

-    match document.fullscreen_element() {
-        Some(elem) => {
-            let raw: Element = canvas.clone().into();
-            raw == elem


Huh, that's odd. I don't seem to have the same error?

ryanisaacg · 2019-10-24T03:27:11Z

src/platform_impl/web/stdweb/mod.rs

@@ -41,6 +41,14 @@ pub fn window_size() -> LogicalSize {
    LogicalSize { width, height }
 }

+// https://developer.mozilla.org/en-US/docs/Web/API/Window/devicePixelRatio
+// TODO: Use media queries to register changes in dpi: https://jsfiddle.net/b6zcg24u/
+// TODO: Where does winit handle DPI changes? we can resize the "backbuffer" (canvas element), but isn't that usually handled by e.g. gfx?


I think changing the "backbuffer" and emitting a DPI change event is sufficient here.

src/platform_impl/web/web_sys/canvas.rs

tangmi · 2019-10-24T08:53:09Z

Thanks for the PR! If you don't have time to rebase against dpi-overhaul, I'll give it a shot if I get a chance.

The race is on! I'm hoping to have some time in the next week or so, but please don't wait on me!

tangmi · 2019-10-31T07:09:34Z

I've rebased my changes onto dpi-overhaul, but web builds on that branch don't seem to be building for me? e.g. dpi-overhaul with web-sys (without my changes) is failing locally for me with errors similar to what travis-ci is showing https://travis-ci.org/rust-windowing/winit/jobs/605355237?utm_medium=notification&utm_source=github_status

I'll take a close look next chance I have, but it unfortunately delays this PR

tangmi · 2019-11-02T09:28:02Z

@Osspial I'm making some progress on this, but it seems that the dpi overhaul adds a lifetime to winit::event::Event. Some backends use 'static and some use an anonymous lifetime--I'm wondering if you can advise on what I should do for web (and if fixing this lifetime error should go in a separate change before this one)?

goddessfreya · 2019-11-11T23:23:37Z

Afaik, after giving a904386 a quick skim, I'd annotate everything with 'static, and swap them out for '_s as need be.

Osspial · 2019-11-21T01:18:50Z

@tangmi Annotating everything with 'static should be fine for now. The lifetime is currently only used for the HiDpiFactorChanged event to control the size of the window upon resizing, but IIRC the web backend doesn't support programmatically resizing the window anyway so that's not something we have to worry about.

It may be worth modifying HiDpiFactorChanged::new_inner_size field to take something along the following lines (naming very much WIP, since I'm not in the headspace to think of good names right now):

enum HiDpiFactorWindowSizeChange<'a> {
    Modifiable(&'a mut Option<PhysicalSize<u32>>),
    Imposed(PhysicalSize<u32>)
}

That way, we can tell the user when the OS is imposing a size change that the user cannot modify. IIRC Wayland has an API along those lines (cc @vberger) as well.

tangmi · 2019-11-21T08:26:28Z

Hi all! I haven't had time recently to work on this, I apologize! I still do intend to do this work (if nobody else does it first) and polish this PR, hopefully soon!

goddessfreya · 2019-12-22T00:38:11Z

Hey @tangmi, it has been a while since we've heard from you. Just checking to make sure everything is alright. Also, merry Christmas!

tangmi · 2019-12-23T22:04:00Z

Hi @goddessfreya! I'm still around, thanks for checking in 😄! I've been caught up in my day job, but I'm putting some more work into this over the holidays.

Currently:

Setting most of the Event-related lifetimes to 'static seemed to be fairly easy (@Osspial do you have any suggestions on how/what I should do to test that this is sound?)
Make size/position types generic over pixel type #1277 changed a lot of return types regarding DPI (I'm getting some compilation errors on the dpi-overhaul branch building for web using cargo web build --features stdweb?), and some return values in the web backend are no longer consistent with their logical/physical-ness, I believe.

I'm working on resolving compilation errors and auditing each of the web DPI-related functions.

Happy holidays!

Osspial · 2019-12-23T23:55:06Z

@tangmi It looks like the web backend, for most practical purposes, is completely safe Rust! If it compiles, you shouldn't have to worry about soundness issues.

Osspial · 2019-12-26T05:09:09Z

@tangmi I'm hoping to try and get the full 0.20 release (or at least a release candidate) out by the start of the new year, and this PR is the last change that's blocking that. Do you think you'll end up getting this done by then?

src/platform_impl/web/event_loop/window_target.rs

tangmi · 2019-12-27T22:57:50Z

@tangmi I'm hoping to try and get the full 0.20 release (or at least a release candidate) out by the start of the new year, and this PR is the last change that's blocking that. Do you think you'll end up getting this done by then?

I think I can, but I might be cutting it close! Currently I have both web backends building, I'm going to go over my original dpi changes to make sure they're correct. I think I'm going to leave off the "DPI changed" to a later PR

Osspial · 2019-12-28T19:36:04Z

Don't worry about updating the DPI module docs - I'll manage that

Osspial · 2019-12-28T20:55:59Z

@ryanisaacg would you be able to do another review pass on this?

tangmi · 2019-12-28T23:21:35Z

Don't worry about updating the DPI module docs - I'll manage that

Thanks, I'll clean up this PR a bit, but won't touch the docs.

Some points I think may be important for the DPI docs (or on winit::window::Window::hidpi_factor):

The web backend uses devicePixelRatio
The hidpi_factor (should, but not yet) change when the user changes the scale factor at which they're viewing the page (e.g. via ctrl +/-)
The canvas actual width and height is the physical size (in pixels), while the canvas element's CSS width and height is the logical size (in "css pixels")
winit::platform::web::*Ext docs could probably mention that one should not attempt to resize the canvas element directly (else the DPI state will get messed up until the next resize)

ryanisaacg · 2019-12-29T00:03:04Z

src/platform_impl/web/window.rs

+    pub fn inner_position(&self) -> Result<PhysicalPosition<i32>, NotSupportedError> {
+        // todo: not supported?
+        // Ok(*self.position.borrow())
+        Err(NotSupportedError::new())


Why mark as not supported?

tangmi · 2019-12-29T03:07:52Z

FYI will affect/be affected by #1334

Osspial · 2019-12-30T04:21:23Z

We can go ahead and merge #1334 after this. It should be a pretty simple change to get this branch's changes working with that.

…sition in the viewport

tangmi · 2019-12-31T22:12:40Z

Whoops, forgot to mark this as "ready to review". I think I'm happy with the changes here, but don't hesitate to request changes--I'll hopefully get to them shortly after the new year. Happy new years!

ryanisaacg · 2019-12-31T22:39:15Z

This is great, thanks!

* fix: use a 'static lifetime for the web backend's `Event` types * implement hidpi for stdweb (web-sys wip?) * fix: make all canvas resizes go through backend::set_canvas_size * update Window docs for web, make `inner/outer_position` return the position in the viewport

tangmi changed the title ~~implement hidpi for stdweb (web-sys wip?)~~ Implement hidpi for web platform Oct 20, 2019

tangmi mentioned this pull request Oct 20, 2019

Tracking Issue: DPI Usability Upgrades #939

Closed

12 tasks

tangmi commented Oct 20, 2019

View reviewed changes

Osspial requested a review from ryanisaacg October 24, 2019 00:26

ryanisaacg reviewed Oct 24, 2019

View reviewed changes

tangmi force-pushed the web-hidpi branch from b207bb0 to 4a7a3a6 Compare October 31, 2019 06:29

tangmi changed the base branch from master to dpi-overhaul October 31, 2019 06:29

goddessfreya added DS - web C - in progress Implementation is proceeding smoothly labels Nov 11, 2019

tangmi added 2 commits December 23, 2019 14:06

fix: use a 'static lifetime for the web backend's Event types

e1280c8

implement hidpi for stdweb (web-sys wip?)

741253f

wip: builds now

f6716be

tangmi force-pushed the web-hidpi branch from 4a7a3a6 to f6716be Compare December 27, 2019 22:17

tangmi commented Dec 27, 2019

View reviewed changes

src/platform_impl/web/event_loop/window_target.rs Show resolved Hide resolved

tangmi added 2 commits December 27, 2019 14:29

wip

8154efe

fix: make all canvas resizes go through backend::set_canvas_size

b4efe0a

tangmi mentioned this pull request Dec 27, 2019

fix: remove deprecated usage of mem::uninitialized. #1341

Merged

8 tasks

ryanisaacg reviewed Dec 29, 2019

View reviewed changes

update Window docs for web, make inner/outer_position return the po…

17ca323

…sition in the viewport

tangmi marked this pull request as ready for review December 31, 2019 22:11

goddessfreya added C - waiting on maintainer A maintainer must review this code and removed C - in progress Implementation is proceeding smoothly labels Dec 31, 2019

ryanisaacg approved these changes Dec 31, 2019

View reviewed changes

ryanisaacg merged commit 2b75c2e into rust-windowing:dpi-overhaul Dec 31, 2019

ryanisaacg removed the C - waiting on maintainer A maintainer must review this code label Dec 31, 2019

tangmi mentioned this pull request Nov 13, 2020

HiDPI support redox-os/orbtk#71

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement hidpi for web platform #1233

Implement hidpi for web platform #1233

tangmi commented Oct 20, 2019 •

edited

Loading

tangmi Oct 20, 2019

ryanisaacg Oct 24, 2019

tangmi Oct 24, 2019

Osspial commented Oct 24, 2019

tangmi commented Oct 24, 2019

ryanisaacg left a comment

ryanisaacg Oct 24, 2019

ryanisaacg Oct 24, 2019

tangmi commented Oct 24, 2019

tangmi commented Oct 31, 2019

tangmi commented Nov 2, 2019

goddessfreya commented Nov 11, 2019 •

edited

Loading

Osspial commented Nov 21, 2019

tangmi commented Nov 21, 2019

goddessfreya commented Dec 22, 2019

tangmi commented Dec 23, 2019 •

edited

Loading

Osspial commented Dec 23, 2019

Osspial commented Dec 26, 2019

tangmi commented Dec 27, 2019

Osspial commented Dec 28, 2019

Osspial commented Dec 28, 2019

tangmi commented Dec 28, 2019

ryanisaacg Dec 29, 2019

tangmi commented Dec 29, 2019

Osspial commented Dec 30, 2019

tangmi commented Dec 31, 2019

ryanisaacg commented Dec 31, 2019

Implement hidpi for web platform #1233

Implement hidpi for web platform #1233

Conversation

tangmi commented Oct 20, 2019 • edited Loading

tangmi Oct 20, 2019

Choose a reason for hiding this comment

ryanisaacg Oct 24, 2019

Choose a reason for hiding this comment

tangmi Oct 24, 2019

Choose a reason for hiding this comment

Osspial commented Oct 24, 2019

tangmi commented Oct 24, 2019

ryanisaacg left a comment

Choose a reason for hiding this comment

ryanisaacg Oct 24, 2019

Choose a reason for hiding this comment

ryanisaacg Oct 24, 2019

Choose a reason for hiding this comment

tangmi commented Oct 24, 2019

tangmi commented Oct 31, 2019

tangmi commented Nov 2, 2019

goddessfreya commented Nov 11, 2019 • edited Loading

Osspial commented Nov 21, 2019

tangmi commented Nov 21, 2019

goddessfreya commented Dec 22, 2019

tangmi commented Dec 23, 2019 • edited Loading

Osspial commented Dec 23, 2019

Osspial commented Dec 26, 2019

tangmi commented Dec 27, 2019

Osspial commented Dec 28, 2019

Osspial commented Dec 28, 2019

tangmi commented Dec 28, 2019

ryanisaacg Dec 29, 2019

Choose a reason for hiding this comment

tangmi commented Dec 29, 2019

Osspial commented Dec 30, 2019

tangmi commented Dec 31, 2019

ryanisaacg commented Dec 31, 2019

tangmi commented Oct 20, 2019 •

edited

Loading

goddessfreya commented Nov 11, 2019 •

edited

Loading

tangmi commented Dec 23, 2019 •

edited

Loading