[Remoting] OpenGL Rendering Layer

remoting/client/gl_render_layer.cc

+// Copyright 2016 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 "remoting/client/gl_render_layer.h"
+
+#include "base/logging.h"
+#include "remoting/client/gl_canvas.h"
+#include "remoting/client/gl_helpers.h"
+
+namespace {
+
+// Assign texture coordinates to buffers for use in shader program.
+const float kVertices[] = {
+    // Points order: upper-left, bottom-left, upper-right, bottom-right.
+
+    // Positions to draw the texture on the normalized canvas coordinate.
+    0, 0, 0, 1, 1, 0, 1, 1,
+
+    // Region of the texture to be used (normally the whole texture).
+    0, 0, 0, 1, 1, 0, 1, 1};
+
+const int kBytesPerPixel = 4;
+
+const int kDefaultUpdateBufferCapacity = 2048 * 2048 * kBytesPerPixel;
+
+}
+
+namespace remoting {
+
+GlRenderLayer::GlRenderLayer(int texture_id, GlCanvas* canvas)
+    : texture_id_(texture_id), canvas_(canvas) {
+  texture_handle_ = CreateTexture();
+  buffer_handle_ = CreateBuffer(kVertices, sizeof(kVertices));
+}
+
+GlRenderLayer::~GlRenderLayer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  glDeleteBuffers(1, &buffer_handle_);
+  glDeleteTextures(1, &texture_handle_);
+}
+
+void GlRenderLayer::SetTexture(const void* texture, int width, int height) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CHECK(width > 0 && height > 0);
+  texture_set_ = true;
+  glActiveTexture(GL_TEXTURE0 + texture_id_);
+  glBindTexture(GL_TEXTURE_2D, texture_handle_);
+
+  glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA,
+               GL_UNSIGNED_BYTE, texture);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+
+  glBindTexture(GL_TEXTURE_2D, 0);
+}
+
+void GlRenderLayer::UpdateTexture(const void* subtexture,

[Sergey Ulanov, 2016/07/09 01:01:20]

Use uint8_t* for the buffer?

[Yuwei, 2016/07/11 22:38:24]

Done.

+                                  int offset_x,
+                                  int offset_y,
+                                  int width,
+                                  int height,
+                                  int pixel_stride) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CHECK(texture_set_);

[Sergey Ulanov, 2016/07/09 01:01:20]

Do you really need CHECK() instead of DCHECK()?

[Yuwei, 2016/07/11 22:38:24]

Not sure when CHECK() will be useful...

[Sergey Ulanov, 2016/07/13 04:42:11]

I CHECK is useful in two cases:
 - when violation of the verified condition may cause security vulnerability
(e.g. buffer overflow).
 - When there is a hard to debug crash in release builds. CHECK() may make it
easier to debug in some cases.

Also see
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md 

[Yuwei, 2016/07/13 18:14:16]

Acknowledged.

+  CHECK(width > 0 && height > 0);

[Sergey Ulanov, 2016/07/09 01:01:20]

DCHECK?

[Yuwei, 2016/07/11 22:38:24]

Done.

+  glActiveTexture(GL_TEXTURE0 + texture_id_);
+  glBindTexture(GL_TEXTURE_2D, texture_handle_);
+
+  bool loosely_packed = pixel_stride > 0 && pixel_stride != width;
+
+  const void* buffer_to_update = subtexture;
+
+  if (loosely_packed) {
+    if (canvas_->GetGlVersion() >= 3) {
+      glPixelStorei(GL_UNPACK_ROW_LENGTH, pixel_stride);
+    } else {
+      // Doesn't support GL_UNPACK_ROW_LENGTH. Manually packs the data.
+      int required_size = width * height * kBytesPerPixel;
+      if (update_buffer_size_ < required_size) {
+        if (required_size < kDefaultUpdateBufferCapacity) {
+          update_buffer_size_ = kDefaultUpdateBufferCapacity;
+        } else {
+          update_buffer_size_ = required_size;
+        }
+        update_buffer_.reset(new uint8_t[update_buffer_size_]);
+      }
+      PackDirtyRegion(update_buffer_.get(), subtexture, width, height,

[Sergey Ulanov, 2016/07/09 01:01:20]

This can be avoided when stride == pixel_size*width. Add a special case for
this?

[Yuwei, 2016/07/09 01:21:24]

Isn't it covered by line 70?

[Sergey Ulanov, 2016/07/13 04:42:11]

yes, sorry, I missed it.

+                      pixel_stride);
+      buffer_to_update = update_buffer_.get();
+    }
+  }
+
+  glTexSubImage2D(GL_TEXTURE_2D, 0, offset_x, offset_y, width, height, GL_RGBA,
+                  GL_UNSIGNED_BYTE, buffer_to_update);
+
+  if (loosely_packed && canvas_->GetGlVersion() >= 3) {
+    glPixelStorei(GL_UNPACK_ROW_LENGTH, 0);
+  }
+  glBindTexture(GL_TEXTURE_2D, 0);
+}
+
+void GlRenderLayer::SetVertexPositions(const float positions[8]) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  glBindBuffer(GL_ARRAY_BUFFER, buffer_handle_);
+  glBufferSubData(GL_ARRAY_BUFFER, 0, sizeof(kVertices) / 2, positions);
+  glBindBuffer(GL_ARRAY_BUFFER, 0);
+}
+
+void GlRenderLayer::SetTextureVisibleArea(const float positions[8]) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  glBindBuffer(GL_ARRAY_BUFFER, buffer_handle_);
+  glBufferSubData(GL_ARRAY_BUFFER, sizeof(kVertices) / 2, sizeof(kVertices) / 2,
+                  positions);
+  glBindBuffer(GL_ARRAY_BUFFER, 0);
+}
+
+void GlRenderLayer::Draw() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CHECK(texture_set_);
+  canvas_->DrawTexture(texture_id_, texture_handle_, buffer_handle_);
+}
+
+

[Sergey Ulanov, 2016/07/09 01:01:20]

remove extra empty line

[Yuwei, 2016/07/11 22:38:24]

Obsolete.

+// static
+void GlRenderLayer::PackDirtyRegion(void* dest,

[Sergey Ulanov, 2016/07/09 01:01:20]

Doesn't need to be class member. 

[Yuwei, 2016/07/11 22:38:24]

Done. Made it inline function.

+                                    const void* source,
+                                    int width,
+                                    int height,
+                                    int stride) {
+  for (int i = 0; i < height; i++) {
+    const uint8_t* current_source =
+        reinterpret_cast<const uint8_t*>(source) + kBytesPerPixel * stride * i;

[Sergey Ulanov, 2016/07/09 01:01:20]

You wouldn't need this cast with uint8_t* used for arguments instead of void*.

[Yuwei, 2016/07/11 22:38:24]

Done.

+    uint8_t* current_dest =
+        reinterpret_cast<uint8_t*>(dest) + kBytesPerPixel * width * i;

[Sergey Ulanov, 2016/07/09 01:01:20]

Instead of using multiplication to calculate position you could add stride after
copying each line:
 dest += kBytesPerPixel * width;
 source += stride;

This makes code cleaner and potentially slightly faster.

[Yuwei, 2016/07/11 22:38:24]

Done.

+    memcpy(current_dest, current_source, width * kBytesPerPixel);
+  }
+}
+
+}  // namespace remoting