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

Conversation

@eyebrowsoffire
Copy link
Contributor

There were two issues here:

  1. We have to stop using the emscripten thread scheduling APIs, as they can be invoked out of order from the rest of the messages that are posted. In some cases, out of order message handling can cause the request for reading pixels in an image to be serviced before some of the texture sources have been transfered to the web worker.

  2. Skia's readPixels fails if there is is a lazy picture image made from a picture that contains a lazy texture image. The pertinent bug is here: https://g-issues.skia.org/issues/349201915

To work around the Skia bug, we just render the image itself onto our scratch canvas and pull the pixels out with glReadPixels.

This fixes flutter/flutter#141326

There were two issues here:

1) We have to stop using the emscripten thread scheduling APIs, as they
can be invoked out of order from the rest of the messages that are
posted. In some cases, out of order message handling can cause the
request for reading pixels in an image to be serviced before some of the
texture sources have been transfered to the web worker.

2) Skia's `readPixels` fails if there is is a lazy picture image made
from a picture that contains a lazy texture image. The pertinent bug is
here: https://g-issues.skia.org/issues/349201915

To work around the Skia bug, we just render the image itself onto our
scratch canvas and pull the pixels out with `glReadPixels`.

This fixes flutter/flutter#141326
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Jun 25, 2024
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

}

// Main thread only
// Worker thread only
Copy link
Contributor

Choose a reason for hiding this comment

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

Important distinction! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The semantics here actually changed as part of this PR. dispose used to be called on the main thread, and then that dispatched to the web worker which called _dispose. Now there's just one method, dispose and that does what _dispose used to do.

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 25, 2024
@auto-submit auto-submit bot merged commit 94023d7 into flutter:main Jun 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 25, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 25, 2024
…150797)

flutter/engine@62e0b5f...94023d7

2024-06-25 [email protected] [skwasm] Fixes for getting pixels from an image. (flutter/engine#53561)
2024-06-25 [email protected] fix: web canvaskit fragment shaders were not using updated uniform values (flutter/engine#53246)
2024-06-25 [email protected] Roll Dart SDK from b5fc85cfcf1b to 65ab2f2cf0d3 (1 revision) (flutter/engine#53565)
2024-06-25 [email protected] Roll Skia from e4e4feb97a54 to da1ea4eb0270 (4 revisions) (flutter/engine#53563)
2024-06-25 [email protected] [DisplayList] Switch to recording DrawVertices objects by reference (flutter/engine#53548)
2024-06-25 [email protected] Roll Skia from 5f21260470cf to e4e4feb97a54 (1 revision) (flutter/engine#53559)
2024-06-25 [email protected] [impeller] Cleanup blur (flutter/engine#53543)

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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
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.

[skwasm] ColorScheme.fromImageProvider always produces the same result

2 participants