cc: Remove the RenderPassId field from AppendQuadsData.

cc: Remove the RenderPassId field from AppendQuadsData.

This field is redundant now that we pass a RenderPass* to AppendQuads.

A few tests were passing a different id and the same RenderPass* to
test DelegatedRendererLayerImpl, so I had to adjust the tests to
pass different RenderPass* instead.

R=vmpstr

Committed: https://crrev.com/4c9b207ec1d2076864f8f0dfd4dab8acb25196c9
Cr-Commit-Position: refs/heads/master@{#313630}

[vmpstr, 2015-01-28 22:05:27]

https://codereview.chromium.org/879423002/diff/1/cc/test/layer_test_common.cc
File cc/test/layer_test_common.cc (right):

https://codereview.chromium.org/879423002/diff/1/cc/test/layer_test_common.cc...
cc/test/layer_test_common.cc:138: void
LayerTestCommon::LayerImplTest::AppendQuadsForPassWithOcclusion(
Should the rest of the functions also take a render_pass instead of using
|render_pass_|? It's not a big concern since this is test, but it's pretty easy
in this function to accidentally use render_pass_ instead of render_pass

[danakj, 2015-01-28 22:06:21]

https://codereview.chromium.org/879423002/diff/1/cc/test/layer_test_common.cc
File cc/test/layer_test_common.cc (right):

https://codereview.chromium.org/879423002/diff/1/cc/test/layer_test_common.cc...
cc/test/layer_test_common.cc:138: void
LayerTestCommon::LayerImplTest::AppendQuadsForPassWithOcclusion(
On 2015/01/28 22:05:27, vmpstr wrote:
> Should the rest of the functions also take a render_pass instead of using
> |render_pass_|? It's not a big concern since this is test, but it's pretty
easy
> in this function to accidentally use render_pass_ instead of render_pass

We could, it would just mean a bunch of code at callsites creating renderpasses
to pass in. I could rename the local variable if that helps?

[vmpstr, 2015-01-28 22:07:19]

On 2015/01/28 22:06:21, danakj wrote:
> https://codereview.chromium.org/879423002/diff/1/cc/test/layer_test_common.cc
> File cc/test/layer_test_common.cc (right):
> 
>
https://codereview.chromium.org/879423002/diff/1/cc/test/layer_test_common.cc...
> cc/test/layer_test_common.cc:138: void
> LayerTestCommon::LayerImplTest::AppendQuadsForPassWithOcclusion(
> On 2015/01/28 22:05:27, vmpstr wrote:
> > Should the rest of the functions also take a render_pass instead of using
> > |render_pass_|? It's not a big concern since this is test, but it's pretty
> easy
> > in this function to accidentally use render_pass_ instead of render_pass
> 
> We could, it would just mean a bunch of code at callsites creating
renderpasses
> to pass in. I could rename the local variable if that helps?

Either that or just drop a comment saying "Note: use passed in render_pass
instead of member variable for this function" or something like that. 

lgtm other than that

[danakj, 2015-01-28 22:18:06]

The CQ bit was checked by danakj@chromium.org

[danakj, 2015-01-28 22:18:22]

Done. given_render_pass.

On Wed, Jan 28, 2015 at 2:07 PM, <vmpstr@chromium.org> wrote:

> On 2015/01/28 22:06:21, danakj wrote:
>
>> https://codereview.chromium.org/879423002/diff/1/cc/test/
>> layer_test_common.cc
>> File cc/test/layer_test_common.cc (right):
>>
>
>
> https://codereview.chromium.org/879423002/diff/1/cc/test/
> layer_test_common.cc#newcode138
>
>> cc/test/layer_test_common.cc:138: void
>> LayerTestCommon::LayerImplTest::AppendQuadsForPassWithOcclusion(
>> On 2015/01/28 22:05:27, vmpstr wrote:
>> > Should the rest of the functions also take a render_pass instead of
>> using
>> > |render_pass_|? It's not a big concern since this is test, but it's
>> pretty
>> easy
>> > in this function to accidentally use render_pass_ instead of render_pass
>>
>
>  We could, it would just mean a bunch of code at callsites creating
>>
> renderpasses
>
>> to pass in. I could rename the local variable if that helps?
>>
>
> Either that or just drop a comment saying "Note: use passed in render_pass
> instead of member variable for this function" or something like that.
>
> lgtm other than that
>
> https://codereview.chromium.org/879423002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2015-01-28 22:19:25]

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

[commit-bot: I haz the power, 2015-01-29 00:07:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-29 00:08:06]

Patchset 2 (id:??) landed as
https://crrev.com/4c9b207ec1d2076864f8f0dfd4dab8acb25196c9
Cr-Commit-Position: refs/heads/master@{#313630}