Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(141)

Issue 1560643002: Revert of Make SkGLContext lifetime more well-defined (Closed)

Created:
4 years, 11 months ago by mtklein
Modified:
4 years, 11 months ago
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Revert of Make SkGLContext lifetime more well-defined (patchset #7 id:120001 of https://codereview.chromium.org/1511773005/ ) Reason for revert: tests/GrContextFactoryTest.cpp failing on most GPU bots, e.g. https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Mac10.9-Clang-MacMini6.2-GPU-HD4000-x86_64-Debug/builds/3393 Original issue's description: > Make SkGLContext lifetime more well-defined > > Remove refcounting from SkGLContext. > > SkGLContext is expected to behave like GrContextFactory would own > it, as implied by the GrContextFactory function. > > If it is refcounted, this does not hold. > > Also other use sites, such as in SkOSWindow_win (command buffer gl > object), confirm the behavior. The object is explicitly owned and > destroyed, not shared. > > Also fixes potential crashes from using GL context of an abandoned > context. > > Also fixes potential crashes in DM/nanobench, if the GrContext lives > longer than GLContext through internal refing of GrContext. > > Moves the non-trivial implementations from GrContextFactory.h to > .cpp, just for consistency sake. > > Changes pathops_unittest.gyp. The pathops_unittest uses > GrContextFactory, but did not link to its implementation. The reason > they worked was that the implementation used (constructors, destructors) > happened to be in the .h file. > > This works towards being able to use command buffer and NVPR from > the SampleApp. > > BUG=skia:2992 > GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1511773005 > > Committed: https://skia.googlesource.com/skia/+/830e012187f951d49d7e46e196ac8d1e653a25da TBR=bsalomon@google.com,kkinnunen@nvidia.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia:2992

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -125 lines) Patch
M bench/nanobench.cpp View 1 chunk +1 line, -1 line 0 comments Download
M dm/DM.cpp View 2 chunks +8 lines, -10 lines 0 comments Download
M gyp/pathops_unittest.gyp View 1 chunk +0 lines, -4 lines 0 comments Download
M include/gpu/gl/SkGLContext.h View 2 chunks +4 lines, -2 lines 0 comments Download
M src/gpu/GrContextFactory.h View 1 chunk +38 lines, -26 lines 0 comments Download
M src/gpu/GrContextFactory.cpp View 4 chunks +17 lines, -57 lines 0 comments Download
M tests/EGLImageTest.cpp View 2 chunks +2 lines, -1 line 0 comments Download
M tests/GrContextFactoryTest.cpp View 1 chunk +0 lines, -24 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
mtklein
Created Revert of Make SkGLContext lifetime more well-defined
4 years, 11 months ago (2016-01-05 12:16:15 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560643002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560643002/1
4 years, 11 months ago (2016-01-05 12:16:25 UTC) #2
commit-bot: I haz the power
Failed to apply the patch.
4 years, 11 months ago (2016-01-05 12:16:29 UTC) #4
mtklein
4 years, 11 months ago (2016-01-05 12:17:37 UTC) #5
Joe beat me to it!  Ignore this.

Powered by Google App Engine
This is Rietveld 408576698