Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2168933002:
Update SkImage_NewFromTexture test (Closed)
|
Created:
4 years, 5 months ago by Brian Osman Modified:
4 years, 5 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionUpdate SkImage_NewFromTexture test, to just test release
procs of SkImage::MakeFromTexture.
Previous code included an old pattern, and may be
responsible for intermittent Mac bot failure
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002
Committed: https://skia.googlesource.com/skia/+/db2cb10f4d2a33acab7bc1aef9d9d6cc566143f9
Patch Set 1 #
Total comments: 2
Patch Set 2 : Restore testing of ReleaseProc #
Total comments: 1
Patch Set 3 : Switch to testing-only texture API #Messages
Total messages: 28 (18 generated)
Description was changed from ========== Remove SkImage_NewFromTexture test Obsolete pattern, and may be responsible for intermittent Mac bot failure BUG=skia: ========== to ========== Remove SkImage_NewFromTexture test Obsolete pattern, and may be responsible for intermittent Mac bot failure BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002 ==========
The CQ bit was checked by brianosman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Remove SkImage_NewFromTexture test Obsolete pattern, and may be responsible for intermittent Mac bot failure BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002 ========== to ========== Remove SkImage_NewFromTexture test Old pattern, and may be responsible for intermittent Mac bot failure BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002 ==========
The CQ bit was unchecked by brianosman@google.com
brianosman@google.com changed reviewers: + benjaminwagner@google.com, bsalomon@google.com, robertphillips@google.com
https://codereview.chromium.org/2168933002/diff/1/tests/ImageTest.cpp File tests/ImageTest.cpp (left): https://codereview.chromium.org/2168933002/diff/1/tests/ImageTest.cpp#oldcode800 tests/ImageTest.cpp:800: // Now exercise the release proc Do we test, somewhere else, that we correctly call the release procs on backend textures and rendertargets?
https://codereview.chromium.org/2168933002/diff/1/tests/ImageTest.cpp File tests/ImageTest.cpp (left): https://codereview.chromium.org/2168933002/diff/1/tests/ImageTest.cpp#oldcode800 tests/ImageTest.cpp:800: // Now exercise the release proc On 2016/07/21 15:11:30, robertphillips wrote: > Do we test, somewhere else, that we correctly call the release procs on backend > textures and rendertargets? Ah, that is probably worth testing (with a much smaller test). This test is problematic in that it wraps the same texture in two GrTexture objects.
I will try and strip this test down to produce something that just tests the release procs. Stay tuned.
The CQ bit was checked by brianosman@google.com to run a CQ dry run
The CQ bit was unchecked by brianosman@google.com
The CQ bit was checked by brianosman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by brianosman@google.com
Stripped this down to just testing the release proc. Also, the previous version didn't have multiple textures after my removal of copy, so it should have been safe. In any case, we can try landing this version to see if it helps the mac bots?
https://codereview.chromium.org/2168933002/diff/20001/tests/ImageTest.cpp File tests/ImageTest.cpp (left): https://codereview.chromium.org/2168933002/diff/20001/tests/ImageTest.cpp#old... tests/ImageTest.cpp:780: backendDesc.fTextureHandle = tex->getTextureHandle(); Can we get rid of 'tex' and use backendDesc.fTextureHandle = ctxInfo.grContext()->getGpu()->createTestingOnlyBackendTexture( pixels.get(), w, h, kRGBA_8888_GrPixelConfig, true); ? At the end of the test we'd also need ctxInfo.grContext()->getGpu()->deleteTestingOnlyBackendTexture(backendDesc.fTextureHandle);
The CQ bit was checked by brianosman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Remove SkImage_NewFromTexture test Old pattern, and may be responsible for intermittent Mac bot failure BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002 ========== to ========== Update SkImage_NewFromTexture test, to just test release procs of SkImage::MakeFromTexture. Previous code included an old pattern, and may be responsible for intermittent Mac bot failure BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002 ==========
Updated to use TestingOnlyBackendTexture API...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by brianosman@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Update SkImage_NewFromTexture test, to just test release procs of SkImage::MakeFromTexture. Previous code included an old pattern, and may be responsible for intermittent Mac bot failure BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002 ========== to ========== Update SkImage_NewFromTexture test, to just test release procs of SkImage::MakeFromTexture. Previous code included an old pattern, and may be responsible for intermittent Mac bot failure BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168933002 Committed: https://skia.googlesource.com/skia/+/db2cb10f4d2a33acab7bc1aef9d9d6cc566143f9 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/db2cb10f4d2a33acab7bc1aef9d9d6cc566143f9 |
