Check that TransformLayer has a finite matrix#8585
Uh oh!
There was an error while loading. Please reload this page.
Conversation
To catch issues like flutter/flutter#30586 We'll also upload a test in the framework that will trigger this DCHECK if flutter#8467 were reverted.
| TransformLayer::~TransformLayer() = default; | ||
| voidTransformLayer::Preroll(PrerollContext* context, const SkMatrix& matrix){ | ||
| // Checks (in some degree) that SkMatrix transform_ is valid and initialized. |
There was a problem hiding this comment.
Can the TransformLayer constructor initialize the transform_ to the identity matrix?
Another option would be setting a flag when set_transform is called and checking that flag here.
There was a problem hiding this comment.
I'll add a flag, together with this isFinite check. The reason that we still need this isFinite check is that someone could call set_transform with an uninitialized SkMatrix.
flow/layers/transform_layer.cc Outdated
| // | ||
| // We have to write this flaky test because there is no reliable way to test | ||
| // whether a variable is initialized or not in C++. | ||
| FML_DCHECK(transform_.isFinite()); |
There was a problem hiding this comment.
The unopt variant is not used in any serious manner when exercising flow. Having uninitialized stuff in the matrix is quite serious. Maybe just terminate the process here.
There was a problem hiding this comment.
I've changed FML_DCHECK to FML_CHECK.
liyuqianforce-pushed the check_transform_matrix branch from 085d11f to c1caefdCompareliyuqian commented Apr 16, 2019
@chinmaygarde@jason-simmons : how does the new patch look to you? |
chinmaygarde commented Apr 17, 2019
Instead of the |
chinmaygarde left a comment
chinmaygarde left a comment There was a problem hiding this comment.
lgtm with the nit about initializing the matrix.
liyuqianforce-pushed the check_transform_matrix branch from 741cfff to c592f71Compare
flutter/engine@388124f...fdd4272 git log 388124f..fdd4272 --no-merges --oneline fdd4272 Roll src/third_party/skia 70ed05e53ad2..652b007a4cbb (1 commits) (flutter/engine#8601) 3b01610 Check that TransformLayer has a finite matrix (flutter/engine#8585) 4805d72 Implement StandardMethodCodec for C++ shells (flutter/engine#8598) 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.
As we've introduced offset to the Opacity layer, we have to override `applyTransform` to make Leader/FollowerLayer work correctly. Fixesflutter#30587 Together with flutter/engine#8585, this test will also exercise test against flutter#30586.
## Description As we've introduced offset to the Opacity layer, we have to override `applyTransform` to make Leader/FollowerLayer work correctly. ## Related Issues Fixes#30587 Together with flutter/engine#8585, this test will also exercise test against #30586. ## Tests I added the following tests: * text field selection toolbar renders correctly inside opacity
4 participants
To catch issues like flutter/flutter#30586
We'll also upload a test in the framework that will trigger this DCHECK
if #8467 were reverted.