cc: Fix BytesPerPixel issue and refactor.

cc/resources/resource_util.h

+// Copyright 2015 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.
+
+#ifndef CC_RESOURCES_RESOURCE_UTIL_H_
+#define CC_RESOURCES_RESOURCE_UTIL_H_
+
+#include <limits>
+
+#include "base/numerics/safe_math.h"
+#include "cc/base/cc_export.h"
+#include "cc/base/math_util.h"
+#include "cc/resources/resource_format.h"
+#include "ui/gfx/geometry/size.h"
+
+#define VERIFY_INTEGER_TYPE                         \

[ericrk, 2015/07/29 18:19:24]

This should be undefined at the end of the file to prevent it from leaking into
the global namespace.

Another option would be to move the template implementations to the cc file and
just explicitly define the int/size_t instantiations here, which would lead to a
linker error if someone tried to use this with a non size-t/int type.

Would be a bit cleaner maybe, but up to you.

[enne (OOO), 2015/07/29 18:21:21]

Yeah, I'd prefer not having a #define in a header file.

[prashant.n, 2015/07/30 18:45:02]

1. Removed macro.

2. Moving functions in .cc would have problem of having unneeded defintions.
e.g. CheckedSizeInBytes<size_t> is not needed. We can skip instantiating those,
but header file definitions would be more scalable.

Your point regarding avoiding other than size_t and int also looks good.

Kindly vote your opinion.

+  static_assert(std::numeric_limits<T>::is_integer, \
+                "T must be an integer type. Preferred type is size_t.")
+
+namespace cc {
+
+class CC_EXPORT ResourceUtil {
+ public:
+  // Returns true if the width is valid and fits in bytes, false otherwise.
+  template <typename T>
+  static bool VerifyWidthInBytes(int width, ResourceFormat format);
+  // Returns true if the size is valid and fits in bytes, false otherwise.
+  template <typename T>
+  static bool VerifySizeInBytes(const gfx::Size& size, ResourceFormat format);
+
+  // Dies with a CRASH() if the size can not be represented as a positive
+  // number of bytes.
+  template <typename T>
+  static T CheckedSizeInBytes(const gfx::Size& size, ResourceFormat format);
+
+  // Returns the width in bytes but may overflow or return 0. Only do this for
+  // computing widths for sizes that have already been checked.
+  template <typename T>
+  static T UncheckedWidthInBytes(int width, ResourceFormat format);
+  // Returns the size in bytes but may overflow or return 0. Only do this for
+  // sizes that have already been checked.
+  template <typename T>
+  static T UncheckedSizeInBytes(const gfx::Size& size, ResourceFormat format);
+  // Returns the width in bytes aligned but may overflow or return 0. Only do
+  // this for computing widths for sizes that have already been checked.
+  template <typename T>
+  static T UncheckedWidthInBytesAligned(int width, ResourceFormat format);
+  // Returns the size in bytes aligned but may overflow or return 0. Only do
+  // this for sizes that have already been checked.
+  template <typename T>
+  static T UncheckedSizeInBytesAligned(const gfx::Size& size,
+                                       ResourceFormat format);
+
+ private:
+  template <typename T>
+  static bool VerifyFitsInBytesInternal(int width,
+                                        int height,
+                                        ResourceFormat format,
+                                        bool verify_size,
+                                        bool aligned);
+
+  template <typename T>
+  static T BytesInternal(int width,
+                         int height,
+                         ResourceFormat format,
+                         bool verify_size,
+                         bool aligned);
+
+  DISALLOW_COPY_AND_ASSIGN(ResourceUtil);
+};
+
+template <typename T>
+bool ResourceUtil::VerifyWidthInBytes(int width, ResourceFormat format) {
+  VERIFY_INTEGER_TYPE;
+  return VerifyFitsInBytesInternal<T>(width, 0, format, false, false);
+}
+
+template <typename T>
+bool ResourceUtil::VerifySizeInBytes(const gfx::Size& size,
+                                     ResourceFormat format) {
+  VERIFY_INTEGER_TYPE;
+  return VerifyFitsInBytesInternal<T>(size.width(), size.height(), format, true,
+                                      false);
+}
+
+template <typename T>
+T ResourceUtil::CheckedSizeInBytes(const gfx::Size& size,
+                                   ResourceFormat format) {
+  VERIFY_INTEGER_TYPE;
+  DCHECK(VerifyFitsInBytesInternal<T>(size.width(), size.height(), format, true,
+                                      false));
+  base::CheckedNumeric<T> checked_value = BitsPerPixel(format);
+  checked_value *= size.width();
+  checked_value = MathUtil::RoundUp<T>(checked_value.ValueOrDie(), 8);

[prashant.n, 2015/07/28 15:47:32]

Need to ensure that width is integral value.

If BitsPerPixel = 4 e.g. ETC1, and width is odd, last pixel should not be
truncated.

[ericrk, 2015/07/29 18:19:24]

If we were within 8 of MAX_INT, RoundUp could overflow... I don't think we are
likely to hit this, but it would be nice to have checked roudning logic for
completeness. At least a TODO would be good.

[prashant.n, 2015/07/30 18:45:02]

Handled and TODO added.

+  checked_value /= 8;
+  checked_value *= size.height();
+  return checked_value.ValueOrDie();
+}
+
+template <typename T>
+T ResourceUtil::UncheckedWidthInBytes(int width, ResourceFormat format) {
+  VERIFY_INTEGER_TYPE;
+  DCHECK(VerifyFitsInBytesInternal<T>(width, 0, format, false, false));
+  return BytesInternal<T>(width, 0, format, false, false);
+}
+
+template <typename T>
+T ResourceUtil::UncheckedSizeInBytes(const gfx::Size& size,
+                                     ResourceFormat format) {
+  VERIFY_INTEGER_TYPE;
+  DCHECK(VerifyFitsInBytesInternal<T>(size.width(), size.height(), format, true,
+                                      false));
+  return BytesInternal<T>(size.width(), size.height(), format, true, false);
+}
+
+template <typename T>
+T ResourceUtil::UncheckedWidthInBytesAligned(int width, ResourceFormat format) {
+  VERIFY_INTEGER_TYPE;
+  DCHECK(VerifyFitsInBytesInternal<T>(width, 0, format, false, true));
+  return BytesInternal<T>(width, 0, format, false, true);
+}
+
+template <typename T>
+T ResourceUtil::UncheckedSizeInBytesAligned(const gfx::Size& size,
+                                            ResourceFormat format) {
+  VERIFY_INTEGER_TYPE;
+  DCHECK(VerifyFitsInBytesInternal<T>(size.width(), size.height(), format, true,
+                                      true));
+  return BytesInternal<T>(size.width(), size.height(), format, true, true);
+}
+
+template <typename T>
+bool ResourceUtil::VerifyFitsInBytesInternal(int width,
+                                             int height,
+                                             ResourceFormat format,
+                                             bool verify_size,
+                                             bool aligned) {
+  base::CheckedNumeric<T> checked_value = BitsPerPixel(format);
+  checked_value *= width;
+  if (!checked_value.IsValid())
+    return false;
+
+  // Roundup bits to byte (8 bits) boundary. If width is 3 and BitsPerPixel is
+  // 4, then it should return 16, so that row pixels do not get truncated.
+  checked_value = MathUtil::RoundUp<T>(checked_value.ValueOrDie(), 8);

[ericrk, 2015/07/29 18:19:24]

same as above

+
+  // If aligned is true, bytes are aligned on 4-bytes boundaries for upload
+  // performance, assuming that GL_PACK_ALIGNMENT or GL_UNPACK_ALIGNMENT have
+  // not changed from default.
+  if (aligned) {
+    checked_value /= 8;
+    if (!checked_value.IsValid())
+      return false;
+    checked_value = MathUtil::RoundUp<T>(checked_value.ValueOrDie(), 4);
+  }
+
+  if (verify_size)
+    checked_value *= height;
+  if (!checked_value.IsValid())
+    return false;
+  T value = checked_value.ValueOrDie();
+  if ((value % 8) != 0)
+    return false;
+  return true;
+}
+
+template <typename T>
+T ResourceUtil::BytesInternal(int width,
+                              int height,
+                              ResourceFormat format,
+                              bool verify_size,
+                              bool aligned) {
+  T bytes = BitsPerPixel(format);
+  bytes *= width;
+  bytes = MathUtil::RoundUp<T>(bytes, 8);
+  bytes /= 8;
+  if (aligned)
+    bytes = MathUtil::RoundUp<T>(bytes, 4);
+  if (verify_size)
+    bytes *= height;
+
+  return bytes;
+}
+
+}  // namespace cc
+
+#endif  // CC_RESOURCES_RESOURCE_UTIL_H_