[web] Fix Scene clip bounds. Trigger resize on DPR Change.

[ditman]

The Scene of the HTML renderer is passing incorrect size information to the engine, and when DPR<1, it can result in elements being culled off of the viewport.

In addition to that, when an app is embedded, not all changes in DPR cause a Resize event (only those some of the dimensions fails by a rounding error!), so this PR ensures that all DPR events in embedded trigger a resize event.

Issues

Fixes flutter/flutter#129182

Testing

Looking good at: https://dit-astral-test.web.app

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[mdebbar]

Overall, LGTM.

I can't speak for the html renderer changes though. I'm not sure where exactly we are supposed to used physical vs logical pixels. cc @yjbanov

[ditman]

I've added a couple of tests, one for the dpr stream itself, and another for the connection between the dpr stream and the size provider, I think this is ready to land.

[mdebbar]

LGTM if LGT @yjbanov

[yjbanov]

Do we want to continue with the singleton pattern? I was under the impression that we were removing all singletons except one top-level class (was it EnginePlatformDispatcher?), which would do a kind of dependency injection.

[ditman]

Yeah, we're still using the singleton pattern for things that are ONE only (browser globals). devicePixelRatio is determined by the browser window. This is similar to the "brightness/dark mode" detector, and implemented in an almost identical way.

(I tried to make this instance-based (by passing a Display), hoping that this API¹ will be eventually usable, but it's still unclear to me from the docs how it can be used to monitor DPR changes, though!)

Footnotes

1. https://developer.mozilla.org/en-US/docs/Web/API/Window_Management_API ↩

[mdebbar]

In one of our recent-ish discussions, we entertained the idea of keeping only ONE singleton in the engine (e.g. EnginePlatformDispatcher), and make all other singletons a member of this ONE singleton.

E.g. instead of DisplayDprStream.instance, we would have EnginePlatformDispatcher.instance.displayDprStream.

It was just a discussion though. We haven't made any real changes in the code base following that discussion. But I think at some point we should start moving to that pattern.

[ditman]

Ah, I understand now! Yes, I could move this to a singleton of the EnginePlatformDispatcher instead of being a loose object.

[yjbanov]

I think the litmus test is that code that consumes an external service should not go through a static to get a reference to that service. EnginePlatformDispatcher.instance.displayDprStream still accesses DisplayDprStream through a static getter. In fact, it may be worse, the code only needed a DisplayDprStream but it depends on EnginePlatformDispatcher, which vends more dependencies than it needs. It still has the same testability issues, where to mock out DisplayDprStream you need to mock out the EnginePlatformDispatcher.instance static getter.

I don't insist that this is fixed in this PR, unless we already switched modes to a singleton-free design.

[ditman]

I changed this a little bit. The CustomElement dimensions provider now just requires a Stream passed by its constructor. For now, the abstract factory class of DimensionsProviders knows where to get that stream from (the singleton instance), but it's very likely that it could just be created from there.

Anyway, I'm going to autosubmit this, and if you find anything egregious, feel free to raise an issue or ping me so I can address it ASAP.

[mdebbar]

Should we also change the type to StreamController<void> or StreamController<Null>?

[ditman]

Nope, I really really want this to be a StreamController<ui.Size>, it's just that we don't know how to implement it efficiently now :P

[jonahwilliams]

Reason for revert: This is causing what looks like bogus goldens on the framework -> engine roll: flutter/flutter#142966