From 92bdb5a38964baf8326a7d6f53926c30e250922c Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Sun, 21 Oct 2018 12:12:37 -0700 Subject: minui: Move GRSurface into a class. This CL adds GRSurface::Create() and dtor for managing the allocated memory in GRSurface class. It also adds GRSurface::data() that hides the underlying implementation, with both of const and non-const overloads. This allows `const GRSurface&` to be more useful - previously it only ensured a const member variable of `data`, instead of a read-only buffer it points to. It also marks the parameters in gr_texticon() and gr_blit() as const, as they're incoming source that shouldn't be altered. It corrects the type of gr_draw, which is the sink to be painted on (an earlier attempt was made in [1], but didn't get the full picture correctly). [1] https://android-review.googlesource.com/c/platform/bootable/recovery/+/704757/ Test: mmma -j bootable/recovery Test: recovery_unit_test on marlin Test: Run graphics test on marlin (fbdev). Test: Run graphics test on blueline (drm). Change-Id: I7904df084cd6c08fa04a9da97d01b4b1a6e3a20c --- minui/graphics.cpp | 59 +++++++++++++++++++------------------- minui/graphics_adf.cpp | 12 ++++---- minui/graphics_adf.h | 17 +++++++---- minui/graphics_drm.cpp | 13 ++++----- minui/graphics_drm.h | 15 ++++++---- minui/graphics_fbdev.cpp | 28 ++++++++---------- minui/graphics_fbdev.h | 23 +++++++++++---- minui/include/minui/minui.h | 34 ++++++++++++++++------ minui/resources.cpp | 70 ++++++++++++++++++++++++--------------------- screen_ui.h | 2 +- tests/unit/minui_test.cpp | 32 +++++++++++++++++++++ 11 files changed, 190 insertions(+), 115 deletions(-) create mode 100644 tests/unit/minui_test.cpp diff --git a/minui/graphics.cpp b/minui/graphics.cpp index e6367d950..4d1f9b2d2 100644 --- a/minui/graphics.cpp +++ b/minui/graphics.cpp @@ -40,7 +40,7 @@ static uint32_t gr_current = ~0; static constexpr uint32_t alpha_mask = 0xff000000; // gr_draw is owned by backends. -static const GRSurface* gr_draw = nullptr; +static GRSurface* gr_draw = nullptr; static GRRotation rotation = GRRotation::NONE; static PixelFormat pixel_format = PixelFormat::UNKNOWN; @@ -121,28 +121,29 @@ static void incr_y(uint32_t** p, int row_pixels) { } // Returns pixel pointer at given coordinates with rotation adjustment. -static uint32_t* pixel_at(const GRSurface* surf, int x, int y, int row_pixels) { +static uint32_t* PixelAt(GRSurface* surface, int x, int y, int row_pixels) { switch (rotation) { case GRRotation::NONE: - return reinterpret_cast(surf->data) + y * row_pixels + x; + return reinterpret_cast(surface->data()) + y * row_pixels + x; case GRRotation::RIGHT: - return reinterpret_cast(surf->data) + x * row_pixels + (surf->width - y); + return reinterpret_cast(surface->data()) + x * row_pixels + (surface->width - y); case GRRotation::DOWN: - return reinterpret_cast(surf->data) + (surf->height - 1 - y) * row_pixels + - (surf->width - 1 - x); + return reinterpret_cast(surface->data()) + (surface->height - 1 - y) * row_pixels + + (surface->width - 1 - x); case GRRotation::LEFT: - return reinterpret_cast(surf->data) + (surf->height - 1 - x) * row_pixels + y; + return reinterpret_cast(surface->data()) + (surface->height - 1 - x) * row_pixels + + y; default: printf("invalid rotation %d", static_cast(rotation)); } return nullptr; } -static void text_blend(uint8_t* src_p, int src_row_bytes, uint32_t* dst_p, int dst_row_pixels, - int width, int height) { +static void TextBlend(const uint8_t* src_p, int src_row_bytes, uint32_t* dst_p, int dst_row_pixels, + int width, int height) { uint8_t alpha_current = static_cast((alpha_mask & gr_current) >> 24); for (int j = 0; j < height; ++j) { - uint8_t* sx = src_p; + const uint8_t* sx = src_p; uint32_t* px = dst_p; for (int i = 0; i < width; ++i, incr_x(&px, dst_row_pixels)) { uint8_t a = *sx++; @@ -176,18 +177,18 @@ void gr_text(const GRFont* font, int x, int y, const char* s, bool bold) { } int row_pixels = gr_draw->row_bytes / gr_draw->pixel_bytes; - uint8_t* src_p = font->texture->data + ((ch - ' ') * font->char_width) + - (bold ? font->char_height * font->texture->row_bytes : 0); - uint32_t* dst_p = pixel_at(gr_draw, x, y, row_pixels); + const uint8_t* src_p = font->texture->data() + ((ch - ' ') * font->char_width) + + (bold ? font->char_height * font->texture->row_bytes : 0); + uint32_t* dst_p = PixelAt(gr_draw, x, y, row_pixels); - text_blend(src_p, font->texture->row_bytes, dst_p, row_pixels, font->char_width, - font->char_height); + TextBlend(src_p, font->texture->row_bytes, dst_p, row_pixels, font->char_width, + font->char_height); x += font->char_width; } } -void gr_texticon(int x, int y, GRSurface* icon) { +void gr_texticon(int x, int y, const GRSurface* icon) { if (icon == nullptr) return; if (icon->pixel_bytes != 1) { @@ -201,10 +202,9 @@ void gr_texticon(int x, int y, GRSurface* icon) { if (outside(x, y) || outside(x + icon->width - 1, y + icon->height - 1)) return; int row_pixels = gr_draw->row_bytes / gr_draw->pixel_bytes; - uint8_t* src_p = icon->data; - uint32_t* dst_p = pixel_at(gr_draw, x, y, row_pixels); - - text_blend(src_p, icon->row_bytes, dst_p, row_pixels, icon->width, icon->height); + const uint8_t* src_p = icon->data(); + uint32_t* dst_p = PixelAt(gr_draw, x, y, row_pixels); + TextBlend(src_p, icon->row_bytes, dst_p, row_pixels, icon->width, icon->height); } void gr_color(unsigned char r, unsigned char g, unsigned char b, unsigned char a) { @@ -221,9 +221,9 @@ void gr_clear() { (gr_current & 0xff) == ((gr_current >> 16) & 0xff) && (gr_current & 0xff) == ((gr_current >> 24) & 0xff) && gr_draw->row_bytes == gr_draw->width * gr_draw->pixel_bytes) { - memset(gr_draw->data, gr_current & 0xff, gr_draw->height * gr_draw->row_bytes); + memset(gr_draw->data(), gr_current & 0xff, gr_draw->height * gr_draw->row_bytes); } else { - uint32_t* px = reinterpret_cast(gr_draw->data); + uint32_t* px = reinterpret_cast(gr_draw->data()); int row_diff = gr_draw->row_bytes / gr_draw->pixel_bytes - gr_draw->width; for (int y = 0; y < gr_draw->height; ++y) { for (int x = 0; x < gr_draw->width; ++x) { @@ -244,7 +244,7 @@ void gr_fill(int x1, int y1, int x2, int y2) { if (outside(x1, y1) || outside(x2 - 1, y2 - 1)) return; int row_pixels = gr_draw->row_bytes / gr_draw->pixel_bytes; - uint32_t* p = pixel_at(gr_draw, x1, y1, row_pixels); + uint32_t* p = PixelAt(gr_draw, x1, y1, row_pixels); uint8_t alpha = static_cast(((gr_current & alpha_mask) >> 24)); if (alpha > 0) { for (int y = y1; y < y2; ++y) { @@ -258,7 +258,7 @@ void gr_fill(int x1, int y1, int x2, int y2) { } } -void gr_blit(GRSurface* source, int sx, int sy, int w, int h, int dx, int dy) { +void gr_blit(const GRSurface* source, int sx, int sy, int w, int h, int dx, int dy) { if (source == nullptr) return; if (gr_draw->pixel_bytes != source->pixel_bytes) { @@ -274,11 +274,12 @@ void gr_blit(GRSurface* source, int sx, int sy, int w, int h, int dx, int dy) { if (rotation != GRRotation::NONE) { int src_row_pixels = source->row_bytes / source->pixel_bytes; int row_pixels = gr_draw->row_bytes / gr_draw->pixel_bytes; - uint32_t* src_py = reinterpret_cast(source->data) + sy * source->row_bytes / 4 + sx; - uint32_t* dst_py = pixel_at(gr_draw, dx, dy, row_pixels); + const uint32_t* src_py = + reinterpret_cast(source->data()) + sy * source->row_bytes / 4 + sx; + uint32_t* dst_py = PixelAt(gr_draw, dx, dy, row_pixels); for (int y = 0; y < h; y += 1) { - uint32_t* src_px = src_py; + const uint32_t* src_px = src_py; uint32_t* dst_px = dst_py; for (int x = 0; x < w; x += 1) { *dst_px = *src_px++; @@ -288,8 +289,8 @@ void gr_blit(GRSurface* source, int sx, int sy, int w, int h, int dx, int dy) { incr_y(&dst_py, row_pixels); } } else { - unsigned char* src_p = source->data + sy * source->row_bytes + sx * source->pixel_bytes; - unsigned char* dst_p = gr_draw->data + dy * gr_draw->row_bytes + dx * gr_draw->pixel_bytes; + const uint8_t* src_p = source->data() + sy * source->row_bytes + sx * source->pixel_bytes; + uint8_t* dst_p = gr_draw->data() + dy * gr_draw->row_bytes + dx * gr_draw->pixel_bytes; for (int i = 0; i < h; ++i) { memcpy(dst_p, src_p, w * source->pixel_bytes); diff --git a/minui/graphics_adf.cpp b/minui/graphics_adf.cpp index 7439df9ac..6fc193f74 100644 --- a/minui/graphics_adf.cpp +++ b/minui/graphics_adf.cpp @@ -45,14 +45,14 @@ int MinuiBackendAdf::SurfaceInit(const drm_mode_modeinfo* mode, GRSurfaceAdf* su surf->row_bytes = surf->pitch; surf->pixel_bytes = (format == DRM_FORMAT_RGB565) ? 2 : 4; - surf->data = static_cast( - mmap(nullptr, surf->pitch * surf->height, PROT_WRITE, MAP_SHARED, surf->fd, surf->offset)); - if (surf->data == MAP_FAILED) { + auto mmapped = + mmap(nullptr, surf->pitch * surf->height, PROT_WRITE, MAP_SHARED, surf->fd, surf->offset); + if (mmapped == MAP_FAILED) { int saved_errno = errno; close(surf->fd); return -saved_errno; } - + surf->mmapped_buffer_ = static_cast(mmapped); return 0; } @@ -185,7 +185,9 @@ void MinuiBackendAdf::Blank(bool blank) { } void MinuiBackendAdf::SurfaceDestroy(GRSurfaceAdf* surf) { - munmap(surf->data, surf->pitch * surf->height); + if (surf->mmapped_buffer_) { + munmap(surf->mmapped_buffer_, surf->pitch * surf->height); + } close(surf->fence_fd); close(surf->fd); } diff --git a/minui/graphics_adf.h b/minui/graphics_adf.h index 2f019ed0b..099d32962 100644 --- a/minui/graphics_adf.h +++ b/minui/graphics_adf.h @@ -14,21 +14,30 @@ * limitations under the License. */ -#ifndef _GRAPHICS_ADF_H_ -#define _GRAPHICS_ADF_H_ +#pragma once + +#include #include #include "graphics.h" +#include "minui/minui.h" class GRSurfaceAdf : public GRSurface { + public: + uint8_t* data() override { + return mmapped_buffer_; + } + private: + friend class MinuiBackendAdf; + int fence_fd; int fd; __u32 offset; __u32 pitch; - friend class MinuiBackendAdf; + uint8_t* mmapped_buffer_{ nullptr }; }; class MinuiBackendAdf : public MinuiBackend { @@ -54,5 +63,3 @@ class MinuiBackendAdf : public MinuiBackend { unsigned int n_surfaces; GRSurfaceAdf surfaces[2]; }; - -#endif // _GRAPHICS_ADF_H_ diff --git a/minui/graphics_drm.cpp b/minui/graphics_drm.cpp index 630b80180..81b49fd95 100644 --- a/minui/graphics_drm.cpp +++ b/minui/graphics_drm.cpp @@ -70,8 +70,8 @@ void MinuiBackendDrm::Blank(bool blank) { void MinuiBackendDrm::DrmDestroySurface(GRSurfaceDrm* surface) { if (!surface) return; - if (surface->data) { - munmap(surface->data, surface->row_bytes * surface->height); + if (surface->mmapped_buffer_) { + munmap(surface->mmapped_buffer_, surface->row_bytes * surface->height); } if (surface->fb_id) { @@ -172,15 +172,14 @@ GRSurfaceDrm* MinuiBackendDrm::DrmCreateSurface(int width, int height) { surface->width = width; surface->row_bytes = create_dumb.pitch; surface->pixel_bytes = create_dumb.bpp / 8; - surface->data = static_cast(mmap(nullptr, surface->height * surface->row_bytes, - PROT_READ | PROT_WRITE, MAP_SHARED, drm_fd, - map_dumb.offset)); - if (surface->data == MAP_FAILED) { + auto mmapped = mmap(nullptr, surface->height * surface->row_bytes, PROT_READ | PROT_WRITE, + MAP_SHARED, drm_fd, map_dumb.offset); + if (mmapped == MAP_FAILED) { perror("mmap() failed"); DrmDestroySurface(surface); return nullptr; } - + surface->mmapped_buffer_ = static_cast(mmapped); return surface; } diff --git a/minui/graphics_drm.h b/minui/graphics_drm.h index 756625b03..f3aad6bfc 100644 --- a/minui/graphics_drm.h +++ b/minui/graphics_drm.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef _GRAPHICS_DRM_H_ -#define _GRAPHICS_DRM_H_ +#pragma once #include @@ -25,11 +24,17 @@ #include "minui/minui.h" class GRSurfaceDrm : public GRSurface { + public: + uint8_t* data() override { + return mmapped_buffer_; + } + private: + friend class MinuiBackendDrm; + uint32_t fb_id; uint32_t handle; - - friend class MinuiBackendDrm; + uint8_t* mmapped_buffer_{ nullptr }; }; class MinuiBackendDrm : public MinuiBackend { @@ -54,5 +59,3 @@ class MinuiBackendDrm : public MinuiBackend { drmModeConnector* main_monitor_connector; int drm_fd; }; - -#endif // _GRAPHICS_DRM_H_ diff --git a/minui/graphics_fbdev.cpp b/minui/graphics_fbdev.cpp index 746f42aaa..f958d62d4 100644 --- a/minui/graphics_fbdev.cpp +++ b/minui/graphics_fbdev.cpp @@ -100,16 +100,16 @@ GRSurface* MinuiBackendFbdev::Init() { gr_framebuffer[0].height = vi.yres; gr_framebuffer[0].row_bytes = fi.line_length; gr_framebuffer[0].pixel_bytes = vi.bits_per_pixel / 8; - gr_framebuffer[0].data = static_cast(bits); - memset(gr_framebuffer[0].data, 0, gr_framebuffer[0].height * gr_framebuffer[0].row_bytes); + gr_framebuffer[0].buffer_ = static_cast(bits); + memset(gr_framebuffer[0].buffer_, 0, gr_framebuffer[0].height * gr_framebuffer[0].row_bytes); /* check if we can use double buffering */ if (vi.yres * fi.line_length * 2 <= fi.smem_len) { double_buffered = true; - memcpy(gr_framebuffer + 1, gr_framebuffer, sizeof(GRSurface)); - gr_framebuffer[1].data = - gr_framebuffer[0].data + gr_framebuffer[0].height * gr_framebuffer[0].row_bytes; + gr_framebuffer[1] = gr_framebuffer[0]; + gr_framebuffer[1].buffer_ = + gr_framebuffer[0].buffer_ + gr_framebuffer[0].height * gr_framebuffer[0].row_bytes; gr_draw = gr_framebuffer + 1; @@ -120,16 +120,12 @@ GRSurface* MinuiBackendFbdev::Init() { // draw in, and then "flipping" the buffer consists of a // memcpy from the buffer we allocated to the framebuffer. - gr_draw = static_cast(malloc(sizeof(GRSurface))); - memcpy(gr_draw, gr_framebuffer, sizeof(GRSurface)); - gr_draw->data = static_cast(malloc(gr_draw->height * gr_draw->row_bytes)); - if (!gr_draw->data) { - perror("failed to allocate in-memory surface"); - return nullptr; - } + gr_draw = new GRSurfaceFbdev; + *gr_draw = gr_framebuffer[0]; + gr_draw->buffer_ = new uint8_t[gr_draw->height * gr_draw->row_bytes]; } - memset(gr_draw->data, 0, gr_draw->height * gr_draw->row_bytes); + memset(gr_draw->buffer_, 0, gr_draw->height * gr_draw->row_bytes); fb_fd = fd; SetDisplayedFramebuffer(0); @@ -150,7 +146,7 @@ GRSurface* MinuiBackendFbdev::Flip() { SetDisplayedFramebuffer(1 - displayed_buffer); } else { // Copy from the in-memory surface to the framebuffer. - memcpy(gr_framebuffer[0].data, gr_draw->data, 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; } @@ -160,8 +156,8 @@ MinuiBackendFbdev::~MinuiBackendFbdev() { fb_fd = -1; if (!double_buffered && gr_draw) { - free(gr_draw->data); - free(gr_draw); + delete[] gr_draw->buffer_; + delete gr_draw; } gr_draw = nullptr; } diff --git a/minui/graphics_fbdev.h b/minui/graphics_fbdev.h index 107e19567..be813dccb 100644 --- a/minui/graphics_fbdev.h +++ b/minui/graphics_fbdev.h @@ -14,14 +14,27 @@ * limitations under the License. */ -#ifndef _GRAPHICS_FBDEV_H_ -#define _GRAPHICS_FBDEV_H_ +#pragma once #include +#include #include "graphics.h" #include "minui/minui.h" +class GRSurfaceFbdev : public GRSurface { + public: + uint8_t* data() override { + return buffer_; + } + + private: + friend class MinuiBackendFbdev; + + // Points to the start of the buffer: either the mmap'd framebuffer or one allocated in-memory. + uint8_t* buffer_; +}; + class MinuiBackendFbdev : public MinuiBackend { public: GRSurface* Init() override; @@ -33,12 +46,10 @@ class MinuiBackendFbdev : public MinuiBackend { private: void SetDisplayedFramebuffer(unsigned n); - GRSurface gr_framebuffer[2]; + GRSurfaceFbdev gr_framebuffer[2]; bool double_buffered; - GRSurface* gr_draw; + GRSurfaceFbdev* gr_draw; int displayed_buffer; fb_var_screeninfo vi; int fb_fd; }; - -#endif // _GRAPHICS_FBDEV_H_ diff --git a/minui/include/minui/minui.h b/minui/include/minui/minui.h index fa13ecdff..e9bd1c4f1 100644 --- a/minui/include/minui/minui.h +++ b/minui/include/minui/minui.h @@ -14,12 +14,13 @@ * limitations under the License. */ -#ifndef _MINUI_H_ -#define _MINUI_H_ +#pragma once +#include #include #include +#include #include #include @@ -27,12 +28,31 @@ // Graphics. // -struct GRSurface { +class GRSurface { + public: + GRSurface() = default; + virtual ~GRSurface(); + + // Creates and returns a GRSurface instance for the given data_size. The starting address of the + // surface data is aligned to SURFACE_DATA_ALIGNMENT. Returns the created GRSurface instance (in + // std::unique_ptr), or nullptr on error. + static std::unique_ptr Create(size_t data_size); + + virtual uint8_t* data() { + return data_; + } + + const uint8_t* data() const { + return const_cast(const_cast(this)->data()); + } + int width; int height; int row_bytes; int pixel_bytes; - unsigned char* data; + + private: + uint8_t* data_{ nullptr }; }; struct GRFont { @@ -75,7 +95,7 @@ void gr_clear(); void gr_color(unsigned char r, unsigned char g, unsigned char b, unsigned char a); void gr_fill(int x1, int y1, int x2, int y2); -void gr_texticon(int x, int y, GRSurface* icon); +void gr_texticon(int x, int y, const GRSurface* icon); const GRFont* gr_sys_font(); int gr_init_font(const char* name, GRFont** dest); @@ -85,7 +105,7 @@ int gr_measure(const GRFont* font, const char* s); // Returns -1 if font is nullptr. int gr_font_size(const GRFont* font, int* x, int* y); -void gr_blit(GRSurface* source, int sx, int sy, int w, int h, int dx, int dy); +void gr_blit(const GRSurface* source, int sx, int sy, int w, int h, int dx, int dy); unsigned int gr_get_width(const GRSurface* surface); unsigned int gr_get_height(const GRSurface* surface); @@ -165,5 +185,3 @@ std::vector get_locales_in_png(const std::string& png_name); // Free a surface allocated by any of the res_create_*_surface() // functions. void res_free_surface(GRSurface* surface); - -#endif diff --git a/minui/resources.cpp b/minui/resources.cpp index 477fbe2a2..c01c1868e 100644 --- a/minui/resources.cpp +++ b/minui/resources.cpp @@ -37,18 +37,23 @@ #include "minui/minui.h" -#define SURFACE_DATA_ALIGNMENT 8 - static std::string g_resource_dir{ "/res/images" }; -static GRSurface* malloc_surface(size_t data_size) { - size_t size = sizeof(GRSurface) + data_size + SURFACE_DATA_ALIGNMENT; - unsigned char* temp = static_cast(malloc(size)); - if (temp == NULL) return NULL; - GRSurface* surface = reinterpret_cast(temp); - surface->data = temp + sizeof(GRSurface) + - (SURFACE_DATA_ALIGNMENT - (sizeof(GRSurface) % SURFACE_DATA_ALIGNMENT)); - return surface; +std::unique_ptr GRSurface::Create(size_t data_size) { + static constexpr size_t kSurfaceDataAlignment = 8; + std::unique_ptr result = std::make_unique(); + size_t aligned_size = + (data_size + kSurfaceDataAlignment - 1) / kSurfaceDataAlignment * kSurfaceDataAlignment; + result->data_ = static_cast(aligned_alloc(kSurfaceDataAlignment, aligned_size)); + if (result->data_ == nullptr) return nullptr; + return result; +} + +GRSurface::~GRSurface() { + if (data_ != nullptr) { + free(data_); + data_ = nullptr; + } } PngHandler::PngHandler(const std::string& name) { @@ -133,18 +138,18 @@ PngHandler::~PngHandler() { // framebuffer pixel format; they need to be modified if the // framebuffer format changes (but nothing else should). -// Allocate and return a GRSurface* sufficient for storing an image of -// the indicated size in the framebuffer pixel format. -static GRSurface* init_display_surface(png_uint_32 width, png_uint_32 height) { - GRSurface* surface = malloc_surface(width * height * 4); - if (surface == NULL) return NULL; +// Allocates and returns a GRSurface* sufficient for storing an image of the indicated size in the +// framebuffer pixel format. +static std::unique_ptr init_display_surface(png_uint_32 width, png_uint_32 height) { + std::unique_ptr surface = GRSurface::Create(width * height * 4); + if (!surface) return nullptr; - surface->width = width; - surface->height = height; - surface->row_bytes = width * 4; - surface->pixel_bytes = 4; + surface->width = width; + surface->height = height; + surface->row_bytes = width * 4; + surface->pixel_bytes = 4; - return surface; + return surface; } // Copy 'input_row' to 'output_row', transforming it to the @@ -202,7 +207,7 @@ int res_create_display_surface(const char* name, GRSurface** pSurface) { png_uint_32 width = png_handler.width(); png_uint_32 height = png_handler.height(); - GRSurface* surface = init_display_surface(width, height); + std::unique_ptr surface = init_display_surface(width, height); if (!surface) { return -8; } @@ -215,11 +220,11 @@ int res_create_display_surface(const char* name, GRSurface** pSurface) { for (png_uint_32 y = 0; y < height; ++y) { std::vector p_row(width * 4); png_read_row(png_ptr, p_row.data(), nullptr); - transform_rgb_to_draw(p_row.data(), surface->data + y * surface->row_bytes, + transform_rgb_to_draw(p_row.data(), surface->data() + y * surface->row_bytes, png_handler.channels(), width); } - *pSurface = surface; + *pSurface = surface.release(); return 0; } @@ -272,11 +277,12 @@ int res_create_multi_display_surface(const char* name, int* frames, int* fps, goto exit; } for (int i = 0; i < *frames; ++i) { - surface[i] = init_display_surface(width, height / *frames); - if (!surface[i]) { + auto created_surface = init_display_surface(width, height / *frames); + if (!created_surface) { result = -8; goto exit; } + surface[i] = created_surface.release(); } if (gr_pixel_format() == PixelFormat::ABGR || gr_pixel_format() == PixelFormat::BGRA) { @@ -287,7 +293,7 @@ int res_create_multi_display_surface(const char* name, int* frames, int* fps, std::vector p_row(width * 4); png_read_row(png_ptr, p_row.data(), nullptr); int frame = y % *frames; - unsigned char* out_row = surface[frame]->data + (y / *frames) * surface[frame]->row_bytes; + unsigned char* out_row = surface[frame]->data() + (y / *frames) * surface[frame]->row_bytes; transform_rgb_to_draw(p_row.data(), out_row, png_handler.channels(), width); } @@ -319,7 +325,7 @@ int res_create_alpha_surface(const char* name, GRSurface** pSurface) { png_uint_32 width = png_handler.width(); png_uint_32 height = png_handler.height(); - GRSurface* surface = malloc_surface(width * height); + std::unique_ptr surface = GRSurface::Create(width * height); if (!surface) { return -8; } @@ -334,11 +340,11 @@ int res_create_alpha_surface(const char* name, GRSurface** pSurface) { } for (png_uint_32 y = 0; y < height; ++y) { - unsigned char* p_row = surface->data + y * surface->row_bytes; + unsigned char* p_row = surface->data() + y * surface->row_bytes; png_read_row(png_ptr, p_row, nullptr); } - *pSurface = surface; + *pSurface = surface.release(); return 0; } @@ -429,7 +435,7 @@ int res_create_localized_alpha_surface(const char* name, if (y + 1 + h >= height || matches_locale(loc, locale)) { printf(" %20s: %s (%d x %d @ %d)\n", name, loc, w, h, y); - GRSurface* surface = malloc_surface(w * h); + std::unique_ptr surface = GRSurface::Create(w * h); if (!surface) { return -8; } @@ -440,10 +446,10 @@ int res_create_localized_alpha_surface(const char* name, for (int i = 0; i < h; ++i, ++y) { png_read_row(png_ptr, row.data(), nullptr); - memcpy(surface->data + i * w, row.data(), w); + memcpy(surface->data() + i * w, row.data(), w); } - *pSurface = surface; + *pSurface = surface.release(); break; } diff --git a/screen_ui.h b/screen_ui.h index 915288793..861b3605a 100644 --- a/screen_ui.h +++ b/screen_ui.h @@ -29,7 +29,7 @@ #include "ui.h" // From minui/minui.h. -struct GRSurface; +class GRSurface; enum class UIElement { HEADER, diff --git a/tests/unit/minui_test.cpp b/tests/unit/minui_test.cpp new file mode 100644 index 000000000..cad6a3d79 --- /dev/null +++ b/tests/unit/minui_test.cpp @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include + +#include "minui/minui.h" + +TEST(GRSurfaceTest, Create_aligned) { + static constexpr size_t kSurfaceDataAlignment = 8; + for (size_t data_size = 100; data_size < 128; data_size++) { + std::unique_ptr surface(GRSurface::Create(data_size)); + ASSERT_TRUE(surface); + ASSERT_EQ(0, reinterpret_cast(surface->data()) % kSurfaceDataAlignment); + } +} -- cgit v1.2.3