This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Create mipmaps for images when uploading them on the IO thread#7751
Merged
brianosman merged 1 commit intoflutter:masterfrom Feb 8, 2019
brianosman:adreno-img
Merged
Create mipmaps for images when uploading them on the IO thread#7751brianosman merged 1 commit intoflutter:masterfrom brianosman:adreno-img
brianosman merged 1 commit intoflutter:masterfrom
brianosman:adreno-img
Conversation
This does several things: - It adds CPU time on the IO thread, but avoids GPU time on the GPU thread. - For images that are never drawn with mipmaps, it adds about 33% memory overhead. For images that are drawn with mipmaps, it saves an entire copy of the base level. - It fixes flutter/flutter#24517, which is a driver bug related to mip-mapping and cross-context images. Overall, I think the tradeoff is good, but I'm curious to see what benchmarks look like.
Contributor
|
We were planning on investigating this. Thanks for making a recommendation about the tradeoff. |
chinmaygarde
approved these changes
Feb 8, 2019
Contributor
Author
|
Ah, didn't know about that issue. Yes - the interesting thing is that right now, even if you create an image without mipmaps, Skia will build them when you draw that image at scale < 1 and filter quality > low. We would actually like to get away from that, and make mipmaps be an immutable property of the image, but that's a pretty drastic API change. If Flutter wanted to do this, though - that would be great. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Feb 8, 2019
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Feb 8, 2019
flutter/engine@25c513c...7c70240 git log 25c513c..7c70240 --no-merges --oneline 7c70240 Create mipmaps for images when uploading them on the IO thread (flutter/engine#7751) 3183d15 Rename macOS framework to FlutterMacOS.framework (flutter/engine#7740) aa27582 Support for loading dynamic patches in AOT mode. (flutter/engine#7744) 5bfb1ec Roll src/third_party/skia f2a3f5943e4c..37064c1739f3 (11 commits) (flutter/engine#7752) cf1d70a Add onPlatformBrightnessChanged/platformBrightness to stub ui window. (flutter/engine#7739) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
kangwang1988
pushed a commit
to XianyuTech/flutter
that referenced
this pull request
Feb 12, 2019
flutter/engine@25c513c...7c70240 git log 25c513c..7c70240 --no-merges --oneline 7c70240 Create mipmaps for images when uploading them on the IO thread (flutter/engine#7751) 3183d15 Rename macOS framework to FlutterMacOS.framework (flutter/engine#7740) aa27582 Support for loading dynamic patches in AOT mode. (flutter/engine#7744) 5bfb1ec Roll src/third_party/skia f2a3f5943e4c..37064c1739f3 (11 commits) (flutter/engine#7752) cf1d70a Add onPlatformBrightnessChanged/platformBrightness to stub ui window. (flutter/engine#7739) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This does several things:
Overall, I think the tradeoff is good, but I'm curious to see what benchmarks look like.