gltrace: Fix glBitmap / glDrawPixels(GL_BITMAP) blob size calculation.

A superficial reading of the OpenGL specification gives the idea that
GL_UNPACK_ALIGMENT should be ignored in some cases, but if one takes in
consideration all the different constraints, it actually follows that
the row stride must be aligned to unpack alignment, one way or the other.

This is particularly true for GL_BITMAPs, whose size was being
underestimated if GL_UNPACK_ALIGMENT != 1.

This also matches what Mesa does.

1 files changed, 26 insertions, 39 deletions

diff --git a/helpers/glsize.hpp b/helpers/glsize.hpp
index 9557a612..9d7efbec 100644
--- a/helpers/glsize.hpp
+++ b/helpers/glsize.hpp
@@ -721,37 +721,28 @@ _align(X x, Y y) {
     return (x + (y - 1)) & ~(y - 1);
 }
 
-static inline void
-_gl_format_size(GLenum format, GLenum type,
-                unsigned & bits_per_element, unsigned & bits_per_pixel)
+static inline unsigned
+_gl_format_size(GLenum format, GLenum type)
 {
-    unsigned num_channels = _gl_format_channels(format);
+    unsigned num_elements = _gl_format_channels(format);
 
     switch (type) {
     case GL_BITMAP:
-        bits_per_pixel = bits_per_element = 1;
-        break;
+        return 1;
     case GL_BYTE:
     case GL_UNSIGNED_BYTE:
-        bits_per_element = 8;
-        bits_per_pixel = bits_per_element * num_channels;
-        break;
+        return 8 * num_elements;
     case GL_SHORT:
     case GL_UNSIGNED_SHORT:
     case GL_HALF_FLOAT:
-        bits_per_element = 16;
-        bits_per_pixel = bits_per_element * num_channels;
-        break;
+        return 16 * num_elements;
     case GL_INT:
     case GL_UNSIGNED_INT:
     case GL_FLOAT:
-        bits_per_element = 32;
-        bits_per_pixel = bits_per_element * num_channels;
-        break;
+        return 32 * num_elements;
     case GL_UNSIGNED_BYTE_3_3_2:
     case GL_UNSIGNED_BYTE_2_3_3_REV:
-        bits_per_pixel = bits_per_element = 8;
-        break;
+        return 8;
     case GL_UNSIGNED_SHORT_4_4_4_4:
     case GL_UNSIGNED_SHORT_4_4_4_4_REV:
     case GL_UNSIGNED_SHORT_5_5_5_1:
@@ -760,8 +751,7 @@ _gl_format_size(GLenum format, GLenum type,
     case GL_UNSIGNED_SHORT_5_6_5_REV:
     case GL_UNSIGNED_SHORT_8_8_MESA:
     case GL_UNSIGNED_SHORT_8_8_REV_MESA:
-        bits_per_pixel = bits_per_element = 16;
-        break;
+        return 16;
     case GL_UNSIGNED_INT_8_8_8_8:
     case GL_UNSIGNED_INT_8_8_8_8_REV:
     case GL_UNSIGNED_INT_10_10_10_2:
@@ -771,32 +761,27 @@ _gl_format_size(GLenum format, GLenum type,
     case GL_UNSIGNED_INT_5_9_9_9_REV:
     case GL_UNSIGNED_INT_S8_S8_8_8_NV:
     case GL_UNSIGNED_INT_8_8_S8_S8_REV_NV:
-        bits_per_pixel = bits_per_element = 32;
-        break;
+        return 32;
     case GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
-        bits_per_pixel = bits_per_element = 64;
-        break;
+        assert(num_elements == 2);
+        return 32 * 2;
     default:
         os::log("apitrace: warning: %s: unexpected type GLenum 0x%04X\n", __FUNCTION__, type);
-        bits_per_pixel = bits_per_element = 0;
-        break;
+        return 0;
     }
 }
 
 static inline size_t
-_glClearBufferData_size(GLenum format, GLenum type) {
-    unsigned bits_per_element;
-    unsigned bits_per_pixel;
-    _gl_format_size(format, type, bits_per_element, bits_per_pixel);
+_glClearBufferData_size(GLenum format, GLenum type)
+{
+    unsigned bits_per_pixel = _gl_format_size(format, type);
     return (bits_per_pixel + 7)/8;
 }
 
 static inline size_t
-_gl_image_size(GLenum format, GLenum type, GLsizei width, GLsizei height, GLsizei depth, GLboolean has_unpack_subimage) {
-
-    unsigned bits_per_element;
-    unsigned bits_per_pixel;
-    _gl_format_size(format, type, bits_per_element, bits_per_pixel);
+_gl_image_size(GLenum format, GLenum type, GLsizei width, GLsizei height, GLsizei depth, GLboolean has_unpack_subimage)
+{
+    unsigned bits_per_pixel = _gl_format_size(format, type);
 
     GLint alignment = 4;
     GLint row_length = 0;
@@ -820,11 +805,13 @@ _gl_image_size(GLenum format, GLenum type, GLsizei width, GLsizei height, GLsize
 
     size_t row_stride = (row_length*bits_per_pixel + 7)/8;
 
-    if ((bits_per_element == 1*8 ||
-         bits_per_element == 2*8 ||
-         bits_per_element == 4*8 ||
-         bits_per_element == 8*8) &&
-        (GLint)bits_per_element < alignment*8) {
+    /*
+     * The OpenGL specification states that the unpack alignment should be
+     * ignored if the number of bits per element is not 1, 2, 4, or 8 times the
+     * number of bits in a GL ubyte, but the matter of fact is that the number
+     * of bits per element is always one of those.
+     */
+    if (_is_pot(alignment)) {
         row_stride = _align(row_stride, alignment);
     }