Use MSAA in GPU rasterization if Skia suggests it

Use MSAA in GPU rasterization if Skia suggests it

Query the recommended sample count for the context from Skia.
This will select either 'gpu' or 'msaa' backend of Skia, depending
on the context capabilities and the display density.

BUG=347962

[Kimmo Kinnunen, 2014-02-28 13:38:34]

Would this be acceptable?

[nduca, 2014-03-01 21:59:12]

i dont like the approach of command line flags to control rasterization choices.
can this be determined from renderer string or some other dynamic thing?

[nduca, 2014-03-01 22:02:28]

https://codereview.chromium.org/184573002/diff/30001/cc/resources/resource_pr...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/184573002/diff/30001/cc/resources/resource_pr...
cc/resources/resource_provider.cc:444: desc.fSampleCnt =
resource_provider()->num_gpu_rasterization_samples();
i think it woudl be better if you obtained this from ganesh. that is, skia
should decide msaa mode, then we should just grcontext for the preferred raster
sample count. Eg gr_context->getRasterSampleCount(). Then skia can take the
burden/logistics on of testing for different msaa modes if thats what they are
wanting to support for some GL renderer string for instance. and from the
compositors point of view, we're not involved.

[Kimmo Kinnunen, 2014-03-03 06:12:29]

On 2014/03/01 22:02:28, nduca wrote:
> i think it woudl be better if you obtained this from ganesh. that is, skia
> should decide msaa mode, then we should just grcontext for the preferred
raster
> sample count. Eg gr_context->getRasterSampleCount(). 

Ok. I think, currently, the design is that caller makes the choice of whether or
not to use MSAA. It is similar to the caller selecting whether or not to use sw
rasterizer or hw rasterizer. As is visible from the API today, MSAA is just
another property when creating the renderable target, just like width and
height.

Brian, any ideas how the above could be achieved?

[Kimmo Kinnunen, 2014-03-03 06:37:26]

One obvious hurdle is that in order to develop non-default MSAA backend, one
would need to have a flag. The flags can be only added to Chrome, not to Skia.
Thus it'd be fairly cumbersome to develop MSAA with Chrome, as one would need to
have an off-the-tree patch locally during the development until MSAA can be
enabled by default.

[bsalomon, 2014-03-03 14:08:48]

On 2014/03/03 06:12:29, kkinnunen wrote:
> On 2014/03/01 22:02:28, nduca wrote:
> > i think it woudl be better if you obtained this from ganesh. that is, skia
> > should decide msaa mode, then we should just grcontext for the preferred
> raster
> > sample count. Eg gr_context->getRasterSampleCount(). 
> 
> Ok. I think, currently, the design is that caller makes the choice of whether
or
> not to use MSAA. It is similar to the caller selecting whether or not to use
sw
> rasterizer or hw rasterizer. As is visible from the API today, MSAA is just
> another property when creating the renderable target, just like width and
> height.
> 
> Brian, any ideas how the above could be achieved?

Right, currently we rely on the client to tell us whether to use MSAA or not. I
suppose we could have a function that's something like
getRecommendedSampleCount(). There is one piece of information we would want the
client to tell us, though: the DPI that the target will be displayed at, so that
we can determine a sufficient sample count if we decide to use MSAA.

Another wrinkle in this is that Chromium currently hides the renderer string
from Ganesh. We do see the extension list, though. So we could base the decision
(initially) on the presence of the NVPR extension.

[Kimmo Kinnunen, 2014-03-06 07:16:00]

How about this?

The related Skia patch is:
https://codereview.chromium.org/188403003/

[Kimmo Kinnunen, 2014-03-06 07:16:51]

https://chromiumcodereview.appspot.com/184573002/diff/50001/cc/resources/reso...
File cc/resources/resource_provider.cc (right):

https://chromiumcodereview.appspot.com/184573002/diff/50001/cc/resources/reso...
cc/resources/resource_provider.cc:615: kDisplayDensityScaleFactor *
output_surface_->SurfaceScaleFactor();
Not entirely sure if this call is thread-safe. Is it?

[Kimmo Kinnunen, 2014-04-23 12:29:48]

Ping.. Anybody know who would be appropriate person to look at this?

[nduca, 2014-04-23 23:07:23]

https://codereview.chromium.org/184573002/diff/90001/cc/resources/resource_pr...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/184573002/diff/90001/cc/resources/resource_pr...
cc/resources/resource_provider.cc:64: // Display density in dots per inch at
scale factor 1.0.
what the heck is this? why do you need this?

https://codereview.chromium.org/184573002/diff/90001/cc/resources/resource_pr...
cc/resources/resource_provider.cc:454: if (surface_generation_id_ !=
surface_->generationID()) {
can you do this as a separate change please?