Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

eyebrowsoffire
Copy link
Contributor

I wasn't adjusting the left and top css styles based on the device pixel ratio. This fixes that and also sets up unit tests for SceneView.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Aug 29, 2023
bounds.bottom.ceilToDouble()
);
final DomCSSStyleDeclaration style = canvas.style;
final double logicalWidth = roundedOutBounds.width / window.devicePixelRatio;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we lose the sub-pixel delta between the rounded values and the actual values? I wonder of this is why we get flutter/flutter#122541 in CanvasKit, and maybe we have the same issue in Skwasm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I first implemented the platform views change (when it was a draft), I actually did have a ton of golden diffs because I had blurring due to what you're talking about. The rounding out of the rectangle fixed it, and got rid of the extra blurring and the goldens were consistent with previous behavior.

That being said, I am not 100% sure the existing behavior is totally correct. This change at least gets us back to the previous rendering behavior though. I think I will follow up and look a little deeper into this, just to make sure that we aren't ending up with any fractional weirdness here.

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 29, 2023
@auto-submit auto-submit bot merged commit 9778c2c into flutter:main Aug 29, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 29, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 29, 2023
…133526)

flutter/engine@ffda7e3...9778c2c

2023-08-29 [email protected] Fix scene view canvas/platform view placement. (flutter/engine#45199)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@pedromassango
Copy link
Member

Does this fixes the resize blur issue flutter/flutter#122541?

@eyebrowsoffire
Copy link
Contributor Author

Does this fixes the resize blur issue flutter/flutter#122541?

It does not, this change only addresses the experimental skwasm renderer, not canvaskit.

gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
I wasn't adjusting the `left` and `top` css styles based on the device pixel ratio. This fixes that and also sets up unit tests for `SceneView`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants