cc: Move DisplayItem::Raster up to DisplayItemList

cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types. Makes the fields public like DrawQuads, but
also const.

R=enne@chromium.org
BUG=671440
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2751833007
Cr-Commit-Position: refs/heads/master@{#457439}
Committed: https://chromium.googlesource.com/chromium/src/+/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2

[danakj, 2017-03-15 18:10:10]

Description was changed from

==========
cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types.

R=enne@chromium.org
BUG=671440
==========

to

==========
cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types.

R=enne@chromium.org
BUG=671440
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[danakj, 2017-03-15 18:26:13]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 18:26:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danakj, 2017-03-15 18:58:28]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 18:58:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danakj, 2017-03-15 18:58:47]

Description was changed from

==========
cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types.

R=enne@chromium.org
BUG=671440
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types. Makes the fields public like DrawQuads, but
also const.

R=enne@chromium.org
BUG=671440
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmpstr, 2017-03-15 19:24:57]

vmpstr@chromium.org changed reviewers:
+ vmpstr@chromium.org

[vmpstr, 2017-03-15 19:24:58]

lgtm, do you have performance numbers on this change?

[danakj, 2017-03-15 19:38:13]

The CQ bit was unchecked by danakj@chromium.org

[enne (OOO), 2017-03-15 19:39:17]

I like it!! lgtm

[danakj, 2017-03-15 19:59:21]

Here's a raster and record run on linux because we don't run any gfx tests
except on linux anymore apparently? https://codereview.chromium.org/2751833007 

And a bug about that:
https://bugs.chromium.org/p/chromium/issues/detail?id=701933

[danakj, 2017-03-15 20:01:23]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 20:02:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 20:50:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 20:50:18]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-03-15 21:04:40]

On 2017/03/15 19:59:21, danakj wrote:
> Here's a raster and record run on linux because we don't run any gfx tests
> except on linux anymore apparently? https://codereview.chromium.org/2751833007


Apparently that try job might never run idk. Running one on
https://ct.skia.org/chromium_perf_runs/ now

[danakj, 2017-03-16 15:09:56]

I don't see any big number changes:
https://00e9e64bac5f8687d242cdcbf2a7e6c24f52aab3f37164ece2-apidata.googleuser...

Rasterize times changes less than record times which are not affected by this
patch, so seems like noise.

[danakj, 2017-03-16 15:10:04]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2017-03-16 15:10:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 15:17:02]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489677004859070,
"parent_rev": "e5fcd337dd2828a5eb9e911bbc2701f720a9cc40", "commit_rev":
"0920d170ce0af366a0d1c386e63a2a2ab88a8dd2"}

[commit-bot: I haz the power, 2017-03-16 15:17:50]

Description was changed from

==========
cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types. Makes the fields public like DrawQuads, but
also const.

R=enne@chromium.org
BUG=671440
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types. Makes the fields public like DrawQuads, but
also const.

R=enne@chromium.org
BUG=671440
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2751833007
Cr-Commit-Position: refs/heads/master@{#457439}
Committed:
https://chromium.googlesource.com/chromium/src/+/0920d170ce0af366a0d1c386e63a...
==========

[commit-bot: I haz the power, 2017-03-16 15:17:51]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/0920d170ce0af366a0d1c386e63a...

[vmpstr, 2017-03-16 17:54:39]

On 2017/03/16 15:09:56, danakj wrote:
> I don't see any big number changes:
>
https://00e9e64bac5f8687d242cdcbf2a7e6c24f52aab3f37164ece2-apidata.googleuser...
> 
> Rasterize times changes less than record times which are not affected by this
> patch, so seems like noise.

Cool, thanks for testing!

[danakj, 2017-03-16 17:59:55]

That link is wild, this one is better:
https://storage.cloud.google.com/cluster-telemetry/tasks/chromium_perf_runs/d...

On Thu, Mar 16, 2017 at 1:54 PM, <vmpstr@chromium.org> wrote:

> On 2017/03/16 15:09:56, danakj wrote:
> > I don't see any big number changes:
> >
> https://00e9e64bac5f8687d242cdcbf2a7e6c24f52aab3f37164ece2-apidata.
> googleusercontent.com/download/storage/v1/b/cluster-
> telemetry/o/tasks%2Fchromium_perf_runs%2Fdanakj-
> 20170315210016%2Fhtml%2Findex.html?qk=AD5uMEsFIlTMarn-
> qtVSZN9PZoazw08qZObNNIMfBje22WXhHuRYZYJOJUvMkbp1ReMZJ2Z80_
> WNv92WhnlMaKMXcSNju3mUUIdCKxhZCAfMEknW24HSJSsOC2SNI8pOWipD81
> U_HdL9pJ9aBQuk7e3JhHzOX7cE3KF-SKxqVt5BZkpvpR8gLi_oc0ELWms_
> D0Zqb387v5GT5d0LMP1ZkfOJc6D7bURFvpxjpEdBftmAHdtJcwE_IGYkquAKVi8dmunQjrEO-
> 35L4FNWlvBuGW3ts7jihHpOOyx3T5XkZOmbc7_9A3Xw-NSuUj_KPtA5p_
> hJ4mkfF7XYoMI9P9M1gNHahzrkmWW7GZt4L1y9yuBxuR0m7Tmg9MjEP2m7Lb
> Y9HL7DpoJRBSa10irTHBmTwuUleFApbJ2Qtb2nXZl_L2wLAf0qrXtwtyoYWY5V9gslF1py9A
> 7xf1H4bQZq0xPHyIO_cOdn5FlCrBMQ8waw3HCUahgoO-safo3ZOglfGZjIf5vH1ZK9Vv9JYtLa
> j3VpC3KvN3SNhmyCkyF11uHHbkVM8I0UQR3tksOIygn1T7q4ucBpn89HFumQ-J_
> sa1WutNme7DWqOTya1JZZGBtbWkuuGfumkU0cAhwBf92nmxBGIjiBcnzDT2Z
> seOj8tw7FdKZDhMafQaS5T9JY8LRFQTXiLdmezAHm_V2dYa1Wy0L5S2R-
> CEB2Iizol6dw88kLIfeDTtbxT9N2iT4hKtMUd955c2ySRbotwW0WgqndPchqVI-
> 4P3tuAsdVdgwjP9aEUphpUKWuct3UWi9phqPel5zi6I7_n4gxDbhQvuKuHknlOEBxXiUHRk-D-
> biYIYxjpGPTsB4AdOBpUkVJ1aV0P_SNTy-Jo2sm7CI
> >
> > Rasterize times changes less than record times which are not affected by
> this
> > patch, so seems like noise.
>
> Cool, thanks for testing!
>
> https://codereview.chromium.org/2751833007/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.