Repository navigation
remove single-view assumption from paintImage - #117495
goderbauer wants to merge 1 commit into
Conversation
flutter-dashboard
Bot
added
framework
goderbauer
changed the title
paintImage
bdc3e83 to
c394485
Compare
|
@dnfield A while ago you added this code to Another option would be to make passing in the dpr required. This would be a breaking change since there are two callers of this method in google3 (those could be easily updated, though). It does feel a little odd to require it just for the debug check, though. Yet another option could be to move the check out of paintImage. However, I didn't really find a better place for it... What are your thoughts? |
| /// | ||
| /// * `devicePixelRatio`: The device pixel ratio of the [FlutterView] into | ||
| /// which the image will be drawn. Can be obtained via | ||
| /// `View.of(context).devicePixelRation`. |
There was a problem hiding this comment.
note to self: mention MediaQuery.devicePixelRatioOf(context) as well
There was a problem hiding this comment.
| /// `View.of(context).devicePixelRation`. | |
| /// `View.of(context).devicePixelRatio`. |
There was a problem hiding this comment.
Can this get defaulted somehow?
For example, just default it to the first view in the collection of views?
There was a problem hiding this comment.
If we default this, the assert check in paintImage becomes more or less meaningless, no? It needs to know the exact dpr in order to figure out whether the current image dimensions are overkill or not.
There was a problem hiding this comment.
We could just use the max device pixel ratio of all views as an upper bound, though. That will greatly simplify things.
| invertColors: invertColors, | ||
| isAntiAlias: isAntiAlias, | ||
| filterQuality: filterQuality, | ||
| devicePixelRatio: View.maybeOf(context)?.devicePixelRatio, |
There was a problem hiding this comment.
note to self: here and below use MediaQuery.maybeDevicePixelRatioOf(context) instead.
|
"Google testing" failed because tests were run without including prerequisite #117244. |
|
Superseded by #118721. |
2 participants
Fixes #117477.
Part of #116929.