Skia: Track the fIsWrapped separately so that we delete correctly

Skia: Track the fIsWrapped separately so that we delete correctly

GrGlTextureRenderTarget inherits virtually from both GrGlRenderTarget and
GrGLTexture, which both have a 'wrap' flag.  The passed in wrap setting could
be different for the two base classes, but since it's virtually inherited,
they share the same flag, so they're either both on, or both off.

As a result, we fail to delete the frambuffer.

To fix this, we now keep a separate isWrapped flag for GrGlRenderTarget.

BUG=437998
Committed: https://skia.googlesource.com/skia/+/9a0c7abfd7ce8694136840fa224e99579b8329f6

[hendrikw, 2014-12-09 19:30:09]

hendrikw@chromium.org changed reviewers:
+ bsalomon@google.com

[hendrikw, 2014-12-09 19:45:35]

PTAL

Should we be doing the same with GrGLTexture?  It currently works because
GrGLTexture is the first class inherited, but this code still seems a bit
sketchy.

Also, I could replace fRTFBOID, fTexFBOID, fMSColorRenderbufferID and fIsWrapped
with an instance of IDDesc, but I'm not sure what skia style dictates.

I looked through the code for similar inheritance patterns, didn't see any
(GrGLProgramBuilder is, but no data), so the remaining issues are probably
unrelated.

[bsalomon, 2014-12-09 20:02:06]

https://codereview.chromium.org/791493003/diff/1/src/gpu/gl/GrGLRenderTarget.h
File src/gpu/gl/GrGLRenderTarget.h (right):

https://codereview.chromium.org/791493003/diff/1/src/gpu/gl/GrGLRenderTarget....
src/gpu/gl/GrGLRenderTarget.h:78: bool          fIsWrapped;
Can you add a comment that we track this separately because this may be both a
texture and an rt, and the tex may be wrapped while the rt is not?

Otherwise, lgtm.

[bsalomon, 2014-12-09 20:03:42]

On 2014/12/09 19:45:35, hendrikw wrote:
> PTAL
> 
> Should we be doing the same with GrGLTexture?  It currently works because
> GrGLTexture is the first class inherited, but this code still seems a bit
> sketchy.

Oh just read this.. I think you're right that we should be tracking it
separately there too.

I don't have an opinion on whether we store the iddesc directly or a parallel
set of fields, whatever works for you.

[hendrikw, 2014-12-09 21:55:09]

On 2014/12/09 20:03:42, bsalomon wrote:
> On 2014/12/09 19:45:35, hendrikw wrote:
> > PTAL
> > 
> > Should we be doing the same with GrGLTexture?  It currently works because
> > GrGLTexture is the first class inherited, but this code still seems a bit
> > sketchy.
> 
> Oh just read this.. I think you're right that we should be tracking it
> separately there too.
> 
> I don't have an opinion on whether we store the iddesc directly or a parallel
> set of fields, whatever works for you.

PTAL, thanks!

[bsalomon, 2014-12-09 22:16:12]

lgtm

[hendrikw, 2014-12-09 22:16:44]

The CQ bit was checked by hendrikw@chromium.org

[commit-bot: I haz the power, 2014-12-09 22:17:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/791493003/20001

[commit-bot: I haz the power, 2014-12-09 22:26:52]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/9a0c7abfd7ce8694136840fa224e99579b8329f6