Add a command-line flag to set GPU rasterization multisampling.

Add a command-line flag to set GPU rasterization multisampling.

The new flag is
    --gpu-rasterization-msaa-sample-count=X

similar to the existing
    --canvas-msaa-sample-count=X

where X is the sample count.

Committed: https://crrev.com/b60ba95e91d9b3cb2193b39d86301e01acf84c1b
Cr-Commit-Position: refs/heads/master@{#313322}

[Stephen White, 2015-01-26 19:59:23]

senorblanco@chromium.org changed reviewers:
+ danakj@chromium.org

[Stephen White, 2015-01-26 19:59:24]

Dana: PTAL. Thanks!

[danakj, 2015-01-26 21:04:32]

https://codereview.chromium.org/881513004/diff/1/content/browser/gpu/composit...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/881513004/diff/1/content/browser/gpu/composit...
content/browser/gpu/compositor_util.cc:318:
command_line.GetSwitchValueASCII(switches::kGpuRasterizationMSAASampleCount);
git cl format

https://codereview.chromium.org/881513004/diff/1/content/browser/gpu/composit...
content/browser/gpu/compositor_util.cc:325: LOG(WARNING) << "Failed to parse
switch " <<
DLOG?

https://codereview.chromium.org/881513004/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/881513004/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:1299: // also be added
to chrome/browser/chromeos/login/chrome_restart_request.cc.
Please add the flag to this file so that you get the same behaviour in guest
mode.

https://codereview.chromium.org/881513004/diff/1/content/renderer/gpu/composi...
File content/renderer/gpu/compositor_dependencies.h (right):

https://codereview.chromium.org/881513004/diff/1/content/renderer/gpu/composi...
content/renderer/gpu/compositor_dependencies.h:33: virtual int
GpuRasterizationMSAASampleCount() = 0;
GetGpu*

https://codereview.chromium.org/881513004/diff/1/content/renderer/render_thre...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/881513004/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:593: base::StringToInt(string_value,
&gpu_rasterization_msaa_sample_count_);
git cl format

https://codereview.chromium.org/881513004/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:596:
DCHECK_LE(gpu_rasterization_msaa_sample_count_, 16);
magic number duplication, i'd maybe not do this

[Stephen White, 2015-01-26 22:39:51]

https://codereview.chromium.org/881513004/diff/1/content/browser/gpu/composit...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/881513004/diff/1/content/browser/gpu/composit...
content/browser/gpu/compositor_util.cc:318:
command_line.GetSwitchValueASCII(switches::kGpuRasterizationMSAASampleCount);
On 2015/01/26 21:04:32, danakj wrote:
> git cl format

Done.

https://codereview.chromium.org/881513004/diff/1/content/browser/gpu/composit...
content/browser/gpu/compositor_util.cc:325: LOG(WARNING) << "Failed to parse
switch " <<
TOn 2015/01/26 21:04:32, danakj wrote:
> DLOG?

This came from ForceNumberOfRendererRasterThreads below; I've fixed both.

https://codereview.chromium.org/881513004/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/881513004/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:1299: // also be added
to chrome/browser/chromeos/login/chrome_restart_request.cc.
On 2015/01/26 21:04:32, danakj wrote:
> Please add the flag to this file so that you get the same behaviour in guest
> mode.

Done.

https://codereview.chromium.org/881513004/diff/1/content/renderer/gpu/composi...
File content/renderer/gpu/compositor_dependencies.h (right):

https://codereview.chromium.org/881513004/diff/1/content/renderer/gpu/composi...
content/renderer/gpu/compositor_dependencies.h:33: virtual int
GpuRasterizationMSAASampleCount() = 0;
On 2015/01/26 21:04:32, danakj wrote:
> GetGpu*

Done.

https://codereview.chromium.org/881513004/diff/1/content/renderer/render_thre...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/881513004/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:593: base::StringToInt(string_value,
&gpu_rasterization_msaa_sample_count_);
On 2015/01/26 21:04:32, danakj wrote:
> git cl format

Done.

https://codereview.chromium.org/881513004/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:596:
DCHECK_LE(gpu_rasterization_msaa_sample_count_, 16);
On 2015/01/26 21:04:32, danakj wrote:
> magic number duplication, i'd maybe not do this

Good idea. In fact, there's no reason for an upper max at all; Skia will just
ignore it if it's wacky. Removed everywhere.

[danakj, 2015-01-26 22:49:12]

LGTM

https://codereview.chromium.org/881513004/diff/40001/content/renderer/render_...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/881513004/diff/40001/content/renderer/render_...
content/renderer/render_thread_impl.cc:595:
DCHECK_GT(gpu_rasterization_msaa_sample_count_, 0);
Should this be GE? Browser could pass 0?

[Stephen White, 2015-01-26 22:52:03]

senorblanco@chromium.org changed reviewers:
+ piman@chromium.org

[Stephen White, 2015-01-26 22:52:13]

+piman for OWNERS

[Stephen White, 2015-01-26 22:53:24]

On 2015/01/26 22:49:12, danakj wrote:
> LGTM
> 
>
https://codereview.chromium.org/881513004/diff/40001/content/renderer/render_...
> File content/renderer/render_thread_impl.cc (right):
> 
>
https://codereview.chromium.org/881513004/diff/40001/content/renderer/render_...
> content/renderer/render_thread_impl.cc:595:
> DCHECK_GT(gpu_rasterization_msaa_sample_count_, 0);
> Should this be GE? Browser could pass 0?

Good catch! Fixed.

[Stephen White, 2015-01-27 17:57:26]

Dana: made a slight tweak to this CL to forward the flag from browser ->
renderer, regardless of whether gpu-rasterization is enabled (since it may be
forced). This fixes the functionality on OSX.

Feel free to take another look, or ignore if you trust me. :)

[danakj, 2015-01-27 17:59:36]

https://codereview.chromium.org/881513004/diff/100001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/881513004/diff/100001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:1084: if
(GpuRasterizationMSAASampleCount() > 0) {
FWIW You could remove the if() here and the if(HasSwitch) in the
RenderThreadImpl.

https://codereview.chromium.org/881513004/diff/100001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:1097: if
(IsForceGpuRasterizationEnabled())
Ah ya, I missed this, sorry. LGTM

[piman, 2015-01-27 18:07:16]

lgtm

https://codereview.chromium.org/881513004/diff/100001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/881513004/diff/100001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:1084: if
(GpuRasterizationMSAASampleCount() > 0) {
nit: cache the return value of GpuRasterizationMSAASampleCount()? It parses
strings, and we use it twice.

[Stephen White, 2015-01-27 18:11:46]

https://codereview.chromium.org/881513004/diff/100001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/881513004/diff/100001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:1084: if
(GpuRasterizationMSAASampleCount() > 0) {
On 2015/01/27 18:07:16, piman (Very slow to review) wrote:
> nit: cache the return value of GpuRasterizationMSAASampleCount()? It parses
> strings, and we use it twice.

Done.

[Stephen White, 2015-01-27 18:11:55]

The CQ bit was checked by senorblanco@chromium.org

[commit-bot: I haz the power, 2015-01-27 18:13:02]

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

[commit-bot: I haz the power, 2015-01-27 19:13:09]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-27 19:14:20]

Patchset 7 (id:??) landed as
https://crrev.com/b60ba95e91d9b3cb2193b39d86301e01acf84c1b
Cr-Commit-Position: refs/heads/master@{#313322}