[Impeller] fix validation error for playground texture upload. #49957
Conversation
| VALIDATION_LOG << "Could not upload texture to device memory for fixture."; | ||
| return nullptr; | ||
| } | ||
| if (enable_mipmapping) { |
There was a problem hiding this comment.
This branch down here was missing the mipmapping support.
jonahwilliams
changed the title
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
Okay, this makes sense. We are just ripping out the SupportsBufferToTextureBlits path to make sure the image format is getting set correctly, right?
This needs a test (something that exercises it stays fixed on ci). I think maybe the easiest thing to do would be for me to land the PR that turns on validation, but have a way to selectively turn it off for certain tests. Then this PR can turn it back on for a handful of tests. How's that sound?
|
Are there other validation errors? If not I think we land this and then follow up to turn on validations. Ultimately this is just test code. |
Chinmay was running into validation errors too when getting the playgrounds working (not goldens) and he was running into some others. My PR's CI runs got messed up, let me try to get them running and get back to you. If this is the only validation error, just pull my patch into this PR. |
|
For reference: This is the draft PR that attempts to turn validation on for the golden image tests #49955 |
|
I ran into another failure later when validations are on: There is still something goofed up. Running Let me see if we can get validation on just for |
|
I think if there are more then we should try and stage the test so that we gradually decrease the amount of validation errors. Trying to get them all in one go isn't realistic, and we'll keep backsliding too. |
gaaclarke
force-pushed
the
fix_CreateTextureForFixture
branch
from
558b48a to
8c03e89
Compare
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
I turned on the validations for one of the tests that would fail before this PR. LGTM, thanks!
|
woohoo! |
gaaclarke
added
the
autosubmit
…142067) flutter/engine@9940541...57d6b51 2024-01-23 [email protected] [Impeller] fix validation error for playground texture upload. (flutter/engine#49957) 2024-01-23 [email protected] [Impeller] Fix runtime effect pipeline depth/stencil. (flutter/engine#49953) 2024-01-23 [email protected] Exclude prebuilts/Library from Mac builder_cache (flutter/engine#49971) 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
2 participants
Buffer to image is missing barriers. We don't really need to use this for playgrounds though, as set contents does the right thing more or less.
Fixes flutter/flutter#142024