Fix glTexSubImage2D for non-32bpp formats.

cc/texture_uploader.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 
 #include "cc/texture_uploader.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/debug/alias.h"
 #include "base/debug/trace_event.h"
 #include "base/metrics/histogram.h"
 #include "cc/prioritized_texture.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
 #include <public/WebGraphicsContext3D.h>
 
 namespace {
 
 // How many previous uploads to use when predicting future throughput.
 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
 
 namespace cc {
 
 TextureUploader::Query::Query(WebKit::WebGraphicsContext3D* context)
     : m_context(context)
     , m_queryId(0)
     , m_value(0)
@@ skipping 16 matching lines @@
 }
 
 void TextureUploader::Query::end()
 {
     m_context->endQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM);
 }
 
 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;
 }
 
 void TextureUploader::Query::markAsNonBlocking()
 {
     m_isNonBlocking = true;
 }
 
@@ skipping 34 matching lines @@
         (*it)->markAsNonBlocking();
     }
 
     DCHECK(!m_numBlockingTextureUploads);
 }
 
 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.
     std::vector<double> sortedHistory(m_texturesPerSecondHistory.begin(),
                                       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;
 }
 
 void TextureUploader::beginQuery()
 {
     if (m_availableQueries.isEmpty())
       m_availableQueries.append(Query::create(m_context));
 
     m_availableQueries.first()->begin();
 }
@@ skipping 59 matching lines @@
     base::debug::Alias(&image_rect_height);
     base::debug::Alias(&dest_offset_width);
     base::debug::Alias(&dest_offset_height);
     TRACE_EVENT0("cc", "TextureUploader::uploadWithTexSubImage");
 
     // Offset from image-rect to source-rect.
     IntPoint offset(source_rect.x() - image_rect.x(),
                     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;
         }
         // 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];
     }
 
     m_context->texSubImage2D(GL_TEXTURE_2D,
                              0,
                              dest_offset.width(),
                              dest_offset.height(),
                              source_rect.width(),
                              source_rect.height(),
@@ skipping 49 matching lines @@
                                             GL_UNSIGNED_BYTE,
                                             GL_WRITE_ONLY));
 
     if (!pixel_dest) {
         uploadWithTexSubImage(
             image, image_rect, source_rect, dest_offset, format);
         return;
     }
 
     unsigned int components_per_pixel = 0;
+    unsigned int bytes_per_component = 1;
     switch (format) {
     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_component = 1;
     unsigned int bytes_per_pixel = components_per_pixel * bytes_per_component;
 
     if (image_rect.width() == source_rect.width() && !offset.x()) {
         memcpy(pixel_dest,
                &image[offset.y() * image_rect.width() * bytes_per_pixel],
                image_rect.width() * source_rect.height() * bytes_per_pixel);
     } else {
         // Strides not equal, so do a row-by-row memcpy from the
         // 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);
     }
 
     m_context->unmapTexSubImage2DCHROMIUM(pixel_dest);
 }
 
 void TextureUploader::processQueries()
 {
     while (!m_pendingQueries.isEmpty()) {
         if (m_pendingQueries.first()->isPending())
             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--;
 
         // Remove the oldest values from our history and insert the new one
         double texturesPerSecond = 1.0 / (usElapsed * 1e-6);
         m_texturesPerSecondHistory.pop_back();
         m_texturesPerSecondHistory.push_front(texturesPerSecond);
 
         m_availableQueries.append(m_pendingQueries.takeFirst());
     }
 }
 
 }