Speculative fix for http://crbug.com/515966

Speculative fix for http://crbug.com/515966

BUG=chromium:515966
Committed: https://skia.googlesource.com/skia/+/85cd78dd6ca93cc5bd4a504b201c3215426d885b

[bsalomon, 2015-07-31 18:15:43]

bsalomon@google.com changed reviewers:
+ egdaniel@google.com

[bsalomon, 2015-07-31 18:15:47]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2015-07-31 18:16:01]

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

[commit-bot: I haz the power, 2015-07-31 18:16:03]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-08-01 00:16 UTC

[krasin, 2015-07-31 18:24:10]

krasin@google.com changed reviewers:
+ krasin@google.com

[krasin, 2015-07-31 18:24:10]

https://codereview.chromium.org/1268723005/diff/20001/src/gpu/GrImmediateDraw...
File src/gpu/GrImmediateDrawTarget.cpp (right):

https://codereview.chromium.org/1268723005/diff/20001/src/gpu/GrImmediateDraw...
src/gpu/GrImmediateDrawTarget.cpp:91: const GrPipeline* pipeline =
reinterpret_cast<GrPipeline*>(pipelineAddr);
Proposal: setupPipeline could return GrPipeline*. This will eliminate the casts
in setupPipeline callers and overall simplify the code.

[krasin, 2015-07-31 18:29:18]

https://codereview.chromium.org/1268723005/diff/20001/src/gpu/GrImmediateDraw...
File src/gpu/GrImmediateDrawTarget.cpp (right):

https://codereview.chromium.org/1268723005/diff/20001/src/gpu/GrImmediateDraw...
src/gpu/GrImmediateDrawTarget.cpp:87:
GrImmediateDrawTarget::setupPipelineAndShouldDraw(void* pipelineAddr,
Note: using SkAlignedSStorage<sizeof(GrPipeline)>* instead of void* will allow
to have better compiler time type checks, and still satisfy all the existing
callers.

I am not 100% sure, that this suggestion makes sense, since I don't know the
code.

[bsalomon, 2015-07-31 18:55:11]

https://codereview.chromium.org/1268723005/diff/20001/src/gpu/GrImmediateDraw...
File src/gpu/GrImmediateDrawTarget.cpp (right):

https://codereview.chromium.org/1268723005/diff/20001/src/gpu/GrImmediateDraw...
src/gpu/GrImmediateDrawTarget.cpp:87:
GrImmediateDrawTarget::setupPipelineAndShouldDraw(void* pipelineAddr,
On 2015/07/31 18:29:17, krasin wrote:
> Note: using SkAlignedSStorage<sizeof(GrPipeline)>* instead of void* will allow
> to have better compiler time type checks, and still satisfy all the existing
> callers.
> 
> I am not 100% sure, that this suggestion makes sense, since I don't know the
> code.

My perhaps imperfect understanding is that the purpose of this is to allow
different implementations to make their own decisions about how to allocate
space for the pipeline object so I'm not sure we want to require the use of
SkAlignedSStorage.

https://codereview.chromium.org/1268723005/diff/20001/src/gpu/GrImmediateDraw...
src/gpu/GrImmediateDrawTarget.cpp:91: const GrPipeline* pipeline =
reinterpret_cast<GrPipeline*>(pipelineAddr);
On 2015/07/31 18:24:10, krasin wrote:
> Proposal: setupPipeline could return GrPipeline*. This will eliminate the
casts
> in setupPipeline callers and overall simplify the code.

Done.

[bsalomon, 2015-07-31 18:59:05]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2015-07-31 18:59:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1268723005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1268723005/40001

[egdaniel, 2015-07-31 19:04:29]

lgtm

[krasin, 2015-07-31 19:05:19]

LGTM

I have verified that if I apply the patch, it does not complain about casts like
this. The only remaining (known) invalid cast is https://crbug.com/515973. After
that compositor_unittests will pass.

[commit-bot: I haz the power, 2015-07-31 19:15:53]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/85cd78dd6ca93cc5bd4a504b201c3215426d885b