Merge changes I69ce001a,I14514017,I8e67cda7

* changes:
  minui: Remove the default and copy ctors for GRSurface.
  minui: Refactor GRSurfaceAdf.
  minui: Refactor GRSurfaceFbdev.

minui/graphics_adf.cpp

@@ -20,6 +20,7 @@
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sys/mman.h>
 #include <unistd.h>
 
@@ -28,51 +29,60 @@
 
 #include "minui/minui.h"
 
-MinuiBackendAdf::MinuiBackendAdf()
+GRSurfaceAdf::~GRSurfaceAdf() {
-    : intf_fd(-1), dev(), current_surface(0), n_surfaces(0), surfaces() {}
+  if (mmapped_buffer_) {
+    munmap(mmapped_buffer_, pitch * height);
+  }
+  if (fence_fd != -1) {
+    close(fence_fd);
+  }
+  if (fd != -1) {
+    close(fd);
+  }
+}
 
-int MinuiBackendAdf::SurfaceInit(const drm_mode_modeinfo* mode, GRSurfaceAdf* surf) {
+std::unique_ptr<GRSurfaceAdf> GRSurfaceAdf::Create(int intf_fd, const drm_mode_modeinfo* mode,
-  *surf = {};
+                                                   __u32 format, int* err) {
-  surf->fence_fd = -1;
+  __u32 offset;
-  surf->fd = adf_interface_simple_buffer_alloc(intf_fd, mode->hdisplay, mode->vdisplay, format,
+  __u32 pitch;
-                                               &surf->offset, &surf->pitch);
+  auto fd = adf_interface_simple_buffer_alloc(intf_fd, mode->hdisplay, mode->vdisplay, format,
-  if (surf->fd < 0) {
+                                              &offset, &pitch);
-    return surf->fd;
+
+  if (fd < 0) {
+    *err = fd;
+    return nullptr;
   }
 
-  surf->width = mode->hdisplay;
+  std::unique_ptr<GRSurfaceAdf> surf = std::unique_ptr<GRSurfaceAdf>(
-  surf->height = mode->vdisplay;
+      new GRSurfaceAdf(mode->hdisplay, mode->vdisplay, pitch, (format == DRM_FORMAT_RGB565 ? 2 : 4),
-  surf->row_bytes = surf->pitch;
+                       offset, pitch, fd));
-  surf->pixel_bytes = (format == DRM_FORMAT_RGB565) ? 2 : 4;
 
   auto mmapped =
       mmap(nullptr, surf->pitch * surf->height, PROT_WRITE, MAP_SHARED, surf->fd, surf->offset);
   if (mmapped == MAP_FAILED) {
-    int saved_errno = errno;
+    *err = -errno;
-    close(surf->fd);
+    return nullptr;
-    return -saved_errno;
   }
   surf->mmapped_buffer_ = static_cast<uint8_t*>(mmapped);
-  return 0;
+  return surf;
 }
 
+MinuiBackendAdf::MinuiBackendAdf() : intf_fd(-1), dev(), current_surface(0), n_surfaces(0) {}
+
 int MinuiBackendAdf::InterfaceInit() {
   adf_interface_data intf_data;
-  int err = adf_get_interface_data(intf_fd, &intf_data);
+  if (int err = adf_get_interface_data(intf_fd, &intf_data); err < 0) return err;
-  if (err < 0) return err;
 
-  int ret = 0;
+  int result = 0;
-  err = SurfaceInit(&intf_data.current_mode, &surfaces[0]);
+  surfaces[0] = GRSurfaceAdf::Create(intf_fd, &intf_data.current_mode, format, &result);
-  if (err < 0) {
+  if (!surfaces[0]) {
-    fprintf(stderr, "allocating surface 0 failed: %s\n", strerror(-err));
+    fprintf(stderr, "Failed to allocate surface 0: %s\n", strerror(-result));
-    ret = err;
     goto done;
   }
 
-  err = SurfaceInit(&intf_data.current_mode, &surfaces[1]);
+  surfaces[1] = GRSurfaceAdf::Create(intf_fd, &intf_data.current_mode, format, &result);
-  if (err < 0) {
+  if (!surfaces[1]) {
-    fprintf(stderr, "allocating surface 1 failed: %s\n", strerror(-err));
+    fprintf(stderr, "Failed to allocate surface 1: %s\n", strerror(-result));
-    surfaces[1] = {};
     n_surfaces = 1;
   } else {
     n_surfaces = 2;
@@ -80,7 +90,7 @@ int MinuiBackendAdf::InterfaceInit() {
 
 done:
   adf_free_interface_data(&intf_data);
-  return ret;
+  return result;
 }
 
 int MinuiBackendAdf::DeviceInit(adf_device* dev) {
@@ -153,12 +163,12 @@ GRSurface* MinuiBackendAdf::Init() {
 }
 
 void MinuiBackendAdf::Sync(GRSurfaceAdf* surf) {
-  static constexpr unsigned int warningTimeout = 3000;
+  static constexpr unsigned int kWarningTimeout = 3000;
 
   if (surf == nullptr) return;
 
   if (surf->fence_fd >= 0) {
-    int err = sync_wait(surf->fence_fd, warningTimeout);
+    int err = sync_wait(surf->fence_fd, kWarningTimeout);
     if (err < 0) {
       perror("adf sync fence wait error\n");
     }
@@ -169,33 +179,22 @@ void MinuiBackendAdf::Sync(GRSurfaceAdf* surf) {
 }
 
 GRSurface* MinuiBackendAdf::Flip() {
-  GRSurfaceAdf* surf = &surfaces[current_surface];
+  const auto& surf = surfaces[current_surface];
 
   int fence_fd = adf_interface_simple_post(intf_fd, eng_id, surf->width, surf->height, format,
                                            surf->fd, surf->offset, surf->pitch, -1);
   if (fence_fd >= 0) surf->fence_fd = fence_fd;
 
   current_surface = (current_surface + 1) % n_surfaces;
-  Sync(&surfaces[current_surface]);
+  Sync(surfaces[current_surface].get());
-  return &surfaces[current_surface];
+  return surfaces[current_surface].get();
 }
 
 void MinuiBackendAdf::Blank(bool blank) {
   adf_interface_blank(intf_fd, blank ? DRM_MODE_DPMS_OFF : DRM_MODE_DPMS_ON);
 }
 
-void MinuiBackendAdf::SurfaceDestroy(GRSurfaceAdf* surf) {
-  if (surf->mmapped_buffer_) {
-    munmap(surf->mmapped_buffer_, surf->pitch * surf->height);
-  }
-  close(surf->fence_fd);
-  close(surf->fd);
-}
-
 MinuiBackendAdf::~MinuiBackendAdf() {
   adf_device_close(&dev);
-  for (unsigned int i = 0; i < n_surfaces; i++) {
-    SurfaceDestroy(&surfaces[i]);
-  }
   if (intf_fd >= 0) close(intf_fd);
 }

minui/graphics_adf.h

@@ -17,6 +17,9 @@
 #pragma once
 
 #include <stdint.h>
+#include <sys/types.h>
+
+#include <memory>
 
 #include <adf/adf.h>
 
@@ -25,6 +28,11 @@
 
 class GRSurfaceAdf : public GRSurface {
  public:
+  ~GRSurfaceAdf() override;
+
+  static std::unique_ptr<GRSurfaceAdf> Create(int intf_fd, const drm_mode_modeinfo* mode,
+                                              __u32 format, int* err);
+
   uint8_t* data() override {
     return mmapped_buffer_;
   }
@@ -32,34 +40,36 @@ class GRSurfaceAdf : public GRSurface {
  private:
   friend class MinuiBackendAdf;
 
-  int fence_fd;
+  GRSurfaceAdf(int width, int height, int row_bytes, int pixel_bytes, __u32 offset, __u32 pitch,
-  int fd;
+               int fd)
-  __u32 offset;
+      : GRSurface(width, height, row_bytes, pixel_bytes), offset(offset), pitch(pitch), fd(fd) {}
-  __u32 pitch;
 
+  const __u32 offset;
+  const __u32 pitch;
+
+  int fd;
+  int fence_fd{ -1 };
   uint8_t* mmapped_buffer_{ nullptr };
 };
 
 class MinuiBackendAdf : public MinuiBackend {
  public:
+  MinuiBackendAdf();
+  ~MinuiBackendAdf() override;
   GRSurface* Init() override;
   GRSurface* Flip() override;
   void Blank(bool) override;
-  ~MinuiBackendAdf() override;
-  MinuiBackendAdf();
 
  private:
-  int SurfaceInit(const drm_mode_modeinfo* mode, GRSurfaceAdf* surf);
   int InterfaceInit();
   int DeviceInit(adf_device* dev);
-  void SurfaceDestroy(GRSurfaceAdf* surf);
   void Sync(GRSurfaceAdf* surf);
 
   int intf_fd;
   adf_id_t eng_id;
   __u32 format;
   adf_device dev;
-  unsigned int current_surface;
+  size_t current_surface;
-  unsigned int n_surfaces;
+  size_t n_surfaces;
-  GRSurfaceAdf surfaces[2];
+  std::unique_ptr<GRSurfaceAdf> surfaces[2];
 };

minui/graphics_drm.cpp

@@ -102,15 +102,15 @@ std::unique_ptr<GRSurfaceDrm> GRSurfaceDrm::Create(int drm_fd, int width, int he
     return nullptr;
   }
 
-  std::unique_ptr<GRSurfaceDrm> surface = std::make_unique<GRSurfaceDrm>(drm_fd);
+  // Cannot use std::make_unique to access non-public ctor.
-  surface->handle = create_dumb.handle;
+  auto surface = std::unique_ptr<GRSurfaceDrm>(new GRSurfaceDrm(
+      width, height, create_dumb.pitch, create_dumb.bpp / 8, drm_fd, create_dumb.handle));
 
   uint32_t handles[4], pitches[4], offsets[4];
 
   handles[0] = surface->handle;
   pitches[0] = create_dumb.pitch;
   offsets[0] = 0;
-
   if (drmModeAddFB2(drm_fd, width, height, format, handles, pitches, offsets, &surface->fb_id, 0) !=
       0) {
     perror("Failed to drmModeAddFB2");
@@ -124,10 +124,6 @@ std::unique_ptr<GRSurfaceDrm> GRSurfaceDrm::Create(int drm_fd, int width, int he
     return nullptr;
   }
 
-  surface->height = height;
-  surface->width = width;
-  surface->row_bytes = create_dumb.pitch;
-  surface->pixel_bytes = create_dumb.bpp / 8;
   auto mmapped = mmap(nullptr, surface->height * surface->row_bytes, PROT_READ | PROT_WRITE,
                       MAP_SHARED, drm_fd, map_dumb.offset);
   if (mmapped == MAP_FAILED) {

minui/graphics_drm.h

@@ -20,7 +20,6 @@
 
 #include <memory>
 
-#include <android-base/macros.h>
 #include <xf86drmMode.h>
 
 #include "graphics.h"
@@ -28,7 +27,6 @@
 
 class GRSurfaceDrm : public GRSurface {
  public:
-  explicit GRSurfaceDrm(int drm_fd) : drm_fd_(drm_fd) {}
   ~GRSurfaceDrm() override;
 
   // Creates a GRSurfaceDrm instance.
@@ -41,13 +39,14 @@ class GRSurfaceDrm : public GRSurface {
  private:
   friend class MinuiBackendDrm;
 
+  GRSurfaceDrm(int width, int height, int row_bytes, int pixel_bytes, int drm_fd, uint32_t handle)
+      : GRSurface(width, height, row_bytes, pixel_bytes), drm_fd_(drm_fd), handle(handle) {}
+
   const int drm_fd_;
 
   uint32_t fb_id{ 0 };
   uint32_t handle{ 0 };
   uint8_t* mmapped_buffer_{ nullptr };
-
-  DISALLOW_COPY_AND_ASSIGN(GRSurfaceDrm);
 };
 
 class MinuiBackendDrm : public MinuiBackend {

minui/graphics_fbdev.cpp

@@ -26,21 +26,27 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include <memory>
+
 #include "minui/minui.h"
 
-MinuiBackendFbdev::MinuiBackendFbdev() : gr_draw(nullptr), fb_fd(-1) {}
+std::unique_ptr<GRSurfaceFbdev> GRSurfaceFbdev::Create(int width, int height, int row_bytes,
+                                                       int pixel_bytes) {
+  // Cannot use std::make_unique to access non-public ctor.
+  return std::unique_ptr<GRSurfaceFbdev>(new GRSurfaceFbdev(width, height, row_bytes, pixel_bytes));
+}
 
 void MinuiBackendFbdev::Blank(bool blank) {
   int ret = ioctl(fb_fd, FBIOBLANK, blank ? FB_BLANK_POWERDOWN : FB_BLANK_UNBLANK);
   if (ret < 0) perror("ioctl(): blank");
 }
 
-void MinuiBackendFbdev::SetDisplayedFramebuffer(unsigned n) {
+void MinuiBackendFbdev::SetDisplayedFramebuffer(size_t n) {
   if (n > 1 || !double_buffered) return;
 
-  vi.yres_virtual = gr_framebuffer[0].height * 2;
+  vi.yres_virtual = gr_framebuffer[0]->height * 2;
-  vi.yoffset = n * gr_framebuffer[0].height;
+  vi.yoffset = n * gr_framebuffer[0]->height;
-  vi.bits_per_pixel = gr_framebuffer[0].pixel_bytes * 8;
+  vi.bits_per_pixel = gr_framebuffer[0]->pixel_bytes * 8;
   if (ioctl(fb_fd, FBIOPUT_VSCREENINFO, &vi) < 0) {
     perror("active fb swap failed");
   }
@@ -96,35 +102,31 @@ GRSurface* MinuiBackendFbdev::Init() {
 
   memset(bits, 0, fi.smem_len);
 
-  gr_framebuffer[0].width = vi.xres;
+  gr_framebuffer[0] =
-  gr_framebuffer[0].height = vi.yres;
+      GRSurfaceFbdev::Create(vi.xres, vi.yres, fi.line_length, vi.bits_per_pixel / 8);
-  gr_framebuffer[0].row_bytes = fi.line_length;
+  gr_framebuffer[0]->buffer_ = static_cast<uint8_t*>(bits);
-  gr_framebuffer[0].pixel_bytes = vi.bits_per_pixel / 8;
+  memset(gr_framebuffer[0]->buffer_, 0, gr_framebuffer[0]->height * gr_framebuffer[0]->row_bytes);
-  gr_framebuffer[0].buffer_ = static_cast<uint8_t*>(bits);
+
-  memset(gr_framebuffer[0].buffer_, 0, gr_framebuffer[0].height * gr_framebuffer[0].row_bytes);
+  gr_framebuffer[1] =
+      GRSurfaceFbdev::Create(gr_framebuffer[0]->width, gr_framebuffer[0]->height,
+                             gr_framebuffer[0]->row_bytes, gr_framebuffer[0]->pixel_bytes);
 
   /* check if we can use double buffering */
   if (vi.yres * fi.line_length * 2 <= fi.smem_len) {
     double_buffered = true;
 
-    gr_framebuffer[1] = gr_framebuffer[0];
+    gr_framebuffer[1]->buffer_ =
-    gr_framebuffer[1].buffer_ =
+        gr_framebuffer[0]->buffer_ + gr_framebuffer[0]->height * gr_framebuffer[0]->row_bytes;
-        gr_framebuffer[0].buffer_ + gr_framebuffer[0].height * gr_framebuffer[0].row_bytes;
-
-    gr_draw = gr_framebuffer + 1;
-
   } else {
     double_buffered = false;
 
-    // Without double-buffering, we allocate RAM for a buffer to
+    // Without double-buffering, we allocate RAM for a buffer to draw in, and then "flipping" the
-    // draw in, and then "flipping" the buffer consists of a
+    // buffer consists of a memcpy from the buffer we allocated to the framebuffer.
-    // memcpy from the buffer we allocated to the framebuffer.
+    memory_buffer.resize(gr_framebuffer[1]->height * gr_framebuffer[1]->row_bytes);
-
+    gr_framebuffer[1]->buffer_ = memory_buffer.data();
-    gr_draw = new GRSurfaceFbdev;
-    *gr_draw = gr_framebuffer[0];
-    gr_draw->buffer_ = new uint8_t[gr_draw->height * gr_draw->row_bytes];
   }
 
+  gr_draw = gr_framebuffer[1].get();
   memset(gr_draw->buffer_, 0, gr_draw->height * gr_draw->row_bytes);
   fb_fd = fd;
   SetDisplayedFramebuffer(0);
@@ -139,25 +141,19 @@ GRSurface* MinuiBackendFbdev::Init() {
 
 GRSurface* MinuiBackendFbdev::Flip() {
   if (double_buffered) {
-    // Change gr_draw to point to the buffer currently displayed,
+    // Change gr_draw to point to the buffer currently displayed, then flip the driver so we're
-    // then flip the driver so we're displaying the other buffer
+    // displaying the other buffer instead.
-    // instead.
+    gr_draw = gr_framebuffer[displayed_buffer].get();
-    gr_draw = gr_framebuffer + displayed_buffer;
     SetDisplayedFramebuffer(1 - displayed_buffer);
   } else {
     // Copy from the in-memory surface to the framebuffer.
-    memcpy(gr_framebuffer[0].buffer_, gr_draw->buffer_, gr_draw->height * gr_draw->row_bytes);
+    memcpy(gr_framebuffer[0]->buffer_, gr_draw->buffer_, gr_draw->height * gr_draw->row_bytes);
   }
   return gr_draw;
 }
 
 MinuiBackendFbdev::~MinuiBackendFbdev() {
-  close(fb_fd);
+  if (fb_fd != -1) {
-  fb_fd = -1;
+    close(fb_fd);
-
-  if (!double_buffered && gr_draw) {
-    delete[] gr_draw->buffer_;
-    delete gr_draw;
   }
-  gr_draw = nullptr;
 }

minui/graphics_fbdev.h

@@ -19,37 +19,50 @@
 #include <linux/fb.h>
 #include <stdint.h>
 
+#include <memory>
+#include <vector>
+
 #include "graphics.h"
 #include "minui/minui.h"
 
 class GRSurfaceFbdev : public GRSurface {
  public:
+  // Creates and returns a GRSurfaceFbdev instance, or nullptr on error.
+  static std::unique_ptr<GRSurfaceFbdev> Create(int width, int height, int row_bytes,
+                                                int pixel_bytes);
+
   uint8_t* data() override {
     return buffer_;
   }
 
+ protected:
+  using GRSurface::GRSurface;
+
  private:
   friend class MinuiBackendFbdev;
 
   // Points to the start of the buffer: either the mmap'd framebuffer or one allocated in-memory.
-  uint8_t* buffer_;
+  uint8_t* buffer_{ nullptr };
 };
 
 class MinuiBackendFbdev : public MinuiBackend {
  public:
+  MinuiBackendFbdev() = default;
+  ~MinuiBackendFbdev() override;
+
   GRSurface* Init() override;
   GRSurface* Flip() override;
   void Blank(bool) override;
-  ~MinuiBackendFbdev() override;
-  MinuiBackendFbdev();
 
  private:
-  void SetDisplayedFramebuffer(unsigned n);
+  void SetDisplayedFramebuffer(size_t n);
 
-  GRSurfaceFbdev gr_framebuffer[2];
+  std::unique_ptr<GRSurfaceFbdev> gr_framebuffer[2];
+  // Points to the current surface (i.e. one of the two gr_framebuffer's).
+  GRSurfaceFbdev* gr_draw{ nullptr };
   bool double_buffered;
-  GRSurfaceFbdev* gr_draw;
+  std::vector<uint8_t> memory_buffer;
-  int displayed_buffer;
+  size_t displayed_buffer{ 0 };
   fb_var_screeninfo vi;
-  int fb_fd;
+  int fb_fd{ -1 };
 };

minui/include/minui/minui.h

@@ -24,13 +24,14 @@
 #include <string>
 #include <vector>
 
+#include <android-base/macros.h>
+
 //
 // Graphics.
 //
 
 class GRSurface {
  public:
-  GRSurface() = default;
   virtual ~GRSurface();
 
   // Creates and returns a GRSurface instance that's sufficient for storing an image of the given
@@ -58,6 +59,8 @@ class GRSurface {
 
  private:
   uint8_t* data_{ nullptr };
+
+  DISALLOW_COPY_AND_ASSIGN(GRSurface);
 };
 
 struct GRFont {