Enable anti-alias in WebGL if only GL_EXT_multisampled_render_to_texture is supported.

Enable anti-alias in WebGL if only GL_EXT_multisampled_render_to_texture is supported.

BUG=429559
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184901

[johnson.lin, 2014-10-30 04:48:49]

johnson.lin@intel.com changed reviewers:
+ rtoy@chromium.org

[johnson.lin, 2014-10-30 04:48:50]

GL_EXT_multisampled_render_to_texture is an extension to support anti-alias in
OpenGLES2.0. It was disabled. This CL is to enable it.

[Raymond Toy, 2014-10-30 17:00:36]

Sorry, I'm not qualified to review this. I don't know anything about this code.

[johnson.lin, 2014-10-31 01:25:01]

johnson.lin@intel.com changed reviewers:
+ adamk@chromium.org, darin@chromium.org, dglazkov@chromium.org,
dominik.rottsches@intel.com, eae@chromium.org, eroman@chromium.org,
eseidel@chromium.org, esprehn@chromium.org, fmalita@chromium.org,
haraken@chromium.org, jchaffraix@chromium.org, jochen@chromium.org,
junov@chromium.org, kbr@chromium.org, leviw@chromium.org, mkwst@chromium.org,
pdr@chromium.org, pfeldman@chromium.org, schenney@chromium.org,
senorblanco@chromium.org, steveblock@chromium.org, thakis@chromium.org,
tkent@chromium.org, vollick@chromium.org

[Julien - ping for review, 2014-10-31 17:25:58]

jchaffraix@chromium.org changed reviewers:
- adamk@chromium.org, darin@chromium.org, dglazkov@chromium.org,
dominik.rottsches@intel.com, eae@chromium.org, eroman@chromium.org,
eseidel@chromium.org, esprehn@chromium.org, fmalita@chromium.org,
haraken@chromium.org, jochen@chromium.org, leviw@chromium.org,
mkwst@chromium.org, pdr@chromium.org, pfeldman@chromium.org, rtoy@chromium.org,
schenney@chromium.org, senorblanco@chromium.org, steveblock@chromium.org,
thakis@chromium.org, tkent@chromium.org, vollick@chromium.org

[Julien - ping for review, 2014-10-31 17:25:58]

Please don't add all the OWNERS in platform/ to a review. Most of us have much
narrower area of expertise (not shown by the coarse OWNERS which is confusing).

It's a lot more impactful to add the reviewers who have been reviewing the files
you touch. Leaving only the reviewers who have looked at DrawingBuffer.cpp in
the past few months.

[Ken Russell (switch to Gerrit), 2014-10-31 23:23:42]

https://codereview.chromium.org/671643005/diff/1/Source/platform/graphics/gpu...
File Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/671643005/diff/1/Source/platform/graphics/gpu...
Source/platform/graphics/gpu/DrawingBuffer.cpp:98: ||
extensionsUtil->supportsExtension("GL_EXT_multisampled_render_to_texture"))
The code in DrawingBuffer::initialize already checks for the
GL_EXT_multisampled_render_to_texture extension, and I'm quite sure that code
path has been tested. Is your patch for the case where the hardware can't
support GL_CHROMIUM_framebuffer_multisample, but does support
GL_EXT_multisampled_render_to_texture? Do you have an example of such hardware
on which we could verify this fix?

Could you please file a bug about this and reference crbug.com/322706 ?

[johnson.lin, 2014-11-03 01:15:08]

@Ken Russell
Yes, the patch is to support device which only support
GL_EXT_multisampled_render_to_texture
In my test, I have met a lot such devices including ZTE Geek phone based on
Intel ATOM.

[johnson.lin, 2014-11-03 01:23:43]

I have file a bug
https://code.google.com/p/chromium/issues/detail?id=429559&thanks=429559&ts=1...

[Ken Russell (switch to Gerrit), 2014-11-03 22:47:52]

Thanks for filing the bug. I updated your CL. Please follow this pattern in the
future.

Could you please make this one small code change? I'll approve this CL
afterward. Thanks.

https://codereview.chromium.org/671643005/diff/1/Source/platform/graphics/gpu...
File Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/671643005/diff/1/Source/platform/graphics/gpu...
Source/platform/graphics/gpu/DrawingBuffer.cpp:100: if (multisampleSupported) {
Please factor out the call to ensureExtensionEnabled("GL_OES_rgb8_rgba8") so
it's only called once on this code path.

[johnson.lin, 2014-11-04 00:53:37]

Thanks will update code

[Ken Russell (switch to Gerrit), 2014-11-04 01:16:00]

Thanks. LGTM

[johnson.lin, 2014-11-04 01:49:20]

@Ken Russell, Will you commit it, thanks

[johnson.lin, 2014-11-04 05:02:43]

The CQ bit was checked by johnson.lin@intel.com

[johnson.lin, 2014-11-04 05:02:43]

The CQ bit was unchecked by johnson.lin@intel.com

[johnson.lin, 2014-11-04 05:02:44]

The CQ bit was checked by johnson.lin@intel.com

[johnson.lin, 2014-11-04 05:02:46]

The CQ bit was unchecked by johnson.lin@intel.com

[Ken Russell (switch to Gerrit), 2014-11-05 00:06:13]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2014-11-05 00:07:20]

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

[commit-bot: I haz the power, 2014-11-05 00:14:29]

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

[commit-bot: I haz the power, 2014-11-05 00:14:30]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)

[Ken Russell (switch to Gerrit), 2014-11-05 01:36:10]

https://codereview.chromium.org/671643005/diff/20001/Source/platform/graphics...
File Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/671643005/diff/20001/Source/platform/graphics...
Source/platform/graphics/gpu/DrawingBuffer.cpp:105:
extensionsUtil->ensureExtensionEnabled("GL_EXT_multisampled_render_to_texture");
This isn't syntactially correct. Please don't upload code until you've tested it
locally.

[johnson.lin, 2014-11-05 01:59:35]

@Ken Russell
The patch was moved from Crosswalk project, and there was type error
This time I have compiled and tested it locally to make sure it is working

[Ken Russell (switch to Gerrit), 2014-11-06 07:21:55]

On 2014/11/05 01:59:35, johnson.lin wrote:
> @Ken Russell
> The patch was moved from Crosswalk project, and there was type error
> This time I have compiled and tested it locally to make sure it is working

Thanks. Please continue to do this in the future.

[Ken Russell (switch to Gerrit), 2014-11-06 07:21:59]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2014-11-06 07:22:14]

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

[commit-bot: I haz the power, 2014-11-06 09:02:02]

Committed patchset #3 (id:40001) as 184901