Fix glTexSubImage2D for non-32bpp formats.

cc/texture_uploader.cc

Index: cc/texture_uploader.cc
diff --git a/cc/texture_uploader.cc b/cc/texture_uploader.cc
index 64b3de52424a4ed223a204145ef769843bfe7c79..6c6044ee2e0bb45647adb918a3e9629fe01ad83f 100644
--- a/cc/texture_uploader.cc
+++ b/cc/texture_uploader.cc
@@ -24,7 +24,8 @@ static const size_t uploadHistorySize = 100;
 
 // Global estimated number of textures per second to maintain estimates across
 // subsequent instances of TextureUploader.
-// More than one thread will not access this variable, so we do not need to synchronize access.
+// More than one thread will not access this variable, so we do not need to
+// synchronize access.

[enne (OOO), 2012/10/22 20:36:14]

Please do not update style along with behavior changes in the same patch.

[sheu, 2012/10/22 21:31:59]

I thought this would be small enough to slip in.  I guess not :-)

 static double estimatedTexturesPerSecondGlobal = 48.0 * 60.0;
 
 } // anonymous namespace
@@ -61,14 +62,16 @@ void TextureUploader::Query::end()
 bool TextureUploader::Query::isPending()
 {
     unsigned available = 1;
-    m_context->getQueryObjectuivEXT(m_queryId, GL_QUERY_RESULT_AVAILABLE_EXT, &available);
+    m_context->getQueryObjectuivEXT(m_queryId, GL_QUERY_RESULT_AVAILABLE_EXT,
+                                    &available);
     return !available;
 }
 
 unsigned TextureUploader::Query::value()
 {
     if (!m_hasValue) {
-        m_context->getQueryObjectuivEXT(m_queryId, GL_QUERY_RESULT_EXT, &m_value);
+        m_context->getQueryObjectuivEXT(m_queryId, GL_QUERY_RESULT_EXT,
+                                        &m_value);
         m_hasValue = true;
     }
     return m_value;
@@ -123,7 +126,8 @@ double TextureUploader::estimatedTexturesPerSecond()
 {
     processQueries();
 
-    // The history should never be empty because we initialize all elements with an estimate.
+    // The history should never be empty because we initialize all elements with
+    // an estimate.
     DCHECK(m_texturesPerSecondHistory.size() == uploadHistorySize);
 
     // Sort the history and use the median as our estimate.
@@ -131,8 +135,10 @@ double TextureUploader::estimatedTexturesPerSecond()
                                       m_texturesPerSecondHistory.end());
     std::sort(sortedHistory.begin(), sortedHistory.end());
 
-    estimatedTexturesPerSecondGlobal = sortedHistory[sortedHistory.size() * 2 / 3];
-    TRACE_COUNTER1("cc", "estimatedTexturesPerSecond", estimatedTexturesPerSecondGlobal);
+    estimatedTexturesPerSecondGlobal =
+        sortedHistory[sortedHistory.size() * 2 / 3];
+    TRACE_COUNTER1("cc", "estimatedTexturesPerSecond",
+                   estimatedTexturesPerSecondGlobal);
     return estimatedTexturesPerSecondGlobal;
 }
 
@@ -212,10 +218,27 @@ void TextureUploader::uploadWithTexSubImage(const uint8_t* image,
                     source_rect.y() - image_rect.y());
 
     const uint8_t* pixel_source;
+
+    unsigned int components_per_pixel = 0;
+    unsigned int bytes_per_component = 1;
+    switch (format) {

[enne (OOO), 2012/10/22 20:36:14]

Can you not duplicate this switch statement from below? Maybe this can go into a
static helper function?

It looks like texture.cc also makes this same mistake.

[sheu, 2012/10/22 21:31:59]

Done.

+    case GL_RGBA:
+    case GL_BGRA_EXT:
+        components_per_pixel = 4;
+        break;
+    case GL_LUMINANCE:
+        components_per_pixel = 1;
+        break;
+    default:
+        NOTREACHED();
+    }
+    unsigned int bytes_per_pixel = components_per_pixel * bytes_per_component;
+
     if (image_rect.width() == source_rect.width() && !offset.x()) {
-        pixel_source = &image[4 * offset.y() * image_rect.width()];
+        pixel_source = &image[bytes_per_pixel * offset.y() * image_rect.width()];
     } else {
-        size_t needed_size = source_rect.width() * source_rect.height() * 4;
+        size_t needed_size =
+            source_rect.width() * source_rect.height() * bytes_per_pixel;
         if (m_subImageSize < needed_size) {
             m_subImage.reset(new uint8_t[needed_size]);
             m_subImageSize = needed_size;
@@ -223,10 +246,10 @@ void TextureUploader::uploadWithTexSubImage(const uint8_t* image,
         // Strides not equal, so do a row-by-row memcpy from the
         // paint results into a temp buffer for uploading.
         for (int row = 0; row < source_rect.height(); ++row)
-            memcpy(&m_subImage[source_rect.width() * 4 * row],
-                   &image[4 * (offset.x() +
+            memcpy(&m_subImage[source_rect.width() * bytes_per_pixel * row],
+                   &image[bytes_per_pixel * (offset.x() +
                                (offset.y() + row) * image_rect.width())],
-                   source_rect.width() * 4);
+                   source_rect.width() * bytes_per_pixel);
 
         pixel_source = &m_subImage[0];
     }
@@ -296,6 +319,7 @@ void TextureUploader::uploadWithMapTexSubImage(const uint8_t* image,
     }
 
     unsigned int components_per_pixel = 0;
+    unsigned int bytes_per_component = 1;
     switch (format) {
     case GL_RGBA:
     case GL_BGRA_EXT:
@@ -307,7 +331,6 @@ void TextureUploader::uploadWithMapTexSubImage(const uint8_t* image,
     default:
         NOTREACHED();
     }
-    unsigned int bytes_per_component = 1;
     unsigned int bytes_per_pixel = components_per_pixel * bytes_per_component;
 
     if (image_rect.width() == source_rect.width() && !offset.x()) {
@@ -319,7 +342,7 @@ void TextureUploader::uploadWithMapTexSubImage(const uint8_t* image,
         // paint results into the pixelDest
         for (int row = 0; row < source_rect.height(); ++row)
             memcpy(&pixel_dest[source_rect.width() * row * bytes_per_pixel],
-                   &image[4 * (offset.x() +
+                   &image[bytes_per_pixel * (offset.x() +
                                (offset.y() + row) * image_rect.width())],
                    source_rect.width() * bytes_per_pixel);
     }
@@ -334,7 +357,8 @@ void TextureUploader::processQueries()
             break;
 
         unsigned usElapsed = m_pendingQueries.first()->value();
-        HISTOGRAM_CUSTOM_COUNTS("Renderer4.TextureGpuUploadTimeUS", usElapsed, 0, 100000, 50);
+        HISTOGRAM_CUSTOM_COUNTS("Renderer4.TextureGpuUploadTimeUS", usElapsed,
+            0, 100000, 50);
 
         if (!m_pendingQueries.first()->isNonBlocking())
             m_numBlockingTextureUploads--;