Type conversion fixes, gpu/ edition.

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 20c4da8efce5dea9ded6e230dde087e69c9cf0b9..600dd46e69f10769d7a8be9251f4978bd1884ea6 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -3131,7 +3131,7 @@ bool GLES2DecoderImpl::CheckFramebufferValid(
       return false;
     if (backbuffer_needs_clear_bits_) {
       glClearColor(0, 0, 0, (GLES2Util::GetChannelsForFormat(
-          offscreen_target_color_format_) & 0x0008) != 0 ? 0 : 1);
+          offscreen_target_color_format_) & 0x0008) != 0 ? 0 : 1.f);
       state_.SetDeviceColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
       glClearStencil(0);
       state_.SetDeviceStencilMaskSeparate(GL_FRONT, kDefaultStencilMask);
@@ -3695,7 +3695,7 @@ bool GLES2DecoderImpl::ResizeOffscreenFrameBuffer(const gfx::Size& size) {
   {
     ScopedFrameBufferBinder binder(this, offscreen_target_frame_buffer_->id());
     glClearColor(0, 0, 0, (GLES2Util::GetChannelsForFormat(
-        offscreen_target_color_format_) & 0x0008) != 0 ? 0 : 1);
+        offscreen_target_color_format_) & 0x0008) != 0 ? 0 : 1.f);
     state_.SetDeviceColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
     glClearStencil(0);
     state_.SetDeviceStencilMaskSeparate(GL_FRONT, kDefaultStencilMask);
@@ -7215,7 +7215,7 @@ error::Error GLES2DecoderImpl::HandleVertexAttribPointer(
   GLuint indx = c.indx;
   GLint size = c.size;
   GLenum type = c.type;
-  GLboolean normalized = c.normalized;
+  GLboolean normalized = !!c.normalized;

[Peter Kasting, 2014/10/16 23:20:11]

This is a bit subtle.

c.normalized is a 32-bit value, while GLboolean is an 8-bit type (not a bool).

Therefore, an explicit or implicit cast directly to GLboolean is functionally
equivalent to something like:

  normalized = c.normalized & 0xff;

...meaning that, depending on the value of c.normalized, this could convert a
nonzero source value into a zero destination, or convert something into a
destination value other than 0 or 1.

I assumed this wasn't the intent of the code, and the real intent was more like:

  normalized = c.normalized != 0;

"!!" is simply a shorter way of writing that same expression.  (2 places)

[no sievers, 2014/10/16 23:52:08]

Yes this looks good since this gets set from gles2_cmd_helper_autogen.h where it
expands GLBoolean to 32bits. It's just that in shared memory we want every field
to be 32bit so we can use pragma pack(4).

[Peter Kasting, 2014/10/17 00:00:40]

I see.

I wonder if it might be better to use static_cast<GLboolean>, then, if the
original source value was a GLboolean.  That way, if for some insane reason
someone had a use for an original value > 1, it would be preserved correctly
here.

Which do you think is better?

[no sievers, 2014/10/17 00:06:24]

Yea maybe it's more correct to do the inverse (i.e. static_cast<GLBoolean>) of
the expansion we do in the client (implicit conversion to 32bit) here.

Either way it would be a client bug if that affects behavior :)

   GLsizei stride = c.stride;
   GLsizei offset = c.offset;
   const void* ptr = reinterpret_cast<const void*>(offset);
@@ -7457,7 +7457,7 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32 immediate_data_size,
   GLsizei height = c.height;
   GLenum format = c.format;
   GLenum type = c.type;
-  GLboolean async = c.async;
+  GLboolean async = !!c.async;

[no sievers, 2014/10/16 23:52:08]

same here

   if (width < 0 || height < 0) {
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glReadPixels", "dimensions < 0");
     return error::kNoError;